Add keyboard navigation to the stacked-window switcher - #1808
Conversation
c900192 to
3e94718
Compare
Indeed we'll want to take this a different route. There are two options:
I'm fine with either approach, and both have their own pros & cons. |
d251b39 to
5e506d8
Compare
|
I was thinking about this a little more and figured I'd elaborate a bit. I think it's ok to remove the badge number entirely, and only show the list of window titles that can be selected. This lends itself to the NSMenu approach quite well. The NSPanel is certainly the better choice for the user experience, but it's also a lot more involved to get right. My initial thought is perhaps simply go with the NSMenu for the first cut, since that should be pretty easy and can be converted over to the NSPanel later. In the meantime, I'm going to go ahead and push out a release and will roll this change into the next one once it's ready. |
|
Happy to go whichever way you prefer. One data point before you decide: I had a go at the NSPanel route and it's further along than you might expect - The one thing that pushed me toward the panel: an NSMenu closes on selection, so you lose being able to walk a stack - down, Return, look, down, Return - which turns out to be the thing I use this for most. With the panel the list stays up and keeps the keyboard. If you'd still rather have NSMenu as the first cut I'll do that instead, no argument. And no rush on my side given you're mid-release. On dropping the count: I can see the case, though the badge is what tells you a stack is there in the first place. Would you want the list on hover with no indicator at all beforehand? |
|
Oh, nice! Let's continue with the NSPanel route for sure. In my testing, the badge and the list are shown at the same time on hover, and that's how I ended up thinking down that path. Is the intent to show the badge even when not hovering, or is it to first show the badge, then the list? |
|
Provided we're not going the NSMenu route, I'm actually good with however you'd like to take it - I was just looking to simplify in case there were road bumps going with the NSPanel. |
|
Good question, and I think you spotted something. Today both appear together on dwell, which does make the count redundant - if you can see three rows, a pill saying 3 is not earning its place. That is the version you were testing, and it is a fair reason to wonder why the badge is there at all. The intent is the two-stage one: dwell on a stack corner shows just the badge, and the list opens when you move onto it. That makes the badge the affordance you act on rather than a label duplicating the list, and passing your cursor over a stack on the way to somewhere else costs you a small pill instead of a full popup. There is a second reason that only applies now that we are on the panel: showing the list means taking key status, since that is how it gets the arrow keys. If the list opens on dwell alone, resting the cursor near a stack corner quietly takes the keyboard from whatever you were typing in. Gating it behind a deliberate move onto the badge means we only take the keyboard when someone has actually opted in. I will implement that and update the PR. If you would rather it stayed one stage, say so and I will drop it back - it is a small change either way. |
|
I like the two stage concept. 👍 |
Arrow keys move the highlight, Return raises the selected window and leaves the list up so the stack can be walked, Escape closes. Rectangle is an accessory app, so its panels can never become key and ordinary focus cannot deliver these keys - the list uses a consuming event tap instead, scoped as tightly as it can be. Only bare navigation keys are claimed: any held modifier goes to the app underneath, the tap is not installed at all while VoiceOver is running, since Quick Nav owns the arrow keys, and an untouched list expires after five seconds so it can never sit holding them. macOS reports every arrow keystroke as carrying .function and .numericPad. Those are properties of the key rather than modifiers being held, so only the four keys a user can actually hold are treated as a reason to pass a keystroke through. The badge also stops counting stacks that are not there: a window covering the screen joins a stack the tiled windows already form, but never forms one on its own. Rows that would fall off the bottom of the screen are not built, since arrow keys could otherwise select - and Return raise - a window with no visible row. The toggle shortcut follows the app's existing shortcut conventions, and the view code moves into StackBadgeWindow, StackBadgeListPanel and StackBadgeRowView, one view per file.
The list is a non-activating panel, so it can become key and receive arrow keys directly once canBecomeKey allows it - the earlier attempt failed because a borderless window refuses key status by default and the panel was only ordered front, never made key. That removes the consuming CGEvent tap and everything that existed to make a global key grab safe: the claim state, the VoiceOver guard, and the five-second expiry. Any key that is not navigation now dismisses the list rather than being eaten. Raising a window activates its app, which takes key status away, so the panel takes it back afterwards. Without that the list stayed on screen while the arrow keys drove the window that had just come forward. The badge and the list are also no longer shown together. A dwell shows the badge; the list opens when the cursor reaches it. Passing over a stack on the way somewhere else now costs a small pill rather than a popup, and since opening the list means taking the keyboard, that only happens once someone has aimed at the badge on purpose.
It was the only row in the popover not sharing the shortcut column, so its recorder sat out to the right of every other one.
A stack formed by screen-covering windows dropped any tiled window at the same corner, so two maximized terminals over a half-width browser listed the terminals and silently omitted the browser. That is the worst window to lose: the covering windows are precisely what makes it invisible, and the badge still showed a plausible count, so there was nothing to suggest a window was missing.
Activating an app is asynchronous, so taking key status back immediately after the call raced the activation and lost. The tick reclaims it whenever the list is visible without it, which does not depend on knowing when activation finishes.
5e506d8 to
1938d61
Compare
|
Updated this to the NSPanel route - the event tap is gone and the description is rewritten to match. One more thing, from using it rather than testing it. I've been running the two-stage for three days. It looks great - the badge is genuinely nice UI and the reveal reads well - but it adds a beat. The travel to the badge costs a moment before you can act, and that's most noticeable exactly when you already knew the stack was there, which is most of the time. Your first instinct - drop the count and just show the list - is where I've ended up too, coming from the other direction. The keyboard argument I made for the two stages still holds, but it doesn't need the badge to carry it: the list can open on dwell and only take key status when the cursor actually enters it. Same "only take the keyboard on deliberate intent" property, one less hop. Not asking you to relitigate the 👍 - the PR does what you endorsed and I'm happy to leave it there. But you were right that the count was redundant and I argued against it, so it seemed worth saying before it ships rather than after. |
A tiled window under a stack of maximized ones was still being dropped. The maximized windows carry the overlap offset, so they sit a cascade step forward of the tiled window they hide, and proximity was being measured from the offset window back to the tiled one - a direction the test deliberately barely allows, since it exists to follow a cascade forward. The earlier test for this placed every window on one origin, where the direction cannot matter, so it passed against code that could not work on any layout where the offset had actually moved something.
The badge briefly excluded screen-covering windows from forming a stack. That exclusion belongs to the overlap offset, where a maximized window shares its origin with every placement and would otherwise shift them all (rxhanson#1766). The badge moves nothing, so it never needed the rule, and with it a maximized window sitting on a half-screen window showed no badge at all - which is the one case where the covered window cannot be seen any other way and the badge is the only thing that would reveal it. The offset keeps its own copy of the check, so rxhanson#1766 is unaffected.
Follow-on to the hover badge from #1795, rewritten around your suggestion. Dwell on a stack corner shows the badge; moving onto the badge opens the list; arrows move the highlight, Return raises the selected window and leaves the list up so you can walk the stack, Escape closes.
The event tap is gone.
.nonactivatingPanelplus acanBecomeKeyoverride was the whole trick, exactly as you said. The reason my earlier attempt failed is that a borderless window returnsfalsefromcanBecomeKeyunless you override it, and I was only ordering the panel front, never making it key - so I concluded panels couldn't take keys in an accessory app and wrote that into this PR as fact. That was wrong, and the correction is entirely yours.Removing the tap took all of its safety machinery with it: the claim state, the modifier filtering that existed to avoid stealing keystrokes, the VoiceOver guard, and the five-second idle expiry. Rectangle no longer consumes any key globally. The CJK/IME limitation I disclosed in the earlier version of this description is also gone, since we no longer intercept anything system-wide.
Net effect on the diff is a deletion - roughly 60 lines less than the tap version, and a lot less to reason about.
Two things worth calling out because they were not obvious:
Also here: rows that would fall off the bottom of the screen are not built, since arrows could otherwise select a window with no visible row; a recordable shortcut to toggle the badge, following the existing shortcut conventions; and the view code split into
StackBadgeWindow,StackBadgeListPanelandStackBadgeRowView, one view per file.Rebased on current main. Full suite passes - 249 tests, no failures.