From 356bd4f46328be6d884ab0758193f65d9fd7aca4 Mon Sep 17 00:00:00 2001 From: Mohammer5 Date: Mon, 25 Nov 2024 21:42:44 +0800 Subject: [PATCH] fix(select a11y): handle remaining TODO comments & rename internal components --- collections/forms/i18n/en.pot | 4 +- .../SingleSelectA11yFieldFF.js | 12 +- .../SingleSelectA11yFieldFF.prod.stories.js | 6 +- .../SingleSelectA11yFieldFF.test.js | 6 +- .../single-select-a11y-field.js | 18 +- .../single-select-a11y-field.prod.stories.js | 16 +- .../single-select-a11y-field.test.js | 14 +- .../__stories__/DefaultPosition.js | 18 +- .../single-select-a11y/__stories__/Dense.js | 13 +- .../single-select-a11y/__stories__/Empty.js | 14 +- .../__stories__/EmptyWithEmptyNode.js | 14 +- .../__stories__/EmptyWithEmptyText.js | 14 +- .../__stories__/FlippedPosition.js | 18 +- .../__stories__/InfiniteLoading.js | 12 +- .../single-select-a11y/__stories__/Loading.js | 5 +- .../__stories__/ShiftedIntoView.js | 17 +- .../__stories__/WithClearButton.js | 16 +- .../__stories__/WithCustomLowMaxHeight.js | 13 +- .../__stories__/WithCustomOptions.js | 13 +- .../__stories__/WithDisabledOption.js | 16 +- .../__stories__/WithFilterField.js | 13 +- .../__stories__/WithInitialFocus.js | 14 +- .../__stories__/WithManyOptions.js | 16 +- .../__stories__/WithNoMatchText.js | 11 +- .../__stories__/WithOnBlur.js | 14 +- .../__stories__/WithOnFocus.js | 14 +- .../__stories__/WithOptionsAndDisabled.js | 13 +- .../__stories__/WithOptionsAndLoading.js | 13 +- .../__stories__/WithOptionsAndLoadingText.js | 13 +- .../__stories__/WithPlaceholder.js | 13 +- .../WithPlaceholderAndSelection.js | 16 +- .../__stories__/WithPrefix.js | 13 +- .../__stories__/WithPrefixAndSelection.js | 16 +- .../single-select-a11y/__stories__/WithRTL.js | 17 +- .../__stories__/WithSelection.js | 16 +- .../__stories__/WithSelectionAndDisabled.js | 16 +- .../__stories__/WithoutOptionsAndLoading.js | 16 ++ .../__stories__/WithoutSelection.js | 9 +- .../keyboard-interactions.test.e2e.js | 32 +++- .../features/menu-positioning.test.e2e.js | 2 +- .../single-select-a11y/is-option-hidden.js | 12 +- .../menu/{menu-filter.js => filter.js} | 10 +- .../menu/{menu-loading.js => loading.js} | 4 +- .../src/single-select-a11y/menu/menu.js | 55 ++---- .../src/single-select-a11y/menu/option.js | 2 +- .../{menu-options-list.js => options-list.js} | 30 ++- .../selected-value/clear-button.js | 90 +++++++++ ...lected-value-container.js => container.js} | 10 +- ...ed-value-placeholder.js => placeholder.js} | 12 +- .../{selected-value-prefix.js => prefix.js} | 4 +- .../selected-value-clear-button.js | 86 --------- .../selected-value/selected-value.js | 39 ++-- .../single-select-a11y.e2e.stories.js | 76 +++----- .../single-select-a11y/single-select-a11y.js | 101 ++++------ .../single-select-a11y.prod.stories.js | 1 + .../single-select-a11y.test.js | 181 +++++++++--------- .../use-handle-key-press/index.js | 2 +- ...js => use-handle-key-press-on-combobox.js} | 8 +- .../use-handle-key-press/use-handle-typing.js | 5 +- ...highlight-first-option-on-previous-page.js | 7 +- .../use-highlight-last-option-on-next-page.js | 7 +- ...ingle-select-a11y-server-side-filtering.js | 25 +-- ...ingle-select-a11y-server-side-filtering.md | 66 ++----- .../single-select-a11y-simple-filtering.js | 18 +- .../single-select-a11y-simple-filtering.md | 33 +--- 65 files changed, 619 insertions(+), 811 deletions(-) create mode 100644 components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js rename components/select/src/single-select-a11y/menu/{menu-filter.js => filter.js} (90%) rename components/select/src/single-select-a11y/menu/{menu-loading.js => loading.js} (93%) rename components/select/src/single-select-a11y/menu/{menu-options-list.js => options-list.js} (81%) create mode 100644 components/select/src/single-select-a11y/selected-value/clear-button.js rename components/select/src/single-select-a11y/selected-value/{selected-value-container.js => container.js} (94%) rename components/select/src/single-select-a11y/selected-value/{selected-value-placeholder.js => placeholder.js} (73%) rename components/select/src/single-select-a11y/selected-value/{selected-value-prefix.js => prefix.js} (85%) delete mode 100644 components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js rename components/select/src/single-select-a11y/use-handle-key-press/{use-handle-key-press.js => use-handle-key-press-on-combobox.js} (96%) diff --git a/collections/forms/i18n/en.pot b/collections/forms/i18n/en.pot index aedabd0ab..3949bef80 100644 --- a/collections/forms/i18n/en.pot +++ b/collections/forms/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2024-11-19T08:12:09.695Z\n" -"PO-Revision-Date: 2024-11-19T08:12:09.696Z\n" +"POT-Creation-Date: 2024-11-28T08:22:51.040Z\n" +"PO-Revision-Date: 2024-11-28T08:22:51.041Z\n" msgid "Upload file" msgstr "Upload file" diff --git a/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.js b/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.js index 364badd5f..89700a357 100644 --- a/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.js +++ b/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.js @@ -28,6 +28,7 @@ export const SingleSelectA11yFieldFF = ({ return ( props.filterable, PropTypes.string), + /** Allows to override what's rendered inside the `button[role="option"]`. + * Can be overriden on an individual option basis **/ + optionComponent: PropTypes.elementType, + /** For a11y: How aggressively the user should be updated about changes in options **/ optionUpdateStrategy: PropTypes.oneOf(['off', 'polite', 'assertive']), diff --git a/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.prod.stories.js b/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.prod.stories.js index dd6cb2c9a..71675ec9a 100644 --- a/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.prod.stories.js +++ b/collections/forms/src/SingleSelectA11yFieldFF/SingleSelectA11yFieldFF.prod.stories.js @@ -61,8 +61,7 @@ export const Default = () => ( ( export const InitialValue = () => ( ', () => {
', () => { props.filterable, PropTypes.string), + /** Allows to override what's rendered inside the `button[role="option"]`. + * Can be overriden on an individual option basis **/ + optionComponent: PropTypes.elementType, + /** For a11y: How aggressively the user should be updated about changes in options **/ optionUpdateStrategy: PropTypes.oneOf(['off', 'polite', 'assertive']), diff --git a/components/select/src/single-select-a11y-field/single-select-a11y-field.prod.stories.js b/components/select/src/single-select-a11y-field/single-select-a11y-field.prod.stories.js index 9d9aead89..6af9e0b72 100644 --- a/components/select/src/single-select-a11y-field/single-select-a11y-field.prod.stories.js +++ b/components/select/src/single-select-a11y-field/single-select-a11y-field.prod.stories.js @@ -40,7 +40,7 @@ export const Default = () => { return ( { return ( { return ( { return ( { return ( { return ( { return ( { return (
', () => { it('should forward props to the SingleSelectA11y component', () => { const options = [] - const customOption = () => null + const optionComponent = () => null const onChange = () => null const onBlur = () => null const onEndReached = () => null @@ -32,14 +32,14 @@ describe('', () => { ', () => { /> ) - expect(SingleSelectA11y.mock.calls[0][0].idPrefix).toBe('foo') + expect(SingleSelectA11y.mock.calls[0][0].name).toBe('foo') expect(SingleSelectA11y.mock.calls[0][0].options).toBe(options) expect(SingleSelectA11y.mock.calls[0][0].onChange).toBe(onChange) expect(SingleSelectA11y.mock.calls[0][0].autoFocus).toBe(false) expect(SingleSelectA11y.mock.calls[0][0].className).toBe('') expect(SingleSelectA11y.mock.calls[0][0].clearText).toBe('') expect(SingleSelectA11y.mock.calls[0][0].clearable).toBe(false) - expect(SingleSelectA11y.mock.calls[0][0].customOption).toBe( - customOption + expect(SingleSelectA11y.mock.calls[0][0].optionComponent).toBe( + optionComponent ) expect(SingleSelectA11y.mock.calls[0][0].dataTest).toBe('') expect(SingleSelectA11y.mock.calls[0][0].dense).toBe(false) @@ -122,7 +122,7 @@ describe('', () => { validationText="Validation text" valid={true} // Props required by SingleSelectA11y - idPrefix="foo" + name="foo" options={[]} onChange={() => null} /> diff --git a/components/select/src/single-select-a11y/__stories__/DefaultPosition.js b/components/select/src/single-select-a11y/__stories__/DefaultPosition.js index 9476342e4..2a2e88acc 100644 --- a/components/select/src/single-select-a11y/__stories__/DefaultPosition.js +++ b/components/select/src/single-select-a11y/__stories__/DefaultPosition.js @@ -3,7 +3,10 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const DefaultPosition = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return (
{ }} > option.value === value - ).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} />
diff --git a/components/select/src/single-select-a11y/__stories__/Dense.js b/components/select/src/single-select-a11y/__stories__/Dense.js index 1dbb334d4..c4b147a3f 100644 --- a/components/select/src/single-select-a11y/__stories__/Dense.js +++ b/components/select/src/single-select-a11y/__stories__/Dense.js @@ -3,19 +3,14 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const Dense = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/Empty.js b/components/select/src/single-select-a11y/__stories__/Empty.js index 21b873079..d365edb14 100644 --- a/components/select/src/single-select-a11y/__stories__/Empty.js +++ b/components/select/src/single-select-a11y/__stories__/Empty.js @@ -1,20 +1,14 @@ import React, { useState } from 'react' import { SingleSelectA11y } from '../single-select-a11y.js' -import { options } from './options.js' export const Empty = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={[]} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyNode.js b/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyNode.js index 7732ff5b1..206a53a32 100644 --- a/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyNode.js +++ b/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyNode.js @@ -1,9 +1,8 @@ import React, { useState } from 'react' import { SingleSelectA11y } from '../single-select-a11y.js' -import { options } from './options.js' export const EmptyWithEmptyNode = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const emptyNode = (
{ return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={[]} empty={emptyNode} /> diff --git a/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyText.js b/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyText.js index 703d9ba26..b3fcab017 100644 --- a/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyText.js +++ b/components/select/src/single-select-a11y/__stories__/EmptyWithEmptyText.js @@ -1,20 +1,14 @@ import React, { useState } from 'react' import { SingleSelectA11y } from '../single-select-a11y.js' -import { options } from './options.js' export const EmptyWithEmptyText = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={[]} empty="Custom empty text" /> diff --git a/components/select/src/single-select-a11y/__stories__/FlippedPosition.js b/components/select/src/single-select-a11y/__stories__/FlippedPosition.js index 6eea4f0c8..9d90e7d97 100644 --- a/components/select/src/single-select-a11y/__stories__/FlippedPosition.js +++ b/components/select/src/single-select-a11y/__stories__/FlippedPosition.js @@ -3,7 +3,10 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const FlippedPosition = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return (
{ }} > option.value === value - ).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} />
diff --git a/components/select/src/single-select-a11y/__stories__/InfiniteLoading.js b/components/select/src/single-select-a11y/__stories__/InfiniteLoading.js index 6744436fa..088303e98 100644 --- a/components/select/src/single-select-a11y/__stories__/InfiniteLoading.js +++ b/components/select/src/single-select-a11y/__stories__/InfiniteLoading.js @@ -25,11 +25,8 @@ const optionChunks = options.reduce((chunks, option, index) => { export const InfiniteLoading = () => { const [loading, setLoading] = useState(false) const [curLoadedPage, setCurLoadedPage] = useState(0) - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const [loadedOptions, setLoadedOptions] = useState(optionChunks[0]) - const valueLabel = value - ? loadedOptions.find((option) => option.value === value).label - : '' const loadNextOptions = useCallback(() => { const nextPage = curLoadedPage + 1 @@ -56,12 +53,11 @@ export const InfiniteLoading = () => { return ( setValue(nextValue)} + onChange={setSelected} options={loadedOptions} onEndReached={loadNextOptions} /> diff --git a/components/select/src/single-select-a11y/__stories__/Loading.js b/components/select/src/single-select-a11y/__stories__/Loading.js index 8e95a0b44..e149ec836 100644 --- a/components/select/src/single-select-a11y/__stories__/Loading.js +++ b/components/select/src/single-select-a11y/__stories__/Loading.js @@ -6,9 +6,8 @@ export const Loading = () => { return ( null} options={options} /> diff --git a/components/select/src/single-select-a11y/__stories__/ShiftedIntoView.js b/components/select/src/single-select-a11y/__stories__/ShiftedIntoView.js index a878b92db..3a1c36bb4 100644 --- a/components/select/src/single-select-a11y/__stories__/ShiftedIntoView.js +++ b/components/select/src/single-select-a11y/__stories__/ShiftedIntoView.js @@ -3,20 +3,17 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const ShiftedIntoView = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( <> option.value === value) - .label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> diff --git a/components/select/src/single-select-a11y/__stories__/WithClearButton.js b/components/select/src/single-select-a11y/__stories__/WithClearButton.js index e935c947f..994f5c2c2 100644 --- a/components/select/src/single-select-a11y/__stories__/WithClearButton.js +++ b/components/select/src/single-select-a11y/__stories__/WithClearButton.js @@ -3,19 +3,17 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithClearButton = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithCustomLowMaxHeight.js b/components/select/src/single-select-a11y/__stories__/WithCustomLowMaxHeight.js index 8472d346f..815584ea3 100644 --- a/components/select/src/single-select-a11y/__stories__/WithCustomLowMaxHeight.js +++ b/components/select/src/single-select-a11y/__stories__/WithCustomLowMaxHeight.js @@ -3,18 +3,13 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { options } from './options.js' export const WithCustomLowMaxHeight = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={options} menuMaxHeight="100px" /> diff --git a/components/select/src/single-select-a11y/__stories__/WithCustomOptions.js b/components/select/src/single-select-a11y/__stories__/WithCustomOptions.js index d1c649d4e..456c1b1d6 100644 --- a/components/select/src/single-select-a11y/__stories__/WithCustomOptions.js +++ b/components/select/src/single-select-a11y/__stories__/WithCustomOptions.js @@ -59,7 +59,7 @@ const CustomOption = ({ label }) => ( ) export const WithCustomOptions = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const optionsWithCustomStyle = useMemo(() => { return options.slice(0, 3).map((option) => ({ ...option, @@ -69,14 +69,9 @@ export const WithCustomOptions = () => { return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={optionsWithCustomStyle} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithDisabledOption.js b/components/select/src/single-select-a11y/__stories__/WithDisabledOption.js index 3a7702279..be6e90319 100644 --- a/components/select/src/single-select-a11y/__stories__/WithDisabledOption.js +++ b/components/select/src/single-select-a11y/__stories__/WithDisabledOption.js @@ -9,18 +9,16 @@ const fiveOptionsWithFourthDisabled = [ ] export const WithDisabledOption = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptionsWithFourthDisabled} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithFilterField.js b/components/select/src/single-select-a11y/__stories__/WithFilterField.js index b0f069b57..5013347e0 100644 --- a/components/select/src/single-select-a11y/__stories__/WithFilterField.js +++ b/components/select/src/single-select-a11y/__stories__/WithFilterField.js @@ -3,7 +3,7 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { options } from './options.js' export const WithFilterField = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const [filterValue, setFilterValue] = useState('') const filteredOptions = useMemo(() => { return filterValue @@ -14,16 +14,11 @@ export const WithFilterField = () => { : options }, [filterValue]) - const valueLabel = value - ? options.find((option) => option.value === value).label - : '' - return ( setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} filterable filterPlaceholder="Filter placeholder" filterValue={filterValue} diff --git a/components/select/src/single-select-a11y/__stories__/WithInitialFocus.js b/components/select/src/single-select-a11y/__stories__/WithInitialFocus.js index 736ded59d..c0f324be8 100644 --- a/components/select/src/single-select-a11y/__stories__/WithInitialFocus.js +++ b/components/select/src/single-select-a11y/__stories__/WithInitialFocus.js @@ -3,21 +3,15 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithInitialFocus = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( <> option.value === value) - .label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> diff --git a/components/select/src/single-select-a11y/__stories__/WithManyOptions.js b/components/select/src/single-select-a11y/__stories__/WithManyOptions.js index d14b38f70..b1493be53 100644 --- a/components/select/src/single-select-a11y/__stories__/WithManyOptions.js +++ b/components/select/src/single-select-a11y/__stories__/WithManyOptions.js @@ -3,18 +3,16 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { options } from './options.js' export const WithManyOptions = () => { - const [value, setValue] = useState('art_entry_point:_no_pmtct') + const [selected, setSelected] = useState({ + label: 'ART entry point: No PMTCT', + value: 'art_entry_point:_no_pmtct', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={options} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithNoMatchText.js b/components/select/src/single-select-a11y/__stories__/WithNoMatchText.js index 5acc1fd74..03c8c323b 100644 --- a/components/select/src/single-select-a11y/__stories__/WithNoMatchText.js +++ b/components/select/src/single-select-a11y/__stories__/WithNoMatchText.js @@ -2,15 +2,18 @@ import React, { useMemo, useState } from 'react' import { SingleSelectA11y } from '../single-select-a11y.js' export const WithNoMatchText = () => { - const [value, setValue] = useState('foo') + const [selected, setSelected] = useState({ + label: 'Foo', + value: 'foo', + }) const [filterValue, setFilterValue] = useState('Bar') const filteredOptions = useMemo(() => [], []) return ( setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} filterable filterPlaceholder="Filter placeholder" filterValue={filterValue} diff --git a/components/select/src/single-select-a11y/__stories__/WithOnBlur.js b/components/select/src/single-select-a11y/__stories__/WithOnBlur.js index 622eb9775..92018d7e7 100644 --- a/components/select/src/single-select-a11y/__stories__/WithOnBlur.js +++ b/components/select/src/single-select-a11y/__stories__/WithOnBlur.js @@ -3,22 +3,16 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithOnBlur = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const [blurTime, setBlurTime] = useState('') return ( <> setBlurTime(new Date().toISOString())} - idPrefix="a11y" - value={value} - valueLabel={ - value - ? fiveOptions.find((option) => option.value === value) - .label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> diff --git a/components/select/src/single-select-a11y/__stories__/WithOnFocus.js b/components/select/src/single-select-a11y/__stories__/WithOnFocus.js index e350f1e4d..6b3f41d25 100644 --- a/components/select/src/single-select-a11y/__stories__/WithOnFocus.js +++ b/components/select/src/single-select-a11y/__stories__/WithOnFocus.js @@ -3,22 +3,16 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithOnFocus = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const [focusTime, setFocusTime] = useState('') return ( <> setFocusTime(new Date().toISOString())} - idPrefix="a11y" - value={value} - valueLabel={ - value - ? fiveOptions.find((option) => option.value === value) - .label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> diff --git a/components/select/src/single-select-a11y/__stories__/WithOptionsAndDisabled.js b/components/select/src/single-select-a11y/__stories__/WithOptionsAndDisabled.js index acefcf4b2..56f8b1f98 100644 --- a/components/select/src/single-select-a11y/__stories__/WithOptionsAndDisabled.js +++ b/components/select/src/single-select-a11y/__stories__/WithOptionsAndDisabled.js @@ -3,19 +3,14 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithOptionsAndDisabled = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoading.js b/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoading.js index c453b2fd0..6af368d1a 100644 --- a/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoading.js +++ b/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoading.js @@ -9,19 +9,14 @@ const options = [ ] export const WithOptionsAndLoading = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={options} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoadingText.js b/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoadingText.js index b1ab26439..6aab8c078 100644 --- a/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoadingText.js +++ b/components/select/src/single-select-a11y/__stories__/WithOptionsAndLoadingText.js @@ -9,20 +9,15 @@ const options = [ ] export const WithOptionsAndLoadingText = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={options} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithPlaceholder.js b/components/select/src/single-select-a11y/__stories__/WithPlaceholder.js index 70d27f42a..95fb5a7e8 100644 --- a/components/select/src/single-select-a11y/__stories__/WithPlaceholder.js +++ b/components/select/src/single-select-a11y/__stories__/WithPlaceholder.js @@ -3,20 +3,15 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { options } from './options.js' export const WithPlaceholder = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) const withoutEmptyOptions = options.slice(1) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={withoutEmptyOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithPlaceholderAndSelection.js b/components/select/src/single-select-a11y/__stories__/WithPlaceholderAndSelection.js index b13999682..ba13213cb 100644 --- a/components/select/src/single-select-a11y/__stories__/WithPlaceholderAndSelection.js +++ b/components/select/src/single-select-a11y/__stories__/WithPlaceholderAndSelection.js @@ -3,19 +3,17 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithPlaceholderAndSelection = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithPrefix.js b/components/select/src/single-select-a11y/__stories__/WithPrefix.js index 57af47deb..889f9e12c 100644 --- a/components/select/src/single-select-a11y/__stories__/WithPrefix.js +++ b/components/select/src/single-select-a11y/__stories__/WithPrefix.js @@ -3,19 +3,14 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithPrefix = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithPrefixAndSelection.js b/components/select/src/single-select-a11y/__stories__/WithPrefixAndSelection.js index 9d6de8c2b..d6d10ced9 100644 --- a/components/select/src/single-select-a11y/__stories__/WithPrefixAndSelection.js +++ b/components/select/src/single-select-a11y/__stories__/WithPrefixAndSelection.js @@ -3,19 +3,17 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithPrefixAndSelection = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithRTL.js b/components/select/src/single-select-a11y/__stories__/WithRTL.js index 4fa224ab8..0eebabf52 100644 --- a/components/select/src/single-select-a11y/__stories__/WithRTL.js +++ b/components/select/src/single-select-a11y/__stories__/WithRTL.js @@ -3,7 +3,10 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithRTL = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) // as options are rendered in Portal, the body dir (of the iframe) needs to be set to 'rtl' useEffect(() => { @@ -16,15 +19,9 @@ export const WithRTL = () => { return (
option.value === value) - .label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} />
diff --git a/components/select/src/single-select-a11y/__stories__/WithSelection.js b/components/select/src/single-select-a11y/__stories__/WithSelection.js index db78e79dc..b6ff64a5f 100644 --- a/components/select/src/single-select-a11y/__stories__/WithSelection.js +++ b/components/select/src/single-select-a11y/__stories__/WithSelection.js @@ -3,18 +3,16 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithSelection = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithSelectionAndDisabled.js b/components/select/src/single-select-a11y/__stories__/WithSelectionAndDisabled.js index bb8c50732..b3a795e4f 100644 --- a/components/select/src/single-select-a11y/__stories__/WithSelectionAndDisabled.js +++ b/components/select/src/single-select-a11y/__stories__/WithSelectionAndDisabled.js @@ -3,19 +3,17 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithSelectionAndDisabled = () => { - const [value, setValue] = useState('anc_1st_visit') + const [selected, setSelected] = useState({ + label: 'ANC 1st visit', + value: 'anc_1st_visit', + }) return ( option.value === value).label - : '' - } - onChange={(nextValue) => setValue(nextValue)} + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js b/components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js new file mode 100644 index 000000000..f9c55fb94 --- /dev/null +++ b/components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js @@ -0,0 +1,16 @@ +import React, { useState } from 'react' +import { SingleSelectA11y } from '../single-select-a11y.js' + +export const WithoutOptionsAndLoading = () => { + const [selected, setSelected] = useState(null) + + return ( + + ) +} diff --git a/components/select/src/single-select-a11y/__stories__/WithoutSelection.js b/components/select/src/single-select-a11y/__stories__/WithoutSelection.js index aa17177f9..6d1abcff5 100644 --- a/components/select/src/single-select-a11y/__stories__/WithoutSelection.js +++ b/components/select/src/single-select-a11y/__stories__/WithoutSelection.js @@ -3,13 +3,14 @@ import { SingleSelectA11y } from '../single-select-a11y.js' import { fiveOptions } from './options.js' export const WithoutSelection = () => { - const [value, setValue] = useState('') + const [selected, setSelected] = useState(null) return ( setValue(nextValue)} + inputMaxHeight="50px" + name="a11y" + selected={selected} + onChange={setSelected} options={fiveOptions} /> ) diff --git a/components/select/src/single-select-a11y/features/keyboard-interactions.test.e2e.js b/components/select/src/single-select-a11y/features/keyboard-interactions.test.e2e.js index bfc6b396b..5213ce653 100644 --- a/components/select/src/single-select-a11y/features/keyboard-interactions.test.e2e.js +++ b/components/select/src/single-select-a11y/features/keyboard-interactions.test.e2e.js @@ -39,6 +39,8 @@ describe('', () => { cy.log( '**Then the first option on the currently displayed page is highlighted**' ) + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.findByRole('option', { selected: true }) .invoke('attr', 'aria-label') .should('equal', 'Select option 70') @@ -86,6 +88,8 @@ describe('', () => { cy.log( '**Then the first option on the currently displayed page is highlighted**' ) + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.findByRole('option', { selected: true }) .invoke('attr', 'aria-label') .should('equal', 'Select option 16') @@ -110,6 +114,8 @@ describe('', () => { }) cy.log('**Then the first option on the previous page is highlighted**') + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.findByRole('option', { selected: true }) .invoke('attr', 'aria-label') .should('equal', 'Select option 61') @@ -145,6 +151,8 @@ describe('', () => { }) cy.log('**Then the first option on the previous page is highlighted**') + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.findByRole('option', { selected: true }) .invoke('attr', 'aria-label') .should('equal', 'Select option 16') @@ -193,6 +201,8 @@ describe('', () => { }) cy.log('**Then the first option is being highlighted**') + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.all( () => cy.findAllByRole('option').invoke('get', 0), () => cy.findByRole('option', { selected: true }).invoke('get', 0) @@ -237,6 +247,8 @@ describe('', () => { }) cy.log('**Then the second option is being highlighted**') + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.all( () => cy.findAllByRole('option').invoke('get', 1), () => cy.findByRole('option', { selected: true }).invoke('get', 0) @@ -263,6 +275,8 @@ describe('', () => { cy.log( '**Then the last option on the currently displayed page is highlighted**' ) + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.all( () => cy.get('[role="option"]:visible').last().invoke('get', 0), () => cy.findByRole('option', { selected: true }).invoke('get', 0) @@ -299,6 +313,9 @@ describe('', () => { force: true, }) + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) + cy.log( '**Then the first enabled option after last option on the currently displayed page is highlighted**' ) @@ -309,14 +326,10 @@ describe('', () => { expect(highlightedOption).to.equal(eighteensOptions) }) - // For some reason, without the timeout, - // cypress will still get the previously visible page - // when using the `:visible` pseudo-selector - cy.wait(0) - cy.log( '**And the first enabled option after last option on the currently displayed page is the first visible option**' ) + cy.all( () => cy.get('[role="option"]:visible').invoke('get', 0), () => cy.findByRole('option', { selected: true }).invoke('get', 0) @@ -467,13 +480,14 @@ describe('', () => { ) for ( let i = 0; - i < 11; // This will bring us to the second last option exactly + i < 98; // This will bring us to the second last option exactly ++i ) { cy.findByRole('combobox').trigger('keydown', { - key: 'PageDown', + key: 'ArrowDown', force: true, }) + cy.wait(1) } cy.findAllByRole('option').eq(98).should('be.visible') @@ -487,6 +501,8 @@ describe('', () => { }) cy.log('**Then the last option is highlighted**') + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.all( () => cy.findAllByRole('option').last().invoke('get', 0), () => @@ -540,6 +556,8 @@ describe('', () => { }) cy.log('**Then the last option is highlighted**') + // For some reason, it takes a little time to change the highlighted option + cy.wait(1) cy.all( () => cy.findAllByRole('option').invoke('get', 98), () => diff --git a/components/select/src/single-select-a11y/features/menu-positioning.test.e2e.js b/components/select/src/single-select-a11y/features/menu-positioning.test.e2e.js index 5ea33c8aa..057bb9afc 100644 --- a/components/select/src/single-select-a11y/features/menu-positioning.test.e2e.js +++ b/components/select/src/single-select-a11y/features/menu-positioning.test.e2e.js @@ -68,7 +68,7 @@ describe('SingleSelectA11y: Menu positioning', () => { const selectedValueRect = $selectedValue.getBoundingClientRect() const menuRect = $menu.getBoundingClientRect() - expect(selectedValueRect.top).to.be.closeTo(menuRect.bottom, 1) + expect(selectedValueRect.top).to.be.closeTo(menuRect.bottom, 2) }) // And the left of the SingleSelect is aligned with the left of the anchor diff --git a/components/select/src/single-select-a11y/is-option-hidden.js b/components/select/src/single-select-a11y/is-option-hidden.js index f7035d0fe..073139cbf 100644 --- a/components/select/src/single-select-a11y/is-option-hidden.js +++ b/components/select/src/single-select-a11y/is-option-hidden.js @@ -1,3 +1,11 @@ +// If this many pixels are invisible, we still consider the option visible +// The reason we're doing this is because of JS' number issues, +// e.g: +// optionOffsetBottom is 345.6000061035156 +// containerOffsetBottom is 345.5999984741211 +// -> Without tolerance, option is invisible +const TOLERANCE = 2 + export function isOptionHidden(option, scrollContainer) { const optionOffsetTop = option.getBoundingClientRect().top const optionHeight = option.offsetHeight @@ -7,7 +15,7 @@ export function isOptionHidden(option, scrollContainer) { const containerOffsetBottom = containerOffsetTop + containerHeight return ( - optionOffsetBottom > containerOffsetBottom || - optionOffsetTop < containerOffsetTop + optionOffsetBottom > containerOffsetBottom + TOLERANCE || + optionOffsetTop < containerOffsetTop - TOLERANCE ) } diff --git a/components/select/src/single-select-a11y/menu/menu-filter.js b/components/select/src/single-select-a11y/menu/filter.js similarity index 90% rename from components/select/src/single-select-a11y/menu/menu-filter.js rename to components/select/src/single-select-a11y/menu/filter.js index 828ed0ec5..11aa20daf 100644 --- a/components/select/src/single-select-a11y/menu/menu-filter.js +++ b/components/select/src/single-select-a11y/menu/filter.js @@ -4,9 +4,9 @@ import PropTypes from 'prop-types' import React from 'react' import i18n from '../../locales/index.js' -export function MenuFilter({ +export function Filter({ + ariaControls, dataTest, - idPrefix, label, placeholder, tabIndex, @@ -19,7 +19,7 @@ export function MenuFilter({
@@ -33,6 +33,6 @@ export function MenuLoading({ message }) { ) } -MenuLoading.propTypes = { +Loading.propTypes = { message: PropTypes.string, } diff --git a/components/select/src/single-select-a11y/menu/menu.js b/components/select/src/single-select-a11y/menu/menu.js index 9d2506b28..476f16656 100644 --- a/components/select/src/single-select-a11y/menu/menu.js +++ b/components/select/src/single-select-a11y/menu/menu.js @@ -1,23 +1,22 @@ import { colors, elevations } from '@dhis2/ui-constants' import { Layer } from '@dhis2-ui/layer' import { Popper } from '@dhis2-ui/popper' -import cx from 'classnames' import PropTypes from 'prop-types' import React, { useEffect, useState } from 'react' import { optionProp } from '../shared-prop-types.js' import { Empty } from './empty.js' -import { MenuFilter } from './menu-filter.js' -import { MenuLoading } from './menu-loading.js' -import { MenuOptionsList } from './menu-options-list.js' +import { Filter } from './filter.js' +import { Loading } from './loading.js' import { NoMatch } from './no-match.js' +import { OptionsList } from './options-list.js' export function Menu({ comboBoxId, focussedOptionIndex, - idPrefix, + name, options, onChange, - customOption, + optionComponent, dataTest, disabled, empty, @@ -34,7 +33,7 @@ export function Menu({ noMatchText, optionUpdateStrategy, selectRef, - selected, + selectedValue, tabIndex, onBlur, onClose, @@ -42,12 +41,12 @@ export function Menu({ onFilterChange, onFilterInputKeyDown, }) { - const [menuWidth, setMenuWidth] = useState('auto') + const [menuWidth, setWidth] = useState('auto') const dataTestPrefix = `${dataTest}-menu` useEffect(() => { if (selectRef) { - const callback = () => setMenuWidth(`${selectRef.offsetWidth}px`) + const callback = () => setWidth(`${selectRef.offsetWidth}px`) callback() // We want to know the width as soon as the selectRef.addEventListener('resize', callback) @@ -75,17 +74,14 @@ export function Menu({ placement="bottom-start" observeReferenceResize > -