-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add UI keyboard navigation #8882
base: main
Are you sure you want to change the base?
Conversation
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
@ndarilek I'd like your opinion on how this can integrate with AccessKit :) |
You added a new example but didn't add metadata for it. Please update the root Cargo.toml file. |
1 similar comment
You added a new example but didn't add metadata for it. Please update the root Cargo.toml file. |
It looks like bevy would need to send the relevant focused entity with the TreeUpdate event. |
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.
Thanks for getting this started! This will be nice for both accessibility and general UX.
Some initial thoughts:
- We will probably need to sync the "UI focus" with the "Access Kit" focus -- both ways. So a keyboard focus change will need to update the info for the screenreader and the screenreader navigation should also adapt the UI style. @ndarilek should know more details about this.
- We shouldn't hard-code keybindings for this. Especially
Tab
is a common keybinding used in games, so this might collide with other controls. We can keep these values as defaults, but this should be configurable. - Inside menus, games often have more ways to control the UI. For example, using the arrow keys and gamepad input. I'm not sure if we should have these relatively common use cases built-in as well, but at some point we should at least add the possibility for the user to add this. Otherwise, we might get some interactions that don't properly update the focus state and screenreader info if the devs don't know about the details.
These things don't necessarily need to be addressed in this PR, but I think we should investigate how easily they could be integrated in order to not block ourselves in the future.
I have added access kit integration. |
I can't really think of any other keybindings for keyboard navigation (other than arrow keys - but they are seperate), but I agree that we would need some API for stopping the tab key from modifying the UI if it has modified the game. How would you suggest allowing this to be configured @TimJentzsch?
I agree that arrow key navigation is very important for lists or for a row of buttons in a dialogue menu. In order to handle arrow key navigation, the game would have to declare a node as being a navigable row or column. Do you have any thoughts on what the API could look like? |
Maybe a
This is probably hard to solve for the general case... although it would be nice if we could somehow provide this functionality if needed. We also have to keep internationalization in mind -- in the future, the locale could affect how the UI is rendered, e.g. in some languages the text is layed out vertical so menus might be layed out horizontally (automatically via flexbox) and the other arrow keys would make more sense for navigation. |
You added a new example but didn't update the readme. Please run |
Can confirm it's hard to solve for the general case. This is what https://lib.rs/crates/bevy-ui-navigation does. And there even was An RFC (RFC41) which I had to close due to feature creep on my side. |
I didn't realise that navigation could be so complex! I'm sorry that I duplicated some of your code; I managed to completely miss it when searching for keyboard navigation. But perhaps some very basic built in tab navigation is still a useful stopgap measure until the more complete implementation is added? |
I'm taking a look at your PR and I think it solves a few of the problems in my impl! 😅 Given the time I spent on this, I'm a bit ashamed… I'll take a more careful look at the code tomorrow, but it seems like a good start. and not victim of the feature creep bevy-ui-navigation has. |
Yeah, my view is that now that we've taken a look at the problem space more broadly we're well equipped to get an incremental start in and build from there :) |
It's been a while since I did the AccessKit work but I believe I added a |
They are :) The element focused should always be the same, the only difference is how you navigate between focused elements. |
i dont know how feasable this actually is, but the way i would solve this is ordering the buttons by the global transform, larger global transform goes at the end of the list and as you press tab it slowly goes through the list of buttons, top too bottom, left too right, as far as im aware most ui goes this way, if buttons are on the same Y plane then the X plane is used for ordering, buttons shouldnt be on top of each other yes? would this work for 3d menus? my brain says it would but im not sure |
Might be worth looking at Godot's APIs/handling of this. I believe, and it's been a while since I've used Godot, that it has both an auto-placement API as well as a way to explicitly tag up/down/left/right/next/previous focus from any given widget. So if the auto layout algorithm does something that makes no sense, you can explicitly tag widgets to receive focus when d-pad/tab/shift-tab controls are used on a widget-by-widget basis. That's what I'd suggest here. An auto-layout algorithm will probably get it right some large amount of time, but there will always be corner cases it misses or handles poorly. |
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.
This needs some work. Notably to avoid iterating over all entities.
I would suggest also taking a glance at RFC 41, the discussion on the RFC and the ui-navigation API. There is a lot of edge cases involved, and it would be a shame to miss the ones we already uncovered.
I'd be happy to open a pr on 0HyperCube/bevy. I can cook up something tomorrow. Please tell me if you'd like that.
@ndarilek This PR currently uses the |
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.
If it's not too much effort, I think this would be a good candidate for automated tests.
Maybe one that simulates a Tab
press and then asserts that the focused entity is the next element and one that simulates Shift
+Tab
and asserts the previous element.
Otherwise, this looks good 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.
Nicely done!
@0HyperCube Looks like |
Could the existing component |
not a fan of mixing keyboard and mouse navigation / focus / interaction with the same components, this often have surprising side effects. In the example, if I click on one of the button, then move my mouse to another and press space, the first button gets clicked with no indication it's going to be the one selected |
This is the standard behaviour on the web @mockersf; although I'm open to suggestions to improve this behaviour. |
We could potentially do something like: struct Interactable {
focus_state: FocusState,
hovered: bool,
clicked: bool,
} However, note that not all interactable UI nodes should be keyboard focusable. In HTML this can be done with |
Any chance that this could make it to |
I would love to see this get added |
Trying to use some of the design from this in the lastest stable bevy 0.14.2, and it would seem that setting Focus() to some UI node (such as a text box UI element, or button) is causing some sort of race condition with the a11y accesskit node tree (Focused node id not in node list panic) myapp::ui::navigation: setting focus state to Focusable { focus_state: Focused { visible: true } } for ent Some(Entity { index: 4, generation: 1 })
thread 'main' panicked at C:\Users\conno.cargo\registry\src\index.crates.io-6f17d22bba15001f\accesskit_consumer-0.22.0\src\tree.rs:36:13:
Focused id #4294967300 is not in the node list |
Do you have a minimal repro? Hoping to do an accesskit improvement pass this cycle and would like to address this. Thanks. |
Objective
Add some very simple keyboard navigation to
bevy_ui
, based on the order that the entities appear within the hierarchy.Solution
The
Focus
resource (from thebevy_a11y
crate) can be changed with tab, shift+tab or by clicking on an entity. Entities that are visible and have the newFocusable
component can be navigated to. To aid styling, a newFocusable
component hasis_focused
andis_focus_visible
functions. Focus is visible when keyboard (or in future gamepad) navigation is used.Changelog
Added
FocusState
component containing the current focus state of the entity to aid with styling.Changed
Focus
resource now also has afocus_visible
property reflecting the:focus-visible
css pseudo-class.