-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spec for Quake Mode #9274
Spec for Quake Mode #9274
Conversation
…bar and more history/context
… college? I would have killed for that power
This comment has been minimized.
This comment has been minimized.
This would be the feature of the year to me. 🎉 I’ve been rocking with Quake mode since I was in college and using Linux Mint. |
- **TODO: FOR DISCUSSION**: Should this just be `any` to match the above? | ||
Read the rest of the doc and come back. | ||
- `"toCurrent"`: Move the window to the current virtual desktop | ||
- `"onCurrent"`: Only summon the window if it's already on the current virtual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gotta say "toCurrent" and "onCurrent" looks really alike. Perhaps "floating" & "fixed"? Or something like that. Same for the monitor setting.
Windows > Window 1 - <un-named window> | ||
Window 2 - "This-window-does-have-a-name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I like that this gives you a way to see the names of the windows.
* **Story E** Variable dropdown speed | ||
* **Story F** Minimize to tray, press a hotkey to activate the terminal window | ||
(#5727) | ||
* **Story G** Terminal doesn't appear in alt+tab view, press a hotkey to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Story G" doesn't get addressed by this spec, I think. This is also specifically how iTerm2 works and I feel like that's the mode that speaks most to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that "minimize to tray"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G could be separate from F. It just feels like a next step to it where you could add a setting to remove it from alt+tab, possibly regardless of whether it's single mode or not, or only if minimize to tray is enabled.
Alrighty @microsoft/windows-console-team, updated this with the results of the discussion Friday. |
This comment has been minimized.
This comment has been minimized.
Many other implementations of this in Windows always have an issue with z-index placements. Sometimes the terminal slides under a pinned window or is inconsistent with apps in full screen mode. Also when a window is checked "Show this on all desktops" mode in virtual workspaces mode, most tiling implementations fall under it. I hope this is being considered as well. Im not familiar enough with the process to add storyboards the the readme. |
i'm really excited for this, this is a killer app for me |
One thing that's missing (or did I just not find it in the spec?) is automatically hiding a quake mode window when it loses focus which is something many implementations (for example ConEmu) offer. Of course this should be a configuration option. |
That's a great idea, and I'll definitely throw something in the spec for that. Thanks! |
This comment has been minimized.
This comment has been minimized.
Misspellings found, please review:
To accept these changes, run the following commands from this repository on this branch
✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉 🗜️ If you see a bunch of garbage and it relates to a binary-ish string, please add a file path to the File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somewhat feel like this spec tries to do too much. I'd personally prefer having a simpler spec initially and refining this feature later on. For instance I totally understand the need for the distinction between monitors and desktops, but I'm not sure how many people actually need that...
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
## Summary of the Pull Request This PR adds some special behavior to the window named "\_quake". * When creating the quake window, it ignores "initialRows" and "initialCols" and opens on the top half of the monitor. - It uses `initialPosition` to determine which monitor this is * It cannot be moved * It can only be vertically resized on the bottom border. * It's always in focus mode. - We should probably have an issue tracking "Allow showing tabs in focus mode"? Maybe? - This one element is maybe the one I'm least attached to When renaming a window to "\_quake", it adopts all those behaviors as well. It does not exit focus mode when leaving QM, nor does it resize back. That seemed unnecessary. ## References * As spec'ed in #9274 * See also #8888 ## PR Checklist * [x] In the pursuit of #653 * [x] I work here * [ ] Tests added/passed * [ ] Requires documentation to be updated, but I'm not gonna do any of that till quake mode is totally done. ## Detailed Description of the Pull Request / Additional comments Note that this doesn't do things like: * dropdown * global hotkey summon * summon to the current monitor * summon to the current desktop I'm doing #653 _very_ piecemeal, to try and make the PRs less egregious. ## Validation Steps Performed * validated that center on launch still works * validated that QM works on different monitors based on `initialPosition` * validated entering/exiting QM behaves as expected ## TODO! * [ ] When snapping the quake window between desktops with <kbd>win+shift+arrow</kbd>, the window doesn't horizontally re-size to the new monitor dimensions. It should.
## Summary of the Pull Request We don't want it acting as the "most recent window" for windowing behavior. The most recent window should always be some other window. This is being made as an atomic commit because we're probably 50% sure on this one. Maybe people do want new tabs to open up in the quake window! If they're running from the commandline, that's easy. If they're running from the shell context menu, that's **H**ard / impossible currently. $20 someone asks for that if we ship this. That of course might just fall into "explorer context menu settings" though. ## 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-59030791 * [x] I work here * [x] Tests added * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I mean, this one's super straightforward, not sure what else there is to add. ## Validation Steps Performed Played with this, it works exactly as you'd think.
Adds support for two new actions: * `globalSummon`, which can be used to activate a window using a _global_ (READ: OS-level) hotkey. - accepts an optional `name` argument. When provided, this will attempt to summon with the given name. When omitted, we'll try to summon the most recent window. * `quakeMode` which is `globalSummon` for the `_quake` window. These actions are stored in the actions array, but are read by the `WindowsTerminal` level and bound to the OS in `IslandWindow`. The monarch registers for these keybindings with the OS. When one is pressed, the monarch will recieve a `WM_HOTKEY` message. It'll use that to look up the corresponding action args. It'll use those to try and summon the right window. ## References * #8888: Quake mode megathread * #9274: Spec (**guys seriously i just need one more ✔️**) * #9785: The start of granting "\_quake" super powers ## PR Checklist * [x] Closes #653 - I'm gonna say this closes it for now, though we have _many_ follow-ups in #8888 * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * Validated that it works with `win` keys * Validated that it works without `win` keys * Validated that it hot-reloads * Validated that it moves to the new monarch * Validated that you can bind both `globalSummon` and `quakeMode` at the same time and do different things * Validated that you can bind `globalSummon` with a name and it creates that name if it doesn't already exist
This adds support for the `desktop` param to the `globalSummon` action. It accepts 3 values: * `toCurrent` (default): The window moves to the current desktop when it's summoned * `any`: 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. * When combined with `name`, if there's a window matching `name`, we'll move it to this desktop. * If there's not a window on this desktop, and `name` is omitted, then we'll make a new window. `quakeMode` was also updated to use `toCurrent` behavior by default. ## References * Original thread: #653 * Spec: #9274 * megathread: #8888 ## PR Checklist * [x] Checks some boxes in #8888 * [x] closes https://github.com/microsoft/terminal/projects/5#card-59030845 * [x] I work here * [x] Tests added * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments S/O to https://github.com/microsoft/PowerToys, who graciously let us use `VirtualDesktopUtils` for 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.
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.
## Summary of the Pull Request Adds the `dropdownDuration` property to `globalSummon`. This controls how fast the window appears on the screen when summoned from minimized. It similarly controls the speed for sliding out of view when the window is dismissed with `"toggleVisibility": true`. `dropdownDuration` specifies the duration in **milliseconds**. This defaults to `0` for `globalSummon`, and defaults to `200` for `quakeMode`. 200 was picked because, according to [`AnimateWindow`](https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-animatewindow): > Typically, an animation takes 200 milliseconds to play. Do note that you won't be able to interact with the window during the animation! Input sent during the dropdown will arrive at the end of the animation, but input sent during the slide-up _won't_. Avoid setting this to large values! The gifs are in Teams. ## References * Original thread: #653 * Spec: #9274 * megathread: #8888 ## PR Checklist * [x] Closes https://github.com/microsoft/terminal/projects/5#card-59030824 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I had the following previously in the doc comments, but it feels better in the PR body: - This was chosen because it was easier to implement and generally nicer than: * `AnimateWindow`, which would show the window borders for the duration of the animation, and occasionally just plain not work. Additionally, for `AnimateWindow` to work, the window much not be visible, so we'd need to first restore the window, then hide it, then animate it. That would flash the taskbar. * `SetWindowRgn` on the root HWND, which caused the xaml content to shift to the left, and caused a black bar to be drawn on the right of the window. Presumably, `SetWindowRgn` and `DwmExtendFrameIntoClientArea` did not play well with each other. * `SetWindowPos(..., SWP_NOSENDCHANGING)`, which worked the absolute best for longer animations, and is the closest to the actual implementation of `AnimateWindow`. This would resize the ROOT window, without sending resizes to the XAML island, allowing the content to _not_ reflow. but for a duration of 200ms, would only ever display ~2 frames. That's basically not even animation anymore, it's now just an "appear". Since that's how long the default animation is, if felt silly to have it basically not work by default. - If a future reader would like to implement this better, **they should feel free to**, and not mistake my notes here as expertise. These are research notes into the dark and terrible land that is Win32 programming. I'm no expert. ## Validation Steps Performed This is the blob of json I'm testing with these days: ```jsonc { "keys": "ctrl+`", "command": { "action": "quakeMode" } }, { "keys": "ctrl+1", "command": { "action": "globalSummon" } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } }, { "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } }, { "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } }, { "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } }, ``` * <kbd>ctrl+\`</kbd> will summon the quake window with a _quick_ animation * <kbd>ctrl+2</kbd> will summon the window with a s l o w animation
…10092) ####⚠️ this pr targets #9977 ## Summary of the Pull Request This adds support for part of the `monitor` property for `globalSummon`. It also goes a little off-spec: ```json "monitor": "any"|"toCurrent"|"toMouse" ``` * `monitor`: This controls the monitor that the window will be summoned from/to - `"any"`: Summon the MRU window, regardless of which monitor it's currently on. - `"toCurrent"`/omitted: (_default_): Summon the MRU window **TO** the monitor with the current **foreground** window. - [**NEW**] `"toMouse"`: Summon the MRU window **TO** the monitor where the **mouse** cursor is. When I was playing with this, It felt like `toMouse` was always what I wanted, not `toCurrent`. We can always just comment that out if we think that's contentious - I'm aware I didn't originally spec that. ## References * Original thread: #653 * Spec: #9274 * megathread: #8888 ## PR Checklist * [x] Closes https://github.com/microsoft/terminal/projects/5#card-60325291 * [x] I work here * [ ] Tests added/passed * [ ] Requires documentation to be updated 😢 ## Detailed Description of the Pull Request / Additional comments I made `toMouse` the default because it felt better. fite-me.jpg ## Validation Steps Performed my ever evolving blob: ```jsonc { "keys": "ctrl+`", "command": { "action": "quakeMode" } }, { "keys": "ctrl+1", "command": { "action": "globalSummon" } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } }, { "keys": "ctrl+2", "command": { "action": "globalSummon", "monitor": "any" } }, // { "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } }, { "keys": "ctrl+3", "command": { "action": "globalSummon", "monitor": "toMouse" } }, // { "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } }, { "keys": "ctrl+4", "command": { "action": "globalSummon", "monitor": "toMouse", "dropdownDuration": 500 } }, { "keys": "ctrl+5", "command": { "action": "globalSummon", "dropdownDuration": 500 } }, ```
## Summary of the Pull Request This is a scoped implementation of "hide on minimize", only to the `_quake` window. When minimized, the `_quake` window won't appear in the taskbar. IT ALSO WON'T APPEAR IN THE TRAY, BECAUSE WE DON'T HAVE ONE YET. I talked about this with @DHowett, and it seemed cool. Other windows will still minimize normally. ## References * Original thread: #653 * Spec: #9274 * megathread: #8888 * minimize to tray: #5727 ## PR Checklist * [x] Closes https://github.com/microsoft/terminal/projects/5#card-61246940 * [x] I work here * [ ] Tests added/passed * [ ] Requires documentation to be updated - probably yea, but something <sub>something <sub>something</sub></sub> ## Detailed Description of the Pull Request / Additional comments After playing with it, it is in fact, cool. ALSO `LOG_IF_WIN32_BOOL_FALSE` should DEFINITELY not be used with `ShowWindow`. [`ShowWindow`](https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-showwindow#return-value) returns false if the window was previously hidden, but doesn't `SetLastError`, so that macro will _throw_. ## Validation Steps Performed ```jsonc { "keys": "ctrl+`", "command": { "action": "quakeMode" } }, { "keys": "ctrl+1", "command": { "action": "globalSummon", "name": "_quake" } }, // { "keys": "ctrl+1", "command": { "action": "globalSummon" } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } }, // { "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } }, { "keys": "ctrl+2", "command": { "action": "globalSummon", "monitor": "any" } }, // { "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } }, { "keys": "ctrl+3", "command": { "action": "globalSummon", "monitor": "toMouse" } }, // { "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } }, { "keys": "ctrl+4", "command": { "action": "globalSummon", "monitor": "toMouse", "dropdownDuration": 500 } }, { "keys": "ctrl+5", "command": { "action": "globalSummon", "dropdownDuration": 500 } }, ```
⇒ doc link ⇐
Summary of the Pull Request
After reading through 114+ comments in #653 and related issues, I think I've finally wrapped my head around all the possible scenarios for quake mode. This also includes "minimize to tray", because the two are a powerful combination. With the work already prototyped in
dev/migrie/f/653-QUAKE-MODE
, I'm starting to believe that we could actually land this in 2.0.Abstract
PR Checklist
Detailed Description of the Pull Request / Additional comments
*** read the spec ***