Skip to content

Commit

Permalink
[TreeItem] correct single-select aria-selected (#20102)
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyhallett authored Mar 22, 2020
1 parent 2a24dd1 commit 46b454a
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 29 deletions.
10 changes: 9 additions & 1 deletion packages/material-ui-lab/src/TreeItem/TreeItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,14 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
}
}, [focused]);

let ariaSelected;
if (multiSelect) {
ariaSelected = selected;
} else if (selected) {
// single-selection trees unset aria-selected
ariaSelected = true;
}

return (
<li
className={clsx(classes.root, className, {
Expand All @@ -375,7 +383,7 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) {
onKeyDown={handleKeyDown}
onFocus={handleFocus}
aria-expanded={expandable ? expanded : null}
aria-selected={!selectionDisabled && isSelected ? isSelected(nodeId) : undefined}
aria-selected={ariaSelected}
ref={handleRef}
tabIndex={tabbable ? 0 : -1}
{...other}
Expand Down
71 changes: 47 additions & 24 deletions packages/material-ui-lab/src/TreeItem/TreeItem.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,34 +199,57 @@ describe('<TreeItem />', () => {
});

describe('aria-selected', () => {
it('should have the attribute `aria-selected=false` if not selected', () => {
const { getByTestId } = render(
<TreeView>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);
describe('single-select', () => {
it('should not have the attribute `aria-selected` if not selected', () => {
const { getByTestId } = render(
<TreeView>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);

expect(getByTestId('test')).to.have.attribute('aria-selected', 'false');
});
expect(getByTestId('test')).to.not.have.attribute('aria-selected');
});

it('should have the attribute `aria-selected=true` if selected', () => {
const { getByTestId } = render(
<TreeView defaultSelected={'test'}>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);
it('should have the attribute `aria-selected=true` if selected', () => {
const { getByTestId } = render(
<TreeView defaultSelected={'test'}>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);

expect(getByTestId('test')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('test')).to.have.attribute('aria-selected', 'true');
});
});

it('should not have the attribute `aria-selected` if disableSelection is true', () => {
const { getByTestId } = render(
<TreeView disableSelection>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);
describe('multi-select', () => {
it('should have the attribute `aria-selected=false` if not selected', () => {
const { getByTestId } = render(
<TreeView multiSelect>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);

expect(getByTestId('test')).to.have.attribute('aria-selected', 'false');
});
it('should have the attribute `aria-selected=true` if selected', () => {
const { getByTestId } = render(
<TreeView multiSelect defaultSelected={'test'}>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);

expect(getByTestId('test')).to.have.attribute('aria-selected', 'true');
});

it('should have the attribute `aria-selected` if disableSelection is true', () => {
const { getByTestId } = render(
<TreeView multiSelect disableSelection>
<TreeItem nodeId="test" label="test" data-testid="test" />
</TreeView>,
);

expect(getByTestId('test')).to.not.have.attribute('aria-selected');
expect(getByTestId('test')).to.have.attribute('aria-selected', 'false');
});
});
});

Expand Down Expand Up @@ -685,7 +708,7 @@ describe('<TreeItem />', () => {
);

getByTestId('one').focus();
expect(getByTestId('one')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('one')).to.not.have.attribute('aria-selected');
fireEvent.keyDown(document.activeElement, { key: ' ' });
expect(getByTestId('one')).to.have.attribute('aria-selected', 'true');
});
Expand All @@ -699,7 +722,7 @@ describe('<TreeItem />', () => {
</TreeView>,
);

expect(getByTestId('one')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('one')).to.not.have.attribute('aria-selected');
fireEvent.click(getByText('one'));
expect(getByTestId('one')).to.have.attribute('aria-selected', 'true');
});
Expand Down
8 changes: 4 additions & 4 deletions packages/material-ui-lab/src/TreeView/TreeView.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ describe('<TreeView />', () => {

const { getByTestId, getByText } = render(<MyComponent />);

expect(getByTestId('one')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('two')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('one')).to.not.have.attribute('aria-selected');
expect(getByTestId('two')).to.not.have.attribute('aria-selected');
fireEvent.click(getByText('one'));
expect(getByTestId('one')).to.have.attribute('aria-selected', 'true');
expect(getByTestId('two')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('two')).to.not.have.attribute('aria-selected');
fireEvent.click(getByText('two'));
expect(getByTestId('one')).to.have.attribute('aria-selected', 'false');
expect(getByTestId('one')).to.not.have.attribute('aria-selected');
expect(getByTestId('two')).to.have.attribute('aria-selected', 'true');
});

Expand Down

0 comments on commit 46b454a

Please sign in to comment.