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. 👍 |
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. |
|
Going down this line of thought, I think the badge is worthwhile if it's an always-on kind of UI. Managing location and edge cases, like what happens when windows are moved or cover existing windows certainly make this a less desirable path. I think going with the single stage of the list appearing is the best route for a first cut. It's nice that the offset feature can be used as the visual indicator instead of a badge as well. |
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.
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.
The badge was a holdover from before the list existed, when a count was all this feature offered. Once the list arrived the badge was labelling something you were about to read anyway, and making it a first stage put a step between wanting the list and getting it. The overlap offset already signals that a stack is there - the covered windows peek out from under the top one - so nothing is lost by removing the pill. Opening the list means taking key status, which is how it receives the arrow keys, so the list appears on dwell but does not take the keyboard until the cursor is on it. Resting near a stack corner while typing therefore costs nothing. Once a window has been raised from the list the keyboard is taken back without that test, since raising activates the window's app and walking a stack lets the pointer drift off the rows - but only then, so switching apps by any other means is left alone. The corridor between the dwell point and the list follows the cursor rather than the geometric corner, since a gap puts the stack down and right of it and the first move toward the list would otherwise read as leaving.
eaaaab3 to
5ab4d9c
Compare
|
Updated: single stage, badge removed. Agreed on both counts, and the offset-as-indicator point is a good one - I had not thought of it that way, but it is exactly what tells you the stack is there. One implementation note worth flagging since it shaped the design. Opening the list means taking key status, because that is how it receives the arrow keys. So the list appears on dwell but does not take the keyboard until the cursor is actually on it - dwelling near a corner while typing costs nothing. The property I was protecting with the two stages survives, it just does not need the badge to carry it. Heads up that the suite currently shows 7 failures in HalfSplitCornerCalculationTests. They are not from this branch - the same 7 fail on main by itself, and they look related to b77e5ca. |
|
Nice! I'll test it out tomorrow! |
|
I like it :) I went back and forth on the idea of simply stealing keystrokes up front, and I think the reason is because on one hand I want this to simply come and go, and on the other hand I can see it being a UI that sits there so you can toggle back and forth as desired. A reason that I might lean toward stealing keystrokes is so you can close it with the escape key. One possible way to work around this is to have the stacked window offset leave space for a horizontal "tab bar" of sorts, and it could be always displayed. Maybe there should be two modes, one that is as it is currently but stealing the keyboard focus immediately as a quick menu that goes away, and one that is the tab bar style that's always on. I was thinking that a keyboard shortcut to show the menu might be nice to avoid having to leave the keyboard. Just thoughts; it's a joy to use as-is. I really need to rework some of the UI, but as things currently stand, it would be nice to have the description for the shortcut recorder present, since it isn't immediately clear that the shortcut will toggle it. Not a big deal since I think I'll be moving this out of the Extras button and into the General Settings soon and can do it at that time if needed. |
Follow-on to the hover badge from #1795, rewritten around your two suggestions: the NSPanel route, and the single stage.
Hovering a stack corner opens the window list directly. Arrows move the highlight, Return raises the selected window and leaves the list up so you can walk the stack, Escape closes, and moving away dismisses.
The count badge is gone. You were right that it was doing little - it labelled something you were about to read anyway, and as a first stage it put a step between wanting the list and getting it. I ran it that way for three days and the beat was noticeable. And as you said, the offset already signals the stack: the covered windows peek out from under the top one, so nothing is lost by dropping the pill.
The event tap is gone too.
.nonactivatingPanelplus acanBecomeKeyoverride was the whole trick, exactly as you said. My earlier attempt failed because a borderless window returnsfalsefromcanBecomeKeyunless overridden, and I was only ordering the panel front rather than making it key - so I concluded panels couldn't take keys in an accessory app and wrote that into this description as fact. That was wrong, and the correction was yours.Removing the tap took all its safety machinery with it: the claim state, the modifier filtering, the VoiceOver guard, and the idle expiry. Rectangle no longer consumes any key globally, and the CJK/IME caveat I disclosed earlier is gone with it.
Three things that were not obvious, in case they save you time reading:
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 feature, following the existing shortcut conventions and round-tripping through config export/import; and the view code split into
StackBadgeListPanelandStackBadgeRowView, one view per file.Rebased on v1.100. The suite is 241 passing, 7 failing - the same 7
HalfSplitCornerCalculationTestsfailures that are on main by itself, so nothing here contributes to them.