[docs] Implement keyboard navigation for demo toolbar #20798
Conversation
eps1lon
added
docs
Details of bundle changes.Comparing: cdea078...0a0704c Details of page changes
|
|
@eps1lon Nice, +1 for dogfooding our use case with the documentation, you might want to benchmark with https://reakit.io/docs/toolbar/ for landing a new module in the lab :). |
There was a problem hiding this comment.
Currently I don't feel comfortable using a dynamic list of refs. Getting this right is difficult for concurrent mode.
I guess it's a matter of timing, we can look at it later on.
I was wondering about the API tradeoff and explored two opposite directions:
- What if we were moving one level down in the abstraction: handling more logic outside React lifecycles, at the DOM level? What should we give up on? For instance, we could have the
getControlPropsto apply adata-xattribute that we would query in the DOM, removing the need for an array of refs? - What if we were moving one level up in the abstraction: introducing Toolbar (context provider) / ToolbarItem (context consumer) components to keep track of the mounted DOM nodes and to scope the rerenders? I assume it would require to have a
componentprop, to wrap other components.
I don't know if any of these two directions would be better, I have found them interesting to consider.
But have to query the full document on every keystroke. A time were performance is most critical. How would an element find the next available sibling? Would it have to try each index and the run some heuristics to determine if that element is focusable? How do we know that an element belongs to the same group?
I have but can only imagine this working with (de)registering in useEffect which means we need to re-render the full widget every time a new item (de)registers. |
I would imagine we can listen for DOM changes, seeking for changes on the internal data-x attribute we set. But the API might not be widely supported enough.
I guess with a querySelectorAll on the data-x attribute
If we want to support toolbar nesting, it's more challenging, we might need to set a marker on the root. Something in this order: https://github.com/oliviertassinari/react-swipeable-views/blob/8f3d3a14cd37a870c50deda67b26791d80ff04fd/packages/react-swipeable-views/src/SwipeableViews.js#L130-L131.
I would expect the same. Should we worry about these re-renders? In any case, I'm not saying any of these options are better! |
I don't mean nesting. Just multiple toolbars in the document. We would need to expose any relevant prop for focusability in |
To be specific: We would need to commit again. This can get quite expensive if you're on your initial load and every widget (which is used for user input) needs to commit twice before it is interactive. I'd rather prematurely optimize user-input than gain...what exactly? |
|
Good implementation @eps1lon :) |
Co-Authored-By: Olivier Tassinari <[email protected]>
|
Let's get this in. Having to manually wire up the controls is definitely not ideal but it works for our use case and doesn't have any drawbacks of the register-descendant pattern. |
Implements keyboard navigation following https://www.w3.org/TR/wai-aria-practices/#toolbar (with optional Home/End keys) for our demo toolbar.
useToolbaris definitely a candidate for the lab and #17281https://material-ui.com/components/selects/:
http://deploy-preview-20798--material-ui.netlify.app/components/selects/
It's not suited for menus or selects since it relies on a static number of elements. Currently I don't feel comfortable using a dynamic list of refs. Getting this right is difficult for concurrent mode.
This work is stacked on top of some refactoring of the DemoComponent:
withStyles->makeStylesDemoLanguages(makesuseToolbarhook easier to use)aria-controlfrom the toolbar and use it granular where appropriate (resetDemo controls rendered demo, codeToggle controls rendered source)