feat: Simplify workspace navigation flow #126
Conversation
Specifically, this change: 1. Removes using enter/escape as indicator keys for enabling and disabling keyboard navigation. 2. Makes it such that focus alone is enough to enable navigation keys. 3. Permanently enables keyboard navigation on page load (as was done before), but instead changes the conditions for navigation keys. 4. Performs some other minor cleanups in navigation_controller.ts.
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
Self-reviewed.
|
PTAL @gonfunko or reroute if needed. FYI @cpcallen and @rachel-fenichel. |
|
I'll address the suggested changes & submit this early next week (I want to make sure I actually retest it since there isn't any CI to verify that I don't break the plugin if I try making changes on GitHub directly (: ). |
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
Self-reviewed changes, and also tested them locally (though I didn't test the shortcuts exhaustively). Things should be in good order, though.
|
Thanks @cpcallen! Going to go ahead and merge this since it's a significant navigation change that would be good to get in front of users for the next testing round. Please note that I resolved your comment threads--if you have any follow-up suggestions please let me know and I'll be happy to address them in a follow-up PR. |
3 participants
Fixes part of #101
Fixes #89
This change removes the need for using enter and escape to enable keyboard navigation when the workspace is focused.
Instead, simply focusing the workspace is sufficient to allow keyboard navigation. Note that this PR goes back to the previous behavior of always enabling keyboard navigation on page load and instead limits the preconditions of navigation keys to check focus rather than feature enabled state. This allows for #89 to be fully mitigated. Since keyboard navigation is never disabled, the cursor position is never lost (and thus doesn't need to be cached). The removal of enter/escape fixes the first part of #101.
This PR also performs some other minor cleanup work in
navigation_controller.ts.