-
Notifications
You must be signed in to change notification settings - Fork 316
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
nav-* properties and arrow key navigation. #519
Conversation
7b07a60
to
e463ab5
Compare
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 a lot for the pull request. This is a very welcome addition, and a long desired feature. First of all I find the approach and code very reasonable, and easy to follow. Good job! I believe these additions cover many common use cases. Even though it covers only a part of what we've discussed in #142, this should be a good foundation that we can later expand upon, such as adding navigation groups.
I have included quite a few comments. Most of them are either minor, or they can be skipped for now and dealt with later. I hope they are not too overwhelming. And please do let me know if you don't agree with some suggestions, or believe it is too much effort. I also have some more general feedback in the following.
While navigating, the drop down box iterates through all the options before it eventually navigates to the next element. This is not how I expect it to behave under keyboard navigation. I believe we have somewhat conflicting behaviors here:
- On the one hand, normal tabbed web browsing behavior: This works like today in RmlUi, if we tab to the drop down, we can use arrow keys to change options.
- Under keyboard/controller navigation: We navigate both to and from the element using the arrow keys (D-pad). To change values, we first have to press Enter/A to display the drop down. (Just tested this in Rocket League, and this is indeed how it works there).
Possible solution: Always navigate away if the element has any nav-...
properties set (do not iterate through options). Drawback is that this behavioral change might be unexpected when enabling navigation, and tabbing behavior no longer acts the same. Is this reasonable, or are there other ideas?
Another similar issue comes from text boxes. With keyboard navigation, the current behavior (moving to the next box when at the end of text) seems reasonable. However, with a controller in particular, we might want to navigate between elements, and have to press 'A' before being able to navigate within the text. It's okay to leave this for now, but any ideas for the future would be great to hear.
Previously we always stopped propagation of key events when a text field is focused. This is no longer the case. Consider the case where a game moves a top-down camera with arrow keys. However, if a text box is focused, then it should always move within this and always prevent camera movement, and not start moving the camera after arriving at the end of the text. Or maybe this case should be handled some other way from the user side.
Some of these might be a reason to consider a separate event for navigation actions. Maybe consider this more in the future when we have more usage experience.
Possible later enhancements (notes to myself or others who want to give it a go):
- Implement :focus-visible, and use that to show where we are navigating in the samples. The following shows examples of when it should be enabled compared to
:focus
:- Mouse click on button: Off.
- Mouse click on text input: On.
- Tabbing to any focusable: On.
- Navigating to any focusable: On.
Context::ProcessMouseLeave(): Always mirror :focus.
- Invaders sample. Good candidate sample for making fully navigable by keyboard.
nav
shorthand?- Box model: e.g. 'auto none'.
- Space key to emulate click.
- Possible auto algorithm improvements?
- Ideally the demo form works reasonably with auto-navigation without manually specifying IDs.
- Idea: Match everything outside the element in the specified direction. Pick the one with the smallest euclidean distance from the self far edge midpoint, to the target near edge midpoint.
- Only consider elements in the same scroll container?
Thanks for the contribution, as discussed, we'll continue this work in #524. |
Four new properties introduced:
The property can have one of 3 values:
"none" is default value. It means that navigation is disabled for the element.
"auto" means that navigation code attempts to locate neighbor element.
"#elementId" mean that navigation should move focus to the provided element.