From fb90f444d2eb61f441ed739893262a4bac79ee5f Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Fri, 13 Mar 2020 17:05:35 +0000 Subject: [PATCH 1/2] [TreeItem] correct single-select aria-selected --- .../material-ui-lab/src/TreeItem/TreeItem.js | 10 ++- .../src/TreeItem/TreeItem.test.js | 71 ++++++++++++------- .../src/TreeView/TreeView.test.js | 8 +-- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js index 1b3c568b7cff2f..edbdc1b67bc96a 100644 --- a/packages/material-ui-lab/src/TreeItem/TreeItem.js +++ b/packages/material-ui-lab/src/TreeItem/TreeItem.js @@ -365,6 +365,14 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) { } }, [focused]); + let ariaSelected; + if (!selectionDisabled) { + if (multiSelect) { + ariaSelected = selected; + } else if (selected) { + ariaSelected = true; + } + } return (
  • ', () => { }); describe('aria-selected', () => { - it('should have the attribute `aria-selected=false` if not selected', () => { - const { getByTestId } = render( - - - , - ); + describe('single-select', () => { + it('should not have the attribute `aria-selected` if not selected', () => { + const { getByTestId } = render( + + + , + ); - 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( - - - , - ); + it('should have the attribute `aria-selected=true` if selected', () => { + const { getByTestId } = render( + + + , + ); - 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( - - - , - ); + describe('multi-select', () => { + it('should have the attribute `aria-selected=false` if not selected', () => { + const { getByTestId } = render( + + + , + ); + + expect(getByTestId('test')).to.have.attribute('aria-selected', 'false'); + }); + it('should have the attribute `aria-selected=true` if selected', () => { + const { getByTestId } = render( + + + , + ); + + expect(getByTestId('test')).to.have.attribute('aria-selected', 'true'); + }); + + it('should not have the attribute `aria-selected` if disableSelection is true', () => { + const { getByTestId } = render( + + + , + ); - expect(getByTestId('test')).to.not.have.attribute('aria-selected'); + expect(getByTestId('test')).to.not.have.attribute('aria-selected'); + }); }); }); @@ -695,7 +718,7 @@ describe('', () => { ); 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'); }); @@ -709,7 +732,7 @@ describe('', () => { , ); - 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'); }); diff --git a/packages/material-ui-lab/src/TreeView/TreeView.test.js b/packages/material-ui-lab/src/TreeView/TreeView.test.js index 2e785dfb28bc32..a679b09641b9e9 100644 --- a/packages/material-ui-lab/src/TreeView/TreeView.test.js +++ b/packages/material-ui-lab/src/TreeView/TreeView.test.js @@ -106,13 +106,13 @@ describe('', () => { const { getByTestId, getByText } = render(); - 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'); }); From 7a735fe362a3ccf95f6bbb606eae554362ff4c6a Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 21 Mar 2020 22:59:23 +0100 Subject: [PATCH 2/2] Ignore disableSelected --- packages/material-ui-lab/src/TreeItem/TreeItem.js | 12 ++++++------ .../material-ui-lab/src/TreeItem/TreeItem.test.js | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/material-ui-lab/src/TreeItem/TreeItem.js b/packages/material-ui-lab/src/TreeItem/TreeItem.js index edbdc1b67bc96a..461f51551362da 100644 --- a/packages/material-ui-lab/src/TreeItem/TreeItem.js +++ b/packages/material-ui-lab/src/TreeItem/TreeItem.js @@ -366,13 +366,13 @@ const TreeItem = React.forwardRef(function TreeItem(props, ref) { }, [focused]); let ariaSelected; - if (!selectionDisabled) { - if (multiSelect) { - ariaSelected = selected; - } else if (selected) { - ariaSelected = true; - } + if (multiSelect) { + ariaSelected = selected; + } else if (selected) { + // single-selection trees unset aria-selected + ariaSelected = true; } + return (
  • ', () => { expect(getByTestId('test')).to.have.attribute('aria-selected', 'true'); }); - it('should not have the attribute `aria-selected` if disableSelection is true', () => { + it('should have the attribute `aria-selected` if disableSelection is true', () => { const { getByTestId } = render( , ); - expect(getByTestId('test')).to.not.have.attribute('aria-selected'); + expect(getByTestId('test')).to.have.attribute('aria-selected', 'false'); }); }); });