Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[TreeView] Fix usage of the aria-selected attribute #14991

Merged
merged 1 commit into from
Oct 16, 2024
Merged

[TreeView] Fix usage of the aria-selected attribute #14991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions packages/x-tree-view/src/TreeItem/TreeItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export const TreeItem = React.forwardRef(function TreeItem(
icons: contextIcons,
runItemPlugins,
items: { disabledItemsFocusable, indentationAtItemLevel },
selection: { multiSelect },
selection: { disableSelection },
expansion: { expansionTrigger },
treeId,
instance,
Expand Down Expand Up @@ -358,17 +358,17 @@ export const TreeItem = React.forwardRef(function TreeItem(
});
const icon = Icon ? <Icon {...iconProps} /> : null;

let ariaSelected;
if (multiSelect) {
ariaSelected = selected;
} else if (selected) {
/* single-selection trees unset aria-selected on un-selected items.
*
* If the tree does not support multiple selection, aria-selected
* is set to true for the selected item and it is not present on any other item in the tree.
* Source: https://www.w3.org/WAI/ARIA/apg/patterns/treeview/
*/
// https://www.w3.org/WAI/ARIA/apg/patterns/treeview/
let ariaSelected: boolean | undefined;
if (selected) {
// - each selected node has aria-selected set to true.
ariaSelected = true;
} else if (disableSelection || disabled) {
// - if the tree contains nodes that are not selectable, aria-selected is not present on those nodes.
ariaSelected = undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They mentioned that "without the aria-selected=“false”, NVDA doesn't render the information, so we don't know if the item is selected or not."
I'm not sure if NVDA does that only when there's at least one selected, but I'm wondering if, in order to "display" the items, it should always be false even when there's no selection or selection is disabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-selected states for Grid that setting aria-selected="false" on a focusable gridcell indicates the cell is selectable.
I believe the same logic applies to Tree items, so ariaSelected=undefined is the way to go indeed.

} else {
// - all nodes that are selectable but not selected have aria-selected set to false.
ariaSelected = false;
}

function handleFocus(event: React.FocusEvent<HTMLLIElement>) {
Expand Down
17 changes: 13 additions & 4 deletions ...ages/x-tree-view/src/internals/plugins/useTreeViewSelection/useTreeViewSelection.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,12 +712,12 @@ describeTreeView<[UseTreeViewSelectionSignature, UseTreeViewExpansionSignature]>
// This `describe` only tests basics scenarios, more complex scenarios are tested in this file's other `describe`.
describe('aria-selected item attribute', () => {
describe('single selection', () => {
it('should not have the attribute `aria-selected=false` if not selected', () => {
it('should have the attribute `aria-selected=false` if not selected', () => {
const view = render({
items: [{ id: '1' }, { id: '2' }],
});

expect(view.getItemRoot('1')).not.to.have.attribute('aria-selected');
expect(view.getItemRoot('1')).to.have.attribute('aria-selected', 'false');
});

it('should have the attribute `aria-selected=true` if selected', () => {
Expand Down Expand Up @@ -750,14 +750,23 @@ describeTreeView<[UseTreeViewSelectionSignature, UseTreeViewExpansionSignature]>
expect(view.getItemRoot('1')).to.have.attribute('aria-selected', 'true');
});

it('should have the attribute `aria-selected=false` if disabledSelection is true', () => {
it('should not have the attribute `aria-selected=false` if disabledSelection is true', () => {
const view = render({
multiSelect: true,
items: [{ id: '1' }, { id: '2' }],
disableSelection: true,
});

expect(view.getItemRoot('1')).to.have.attribute('aria-selected', 'false');
expect(view.getItemRoot('1')).not.to.have.attribute('aria-selected');
});

it('should not have the attribute `aria-selected=false` if the item is disabled', () => {
const view = render({
multiSelect: true,
items: [{ id: '1', disabled: true }, { id: '2' }],
});

expect(view.getItemRoot('1')).not.to.have.attribute('aria-selected');
});
});
});
Expand Down
20 changes: 10 additions & 10 deletions packages/x-tree-view/src/useTreeItem2/useTreeItem2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const useTreeItem2 = <
const {
runItemPlugins,
items: { onItemClick, disabledItemsFocusable, indentationAtItemLevel },
selection: { multiSelect, disableSelection, checkboxSelection },
selection: { disableSelection, checkboxSelection },
expansion: { expansionTrigger },
treeId,
instance,
Expand Down Expand Up @@ -194,17 +194,17 @@ export const useTreeItem2 = <
...extractEventHandlers(externalProps),
};

// https://www.w3.org/WAI/ARIA/apg/patterns/treeview/
let ariaSelected: boolean | undefined;
if (multiSelect) {
ariaSelected = status.selected;
} else if (status.selected) {
/* single-selection trees unset aria-selected on un-selected items.
*
* If the tree does not support multiple selection, aria-selected
* is set to true for the selected item and it is not present on any other item in the tree.
* Source: https://www.w3.org/WAI/ARIA/apg/patterns/treeview/
*/
if (status.selected) {
// - each selected node has aria-selected set to true.
ariaSelected = true;
} else if (disableSelection || status.disabled) {
// - if the tree contains nodes that are not selectable, aria-selected is not present on those nodes.
ariaSelected = undefined;
} else {
// - all nodes that are selectable but not selected have aria-selected set to false.
ariaSelected = false;
}

const props: UseTreeItem2RootSlotPropsFromUseTreeItem = {
Expand Down
Loading