From 89a868bd2a46a0fba74bbc031165e1e614b8ef9d Mon Sep 17 00:00:00 2001 From: Silviu Date: Thu, 18 Apr 2019 12:07:29 +0200 Subject: [PATCH 1/6] added cleanup after timeout --- packages/react/src/components/Dropdown/Dropdown.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index 33a6ee81f4..b1ae86be63 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -226,6 +226,8 @@ class Dropdown extends AutoControlledComponent, Dropdo static className = 'ui-dropdown' + static a11yStatusCleanupTime = 500 + static slotClassNames: DropdownSlotClassNames static propTypes = { @@ -950,6 +952,9 @@ class Dropdown extends AutoControlledComponent, Dropdo if (getA11ySelectionMessage && getA11ySelectionMessage.onAdd) { this.setState({ a11ySelectionStatus: getA11ySelectionMessage.onAdd(item) }) + setTimeout(() => { + this.setState({ a11ySelectionStatus: '' }) + }, Dropdown.a11yStatusCleanupTime) } if (multiple) { @@ -1035,6 +1040,9 @@ class Dropdown extends AutoControlledComponent, Dropdo if (getA11ySelectionMessage && getA11ySelectionMessage.onRemove) { this.setState({ a11ySelectionStatus: getA11ySelectionMessage.onRemove(poppedItem) }) + setTimeout(() => { + this.setState({ a11ySelectionStatus: '' }) + }, Dropdown.a11yStatusCleanupTime) } this.trySetStateAndInvokeHandler('onSelectedChange', null, { value }) From b34365d9b1fd7cd1a3174de41831fc56f32b90ce Mon Sep 17 00:00:00 2001 From: Silviu Date: Thu, 18 Apr 2019 13:00:31 +0200 Subject: [PATCH 2/6] addded unit tests --- .../components/Dropdown/Dropdown-test.tsx | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx index 9984d1c3f4..171555e0d9 100644 --- a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx +++ b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx @@ -4,10 +4,12 @@ import * as _ from 'lodash' import Dropdown from 'src/components/Dropdown/Dropdown' import DropdownSearchInput from 'src/components/Dropdown/DropdownSearchInput' +import DropdownSelectedItem from 'src/components/Dropdown/DropdownSelectedItem' import { isConformant } from 'test/specs/commonTests' import { mountWithProvider } from 'test/utils' jest.dontMock('keyboard-key') +jest.useFakeTimers() describe('Dropdown', () => { const items = ['item1', 'item2', 'item3', 'item4', 'item5'] @@ -957,4 +959,86 @@ describe('Dropdown', () => { expect(preventDefault).not.toBeCalled() }) }) + + describe('getA11ySelectionMessage', () => { + afterEach(() => { + jest.runAllTimers() + }) + + it('has the onAdd message inserted after an item has been added to selection', () => { + const wrapper = mountWithProvider( + 'bla bla added' }} + />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + + expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla added') + }) + + it('has the onRemove message inserted after an item has been removed from selection', () => { + const wrapper = mountWithProvider( + 'bla bla removed' }} + />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + jest.runAllTimers() + const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) + removeIcon.simulate('click') + + expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla removed') + }) + + it('has the onAdd message cleared after being displayed', () => { + const wrapper = mountWithProvider( + 'bla bla' }} />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + + jest.runAllTimers() + expect(dropdown.state('a11ySelectionStatus')).toBe('') + }) + + it('has the onRemove message cleared after being displayed', () => { + const wrapper = mountWithProvider( + 'bla bla' }} + />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + jest.runAllTimers() + const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) + removeIcon.simulate('click') + jest.runAllTimers() + + expect(dropdown.state('a11ySelectionStatus')).toBe('') + }) + }) }) From 1763ef8bf7f8d69f47a2cddc0821da1191039212 Mon Sep 17 00:00:00 2001 From: Silviu Date: Thu, 18 Apr 2019 13:31:13 +0200 Subject: [PATCH 3/6] added cleanup --- .../src/components/Dropdown/Dropdown.tsx | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index b1ae86be63..38e2306d4a 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -317,6 +317,12 @@ class Dropdown extends AutoControlledComponent, Dropdo } } + a11yStatusTimeout: any + + componentWillUnmount() { + clearTimeout(this.a11yStatusTimeout) + } + public renderComponent({ ElementType, classes, @@ -952,9 +958,7 @@ class Dropdown extends AutoControlledComponent, Dropdo if (getA11ySelectionMessage && getA11ySelectionMessage.onAdd) { this.setState({ a11ySelectionStatus: getA11ySelectionMessage.onAdd(item) }) - setTimeout(() => { - this.setState({ a11ySelectionStatus: '' }) - }, Dropdown.a11yStatusCleanupTime) + this.setA11ySelectionMessage() } if (multiple) { @@ -1040,9 +1044,7 @@ class Dropdown extends AutoControlledComponent, Dropdo if (getA11ySelectionMessage && getA11ySelectionMessage.onRemove) { this.setState({ a11ySelectionStatus: getA11ySelectionMessage.onRemove(poppedItem) }) - setTimeout(() => { - this.setState({ a11ySelectionStatus: '' }) - }, Dropdown.a11yStatusCleanupTime) + this.setA11ySelectionMessage() } this.trySetStateAndInvokeHandler('onSelectedChange', null, { value }) @@ -1138,6 +1140,17 @@ class Dropdown extends AutoControlledComponent, Dropdo // otherwise, highlight no item. return null } + + /** + * Function that sets and cleans the selection message after it has been set, + * so it is not read anymore via virtual cursor. + */ + private setA11ySelectionMessage = (): void => { + clearTimeout(this.a11yStatusTimeout) + this.a11yStatusTimeout = setTimeout(() => { + this.setState({ a11ySelectionStatus: '' }) + }, Dropdown.a11yStatusCleanupTime) + } } Dropdown.slotClassNames = { From 7bf3fe79820d7d895c5c5feec1096c2363b481c5 Mon Sep 17 00:00:00 2001 From: Silviu Date: Thu, 18 Apr 2019 17:29:16 +0200 Subject: [PATCH 4/6] merged the suites together --- .../components/Dropdown/Dropdown-test.tsx | 162 +++++++++--------- 1 file changed, 80 insertions(+), 82 deletions(-) diff --git a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx index 171555e0d9..63c567f26e 100644 --- a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx +++ b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx @@ -782,6 +782,10 @@ describe('Dropdown', () => { }) describe('getA11ySelectionMessage', () => { + afterEach(() => { + jest.runAllTimers() + }) + it('creates message container element', () => { mountWithProvider() expect( @@ -790,6 +794,82 @@ describe('Dropdown', () => { ), ).toBeTruthy() }) + + it('has the onAdd message inserted after an item has been added to selection', () => { + const wrapper = mountWithProvider( + 'bla bla added' }} + />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + + expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla added') + }) + + it('has the onRemove message inserted after an item has been removed from selection', () => { + const wrapper = mountWithProvider( + 'bla bla removed' }} + />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + jest.runAllTimers() + const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) + removeIcon.simulate('click') + + expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla removed') + }) + + it('has the onAdd message cleared after being displayed', () => { + const wrapper = mountWithProvider( + 'bla bla' }} />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + + jest.runAllTimers() + expect(dropdown.state('a11ySelectionStatus')).toBe('') + }) + + it('has the onRemove message cleared after being displayed', () => { + const wrapper = mountWithProvider( + 'bla bla' }} + />, + ) + const dropdown = wrapper.find(Dropdown) + const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) + + triggerButton.simulate('click') + const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) + firstItem.simulate('click') + jest.runAllTimers() + const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) + removeIcon.simulate('click') + jest.runAllTimers() + + expect(dropdown.state('a11ySelectionStatus')).toBe('') + }) }) describe('searchQuery', () => { @@ -959,86 +1039,4 @@ describe('Dropdown', () => { expect(preventDefault).not.toBeCalled() }) }) - - describe('getA11ySelectionMessage', () => { - afterEach(() => { - jest.runAllTimers() - }) - - it('has the onAdd message inserted after an item has been added to selection', () => { - const wrapper = mountWithProvider( - 'bla bla added' }} - />, - ) - const dropdown = wrapper.find(Dropdown) - const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) - - triggerButton.simulate('click') - const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) - firstItem.simulate('click') - - expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla added') - }) - - it('has the onRemove message inserted after an item has been removed from selection', () => { - const wrapper = mountWithProvider( - 'bla bla removed' }} - />, - ) - const dropdown = wrapper.find(Dropdown) - const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) - - triggerButton.simulate('click') - const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) - firstItem.simulate('click') - jest.runAllTimers() - const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) - removeIcon.simulate('click') - - expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla removed') - }) - - it('has the onAdd message cleared after being displayed', () => { - const wrapper = mountWithProvider( - 'bla bla' }} />, - ) - const dropdown = wrapper.find(Dropdown) - const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) - - triggerButton.simulate('click') - const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) - firstItem.simulate('click') - - jest.runAllTimers() - expect(dropdown.state('a11ySelectionStatus')).toBe('') - }) - - it('has the onRemove message cleared after being displayed', () => { - const wrapper = mountWithProvider( - 'bla bla' }} - />, - ) - const dropdown = wrapper.find(Dropdown) - const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) - - triggerButton.simulate('click') - const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) - firstItem.simulate('click') - jest.runAllTimers() - const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) - removeIcon.simulate('click') - jest.runAllTimers() - - expect(dropdown.state('a11ySelectionStatus')).toBe('') - }) - }) }) From 80e6e16556f95dc88e247a1a26408657853b49ce Mon Sep 17 00:00:00 2001 From: Silviu Avram Date: Tue, 23 Apr 2019 08:41:44 +0200 Subject: [PATCH 5/6] added changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1ec5cec9c..d721450c19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### BREAKING CHANGES - Rename `inputFocusBorderBottomColor` to `inputFocusBorderColor` in `InputVariables` @layershifter ([#1247](https://github.com/stardust-ui/react/pull/1247)) +### Fixes +- Fix a11y message cleanup for add and remove items in `Dropdown` @silviuavram ([#1237](https://github.com/stardust-ui/react/pull/1237)) + ### Features - Move `Input` styles to Base theme @layershifter ([#1247](https://github.com/stardust-ui/react/pull/1247)) From ff1d3249d4ba35336ad09f14aa2b5bb9ab2e9785 Mon Sep 17 00:00:00 2001 From: Silviu Avram Date: Wed, 24 Apr 2019 10:38:51 +0200 Subject: [PATCH 6/6] merged the status tests --- .../components/Dropdown/Dropdown-test.tsx | 41 +++---------------- 1 file changed, 6 insertions(+), 35 deletions(-) diff --git a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx index 63c567f26e..c47b725dba 100644 --- a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx +++ b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx @@ -795,7 +795,7 @@ describe('Dropdown', () => { ).toBeTruthy() }) - it('has the onAdd message inserted after an item has been added to selection', () => { + it('has the onAdd message inserted and cleared after an item has been added to selection', () => { const wrapper = mountWithProvider( { firstItem.simulate('click') expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla added') - }) - it('has the onRemove message inserted after an item has been removed from selection', () => { - const wrapper = mountWithProvider( - 'bla bla removed' }} - />, - ) - const dropdown = wrapper.find(Dropdown) - const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) - - triggerButton.simulate('click') - const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) - firstItem.simulate('click') jest.runAllTimers() - const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) - removeIcon.simulate('click') - expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla removed') - }) - - it('has the onAdd message cleared after being displayed', () => { - const wrapper = mountWithProvider( - 'bla bla' }} />, - ) - const dropdown = wrapper.find(Dropdown) - const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`) - - triggerButton.simulate('click') - const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0) - firstItem.simulate('click') - - jest.runAllTimers() expect(dropdown.state('a11ySelectionStatus')).toBe('') }) - it('has the onRemove message cleared after being displayed', () => { + it('has the onRemove message inserted and cleared after an item has been removed from selection', () => { const wrapper = mountWithProvider( 'bla bla' }} + getA11ySelectionMessage={{ onRemove: item => 'bla bla removed' }} />, ) const dropdown = wrapper.find(Dropdown) @@ -866,6 +834,9 @@ describe('Dropdown', () => { jest.runAllTimers() const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`) removeIcon.simulate('click') + + expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla removed') + jest.runAllTimers() expect(dropdown.state('a11ySelectionStatus')).toBe('')