Skip to content

Keyboard menu/widget navigation - #628

Merged
Keavon merged 21 commits into
masterfrom
widget-keyboard-nav
May 25, 2022
Merged

Keyboard menu/widget navigation#628
Keavon merged 21 commits into
masterfrom
widget-keyboard-nav

Conversation

@0HyperCube

@0HyperCube 0HyperCube commented May 7, 2022 •
edited by Keavon
Loading

Copy link
Copy Markdown
Contributor

Closes #575

@0HyperCube 0HyperCube mentioned this pull request May 7, 2022
Closed
6 tasks
@Keavon Keavon linked an issue May 8, 2022 that may be closed by this pull request
Editor GUI keyboard navigation #575
Closed
6 tasks
@Keavon
Keavon marked this pull request as draft May 8, 2022 22:02
@Keavon
Keavon force-pushed the widget-keyboard-nav branch from 7580133 to 2566c1f Compare May 10, 2022 16:49
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 10, 2022 •
edited
Loading

Copy link
Copy Markdown

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8752110
Status: ✅  Deploy successful!
Preview URL: https://94c11dc2.graphite-master.pages.dev

View logs

@Keavon
Keavon force-pushed the master branch 2 times, most recently from c0faee2 to 5a3db9b Compare May 12, 2022 21:22
@0HyperCube
0HyperCube marked this pull request as ready for review May 16, 2022 18:55
@0HyperCube
0HyperCube requested a review from Keavon May 16, 2022 18:55
@Keavon

Keavon commented May 19, 2022

Copy link
Copy Markdown
Member

I'm currently looking at merging #649 first since it'll be easier to deal with conflicts in that order. Still working on investigating a bug in that branch though.

Keavon
Keavon requested changes May 22, 2022
View reviewed changes

@Keavon Keavon left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! My earlier concerns are now resolved.

Comment thread frontend/src/App.vue Outdated
Comment thread frontend/src/components/widgets/WidgetSection.vue Outdated
Comment thread frontend/src/App.vue
Comment thread frontend/src/components/widgets/floating-menus/MenuList.vue
Comment thread frontend/src/components/widgets/inputs/FontInput.vue Outdated
Comment thread frontend/src/components/widgets/separators/Separator.vue Outdated
Comment thread frontend/src/io-managers/input.ts Outdated
Keavon
Keavon reviewed May 22, 2022
View reviewed changes
Comment thread frontend/src/components/widgets/inputs/MenuBarInput.vue
@Keavon

Keavon commented May 23, 2022

Copy link
Copy Markdown
Member

Only one small request: when using the arrow keys to navigate a menu, reaching the end should cap rather than looping back to the start.

Keavon
Keavon reviewed May 23, 2022
View reviewed changes
Comment thread frontend/src/components/widgets/floating-menus/DialogModal.vue Outdated
@0HyperCube

0HyperCube commented May 23, 2022

Copy link
Copy Markdown
Contributor Author

Only one small request: when using the arrow keys to navigate a menu, reaching the end should cap rather than looping back to the start.

The standard behavior in gtk appears to be to have looping in the menubar but not in the dropdowns. I will add this.

Keavon
Keavon reviewed May 24, 2022
View reviewed changes
Comment thread frontend/src/components/floating-menus/DialogModal.vue Outdated
Keavon
Keavon reviewed May 24, 2022
View reviewed changes
Comment thread frontend/src/components/floating-menus/DialogModal.vue Outdated
Keavon
Keavon reviewed May 24, 2022
View reviewed changes
Comment thread editor/src/document/document_message_handler.rs Outdated
Keavon
Keavon approved these changes May 24, 2022
View reviewed changes

@Keavon Keavon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to merge (after the last comment)! Sorry for the repeated nitpicks.

@Keavon

Keavon commented May 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

An issue with the starting active color:

https://files.keavon.com/-/FirstLuminousPrayingmantid/capture_4_.mp4

@Keavon

Keavon commented May 25, 2022 •
edited by 0HyperCube
Loading

Copy link
Copy Markdown
Member

Here's a list of a few more issues I ran into while QA testing. Feel free to fix now or merge now and fix later.

  • I'm noticing one small UX inconsistency: Space or Enter work for Menu Bar and Dropdown inputs, but only Enter works for buttons.
  • Opening Menu Bar groups with Space/Enter doesn't close other open ones so multiple stack up at the same time. And when you've tabbed away from the open menu bar, Escape doesn't exit it either (but that should be impossible to cause once the previous sentence is fixed).
  • Doesn't have to be in this PR, but we should make Escape close the open popover. This works on Dialogs and MenuLists but not Popovers.
  • If you hold down the up/down keys to switch blend modes, it takes a little time to apply because of the backend being slow to update the frontend. But if you reach the end and keep holding the down key, it still (I assume) stacks up change requests and freezes the backend for a long time. We should ideally cap that on the frontend so the keyrepeat of holding down the arrow keys shouldn't freeze the editor longer than the time it would take to freeze for every item in the list before reaching the end, rather than stacking up an indefinite length of freezes by going past the end.

@Keavon Keavon changed the title Keyboard menu navigation Keyboard menu/widget navigation May 25, 2022
@Keavon
Keavon merged commit 2e5ccfe into master May 25, 2022
@Keavon
Keavon deleted the widget-keyboard-nav branch May 25, 2022 21:10
Keavon added a commit that referenced this pull request Jun 16, 2022
@0HyperCube @Keavon
Keyboard menu/widget navigation (#628)
889e988 
* Keyboard menu navigation

* Fix dropdown keyboard navigation

* Fix merge error

* Some code review

* Interactive dropdowns

* Query by data attr not class name

* Add locking behaviour

* Change query selector style

* Change query selector style (again)

* Code review feedback

* Fix highlighted entry regression

* Styling and disabling checkbox tabindex in MenuLists

* Don't redirect space off canvas to backend

* Do not emit update if value same

* Escape closes all floating menus

* Close dropdowns on blur

Co-authored-by: Keavon Chambers <keavon@keavon.com>
Keavon added a commit that referenced this pull request Jul 30, 2023
@0HyperCube @Keavon
Keyboard menu/widget navigation (#628)
81c3420 
* Keyboard menu navigation

* Fix dropdown keyboard navigation

* Fix merge error

* Some code review

* Interactive dropdowns

* Query by data attr not class name

* Add locking behaviour

* Change query selector style

* Change query selector style (again)

* Code review feedback

* Fix highlighted entry regression

* Styling and disabling checkbox tabindex in MenuLists

* Don't redirect space off canvas to backend

* Do not emit update if value same

* Escape closes all floating menus

* Close dropdowns on blur

Co-authored-by: Keavon Chambers <keavon@keavon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Keavon Keavon Keavon approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Editor GUI keyboard navigation

2 participants

@0HyperCube @Keavon