Open ryancogswell opened 5 years ago
@ryancogswell Absolutely fantastic write-up! Thanks for taking the time to put it together.
It's related to #6955.
Absolutely fantastic write-up! Thanks for taking the time to put it together.
@mbrookes Thanks! It was rather time-consuming for me, largely due to continually finding that I needed to do more reading. I was pretty ignorant about a lot of these accessibility resources prior to working on Material-UI.
Now to get into some of the implementation details...
I think the heuristic currently in MenuList
is probably still sufficient: elements with an explicit tabindex attribute that are not disabled. Any elements that are inherently focusable will need to have an explicit tabindex of -1 added to them to remove them from the tab sequence, so that should then make them identifiable via this heuristic. The main difference to the current MenuList
implementation would be to no longer limit this to the immediate children of the List
.
This is the messiest part of the solution. We can probably leverage Context in some fashion to detect when we should add the tabIndex={-1}
to the main Material-UI elements we expect to occur within lists (e.g. ButtonBase
and Checkbox
), but it is difficult to know what all elements will be used within lists. We can document the need to add tabIndex={-1}
to any focusable elements for which the library doesn't take care of this (which would include any non-Material-UI focusable element), but it will be a rather error-prone area for less-standard lists. I'm interested in hearing any different ideas about how to manage this aspect.
Given the messiness involved in reliably removing focusable elements from the tab sequence, I don't think this should be default behavior. If some elements within list items remain part of the tab sequence while some are removed, the result would be worse than the current behavior where everything is in the tab sequence. Perhaps we could consider making it the default in v5 after we have learned some lessons and addressed issues.
@ryancogswell The more I think about this composite widget, the more I realize it's a hard problem! You know how to pick your fights! You might want to review the implementation tradeoffs of Rover by @diegohaz. I'm wondering if the React hasn't started an effort on it. I know they have an upcoming focus scope component, to verify.
I'm excited about the implication for our Tab component, the upcoming Tree view and the idea we can share the same logic with the Menu and potentially the different autocomplete components. This demo looks pretty messy to get right: https://www.w3.org/TR/wai-aria-practices/examples/treeview/treeview-1/treeview-1a.html. There is a dimensionality issue to handle at the same time (up / down / right / left). The logic might be even more complex for supporting nested menu where the nodes are portal, so not under the same host node.
1.
I'm happy to reuse the same method. It has yet to be proven suboptimal.
2.
Yes, it's a hard problem! Did you consider doing it without involving React? Would the following logic works:
tabIndex={-1}
to all the elements we want to include in the composite widget.autoFocus
prop to programmatically change the active item.Now, I believe we can't use such logic to build a combobox (should we even consider it?) as the focus needs to stay on the input. The keyboard interaction and selected states are simulated. I imagine it's not that much an issue, we could create an abstraction for the core logic and have an adapter that either interact with the DOM or rerender in a Reacy lifecycle.
3.
I would prefer to have it opt-in. It's not obvious to me when a list should be considered composite or when it shouldn't. In doubt, I would keep the default browser behavior.
Did you consider doing it without involving React?
@oliviertassinari Yes, I have in mind something similar to what you laid out. tabIndex={-1}
would be managed in React, but the actual focus navigation and the roving tabindex would just happen in the DOM (probably with the help of a ref or two). I'm picturing leveraging listItemElement.querySelectorAll('[tabindex]')
to find the nodes in the DOM to include in the navigation.
I'm not sure how soon I'll actually work on this. Dropdowns are the higher priority for me (and I would assume for Material-UI as well). We actually just added some functionality to our app that would definitely be a little nicer as a dropdown. The composite widget support is a nice accessibility feature, but most people probably won't realize that they should be doing this until we add documentation about it. I'll probably mainly use it as a filler task if I have time while waiting for code review for the popup/dropdown tasks.
@oliviertassinari
it's not obvious to me when a list should be considered composite or when it shouldn't.
Could you give an example of when it shouldn't?
Could you give an example of when it shouldn't?
I agree with @mbrookes's implied point, that all lists probably should be treated as composites. I just think it needs to be opt-in because for some lists it will require work by users to ensure that all the focusable elements are removed from the tab sequence. Once this capability reaches maturity, it should be a pretty small percentage of lists that need additional work by users, so as long as we provide sufficient guidance in the migration guide for v5, it may make sense to switch to opt-out at that point.
@oliviertassinari As far as proving out this functionality, my plan is for most of the List
demos to use it, and I would also convert the left-side navigation for the docs to use it. I think the docs navigation will be a particularly good use case for demonstrating the benefits of this enhancement.
@mbrookes @ryancogswell I don't have a strong opinion on this. I'm not sure I understand all the implications. What happens for somebody displaying data with a list (ul > li > div), considering it has nested focusable link elements inside each list item?
Dropdowns are the higher priority for me (and I would assume for Material-UI as well)
I agree.
What happens for somebody displaying data with a list (ul > li > div), considering it has nested focusable link elements inside each list item?
There are quite a few complex possible scenarios. The most complex involve editable content. The ARIA documentation provides guidance for toggling between grid navigation vs. editing here, but I won't even try to handle that in the initial implementation. Whether we can consider switching to opt-out in the future depends on how mature this is at that point and how rare the scenarios are that aren't handled well.
Is there something left to do on this issue?
This is a continuation of the discussion started by @mbrookes in #15334 and #15421. I have not yet made any attempts at an implementation, but I wanted to go ahead and document my thoughts about the problem space.
What is a composite widget?
WAI-ARIA describes a composite widget as having the following characteristics:
What are examples of composite widgets?
WAI-ARIA describes the following design patterns and widgets as composite widgets:
What is the accessibility benefit of composite widgets?
Composite widgets allow keyboard users to navigate quickly between different interactive portions of the page. tab and shift+tab can be used to skip between different composite widgets and to any focusable elements that are not part of a composite, and then the arrow keys can be used to navigate within a composite widget.
What is the current state of support for composite widgets in Material-UI?
The following widgets are already treated as composites in their Material-UI counterparts:
The following widgets have a Material-UI counterpart that is not treated as a composite widget (i.e. tab navigates within these widgets rather than skipping to the next focusable element outside the widget):
The Tree View and Treegrid patterns do not currently have any direct Material-UI counterpart, however nested Lists can be used to create tree widgets. The Grid pattern can also be implemented in Material-UI using components other than Lists, for instance a grouping of Cards could be considered a WAI-ARIA Grid. If each Card has multiple focusable elements, then each Card in a group of Cards would represent a row in the grid.
What should be done to improve support for composite widgets in Material-UI?
Adding composite widget support for Lists
In the pull requests by @mbrookes that preceded this issue, the main idea was to move the keyboard focus navigation from MenuList down into List so that the focus logic could be used more broadly. While I think something along these lines is possible, there are a number of challenges to address:
tabindex
of -1, so that tab and shift+tab can be used reliably to skip out of the composite widget.Adding composite widget support for other components
I think it would be worthwhile to consider other scenarios (e.g. group of Cards, AppBar, Tabs) at the same time to explore which implementation aspects might make sense to be shared and to work towards a more general solution for composite widgets, but I'm not sure if a general solution will actually make sense in the end.