Add desktop param to globalSummon; set _quake = toCurrent#9954
Conversation
This comment has been minimized.
This comment has been minimized.
carlos-zamora
left a comment
carlos-zamora
left a comment
There was a problem hiding this comment.
I want to approve, but this comment is concerning. That's really the only concern I have. Please keep me in the loop and I'll be an approve for you.
|
I guess don't forget the docs/schema too. But I think you wanted to go back and do that later? idk |
yea I got a bunch more to do haha. Plus, I'm gonna have that big ol' table in the docs. Makes sense to do it atomically. |
This comment has been minimized.
This comment has been minimized.
|
Okay ignore the above - I managed to fix that with some creative merges. |
src/cascadia/Remoting/Peasant.cpp
Outdated
| void Peasant::Summon(const Remoting::SummonWindowBehavior& summonBehavior) | ||
| { | ||
| _SummonRequestedHandlers(*this, nullptr); | ||
| auto localCopy = winrt::make_self<implementation::SummonWindowBehavior>(summonBehavior); |
There was a problem hiding this comment.
Why are we copying it? Is it because it's an OOP object, and we can get a local?
There was a problem hiding this comment.
Yea basically. Stash the local copy so that if during the SummonRequestedHandlers, the Monarch dies, then the Peasant can still complete the summon, without the handler knowing that the args are OOP.
ghost
added
Needs-Author-Feedback
(cherry picked from commit 9e94d348efa0783fa8381bc0297e0cec8153133a)
DHowett
force-pushed
the
dev/migrie/f/quake-toCurrent-desktop
branch
from
6c424f7 to
fe51444
Compare
|
I did a squash-merge-onto-base-branch cherry-pick-onto-main force-push manoeuvre. |
DHowett
changed the title
desktop param in globalSummon, Quake window moves to current desktopdesktop param to globalSummon; set _quake = toCurrent
This adds a `toggleVisibility` parameter to `globalSummon`. * When `true` (default): when you press the global summon keybinding, and the window is currently the foreground window, we'll minimize the window. * When `false`, we'll just do nothing. ## References * Original thread: #653 * Spec: #9274 * megathread: #8888 ## PR Checklist * [x] Checks a box in #8888 * [x] closes https://github.com/microsoft/terminal/projects/5#card-59030814 * [x] I work here * [ ] No tests for this one. * [ ] yes yes eventually I'll come back on the docs ## Detailed Description of the Pull Request / Additional comments I've got nothing extra to add here. This one's pretty simple. I'm only targeting #9954 since that one laid so much foundation to build on, with the `SummonBehavior` ## Validation Steps Performed Played with this for a while, and it's amazing.
… already (#10025) ## Summary of the Pull Request This is to mitigate MSFT:33035972. If you call `MoveWindowToDesktop` while an app is set to "Show windows from this app on all desktops", the OS will clear that "Show windows from this app on all desktops" state. But it _won't_ clear that state from the task view, so it'll just plain look broken. We can mitigate this just by checking if we're already on the current desktop first. "Show windows from this app on all desktops" windows will _always_ be on every desktop, so that API will return true, and we can avoid tearing the state. ## References * added in #9954 ## PR Checklist * [x] Closes https://github.com/microsoft/terminal/projects/5#card-60325102 * [x] I work here * [ ] Tests aren't possible * [n/a] Requires documentation to be updated ## Validation Steps Performed * it works again
|
🎉 Handy links: |
3 participants
This adds support for the
desktopparam to theglobalSummonaction. It accepts 3 values:toCurrent(default): The window moves to the current desktop when it's summonedany: We don't care what desktop the window is on. We'll go to the desktop the window is on when we summon it.onCurrent: We'll only try to summon the MRU window on this desktop when summoning a window.name, if there's a window matchingname, we'll move it to this desktop.nameis omitted, then we'll make a new window.quakeModewas also updated to usetoCurrentbehavior by default.References
PR Checklist
Detailed Description of the Pull Request / Additional comments
S/O to https://github.com/microsoft/PowerToys, who graciously let us use
VirtualDesktopUtilsfor figuring out what desktop is the current desktop. Yea, that's all we needed that entire file for. No, there isn't an API for this (surprised-pikachu.png)Validation Steps Performed
Played with this for a while, and it's amazing.