[TreeView] Focus selected when tree receives focus #20205
Conversation
|
@material-ui/lab: parsed: +0.39% , gzip: +0.43% Details of bundle changes.Comparing: 978381d...bebcef3 Details of page changes
|
oliviertassinari
added
the
component: tree view
There was a problem hiding this comment.
The aria practices does not describe what should happen if the tree receives focus and the selected tree item is in a collapsed parent.
Because the tree itself is not focusable. It doesn't make sense to make the widget container focusable and its children. In that case you want a focusable container and aria-activedescendant.
If you want to use our Treeview implementation and a focusable role="tree" then focus handling is your responsibility. It sounds like we don't have a good customization story for this?
It seems like you're trying to fix multiple issues at the same time. Feel free to fix #19882 but any change that affects the a11y story needs more research.
github-actions
bot
added
the
PR: out-of-date
I agree. In my implementation, like yours, at any one time the user experience is that the tree is not focusable just the tree items. I move focus to the selected item when the tree is focused. To the user they are tabbing to the tree item. I hide the focus outline so there is no flicker. This behaviour adheres to https://www.w3.org/TR/wai-aria-practices/#TreeView.
Your current tests would suggest that you are following this and there is no change to the a11y story in this regard. As I have already mentioned your code does not 'focus the selected node if a node is selected before the tree receives focus'. The test only passes because the selected node happens to be the focused node. If the test had selected 'two' and navigated to 'three', before focusing away and back again, the test will fail. My code does pass these tests ( and all other tests ), it literally does 'focus the selected node when the tree receives focus'.
The 'tree' is focusable.
If you were to get your code to pass the test 'should focus the selected node if a node is selected before the tree receives focus' what would you do if the selected node was in a collapsed parent ?
If I change the code to make the TreeView tabbable ( thus allowing for 'when a tree receives focus...' ) then my code will conform to blur behaviour. To reiterate - my code behaves the same as your tests suggest that your code should behave. |
oliviertassinari
changed the title
|
@tonyhallett Please update this branch. |
|
This is interesting. The guideline says one thing but the demos on the guideline showcase the current behaviour. I've also only seen that the last focus is what next receives focus on all other tree views. |
|
This is now the case and was implemented here: https://github.com/mui-org/material-ui/pull/21695/files#diff-1192977187ec51459a4dd0b09758a929R679 |
In addition to making the tree view agree with aria practices this pull request also resolves some other outstanding issues.
TreeItem hijacks focus when typing on an input box that updates it
This code is present in this codesandbox that replicates the one in the issue. The steps below that result in the issue behaviour as seen in the original sandbox are no longer an issue.
TreeView programmatic focus does not change tab indices inconsistent behaviour is no longer an issue as it selection and not last focus that determines what is focused when the tree receives focus ( as per aria practices ).
TreeView cannot be controlled by keyboard after item removal is also no longer an issue.
Note that there is no longer a need for tabbable state.
Remarks on the tests :
Tests that conditionally render a tree item have to render a span - due to bug
I corrected the tests regarding this focus behaviour - but perhaps the tests should be in TreeView.test.js instead of TreeItem.test.js.
Remarks on the code :
The aria practices does not describe what should happen if the tree receives focus and the selected tree item is in a collapsed parent. I decided to expand the tree to make it visible for now ( I have not raised the event - if this behaviour is agreed I will change ), the alternative is to focus the first item.
https://github.com/tonyhallett/material-ui/blob/bebcef3ff8da24ee3daa04eed50d095da585ef89/packages/material-ui-lab/src/TreeView/TreeView.js#L483
The clean up of selected can probably be improved upon.
https://github.com/tonyhallett/material-ui/blob/bebcef3ff8da24ee3daa04eed50d095da585ef89/packages/material-ui-lab/src/TreeView/TreeView.js#L514
When a focused item is removed I have decided to refocus the tree view. I can change this to making the TreeView tabbable again.
https://github.com/tonyhallett/material-ui/blob/bebcef3ff8da24ee3daa04eed50d095da585ef89/packages/material-ui-lab/src/TreeView/TreeView.js#L574
You can see this code on this sandbox