From 0dc37607135a460146677196057949871bae1711 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} | 13 +- .../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 | 102 +++++----- .../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 +- 61 files changed, 585 insertions(+), 707 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} (84%) 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..e1483bf3c 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-28T02:02:47.011Z\n" +"PO-Revision-Date: 2024-11-28T02:02:47.011Z\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 84% 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..1219f446f 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({ onKeyDown(e)} /> + { + /* @TODO: Put into the PR's description & create Jira ticket (maybe) */ '' + } + + ) +} + +ClearButton.propTypes = { + clearText: PropTypes.string.isRequired, + onClear: PropTypes.func.isRequired, + dataTest: PropTypes.string, +} + +function ClearButtonWithTooltip({ onClear, clearText, dataTest }) { + const clearButton = ( + + ) + + if (!clearText) { + return clearButton + } + + return ( + + {clearButton} + + ) +} + +ClearButtonWithTooltip.propTypes = { + clearText: PropTypes.string.isRequired, + onClear: PropTypes.func.isRequired, + dataTest: PropTypes.string, +} + +export { ClearButtonWithTooltip as ClearButton } diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-container.js b/components/select/src/single-select-a11y/selected-value/container.js similarity index 94% rename from components/select/src/single-select-a11y/selected-value/selected-value-container.js rename to components/select/src/single-select-a11y/selected-value/container.js index cb7886a46..b0cf4d6d5 100644 --- a/components/select/src/single-select-a11y/selected-value/selected-value-container.js +++ b/components/select/src/single-select-a11y/selected-value/container.js @@ -3,11 +3,10 @@ import cx from 'classnames' import PropTypes from 'prop-types' import React, { forwardRef, useCallback, useEffect } from 'react' -export const SelectedValueContainer = forwardRef(function Container( +export const Container = forwardRef(function Container( { children, comboBoxId, - idPrefix, autoFocus, dataTest, dense, @@ -15,6 +14,7 @@ export const SelectedValueContainer = forwardRef(function Container( error, expanded, labelledBy, + name, placeholder, tabIndex, valid, @@ -53,7 +53,7 @@ export const SelectedValueContainer = forwardRef(function Container( className={cx({ error, warning, valid, disabled, dense })} data-test={dataTest} ref={ref} - aria-controls={`${idPrefix}-listbox`} + aria-controls={`${name}-listbox`} aria-expanded={expanded.toString()} aria-haspopup="listbox" aria-labelledby={labelledBy} @@ -124,10 +124,10 @@ export const SelectedValueContainer = forwardRef(function Container( ) }) -SelectedValueContainer.propTypes = { +Container.propTypes = { children: PropTypes.any.isRequired, comboBoxId: PropTypes.string.isRequired, - idPrefix: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, onKeyDown: PropTypes.func.isRequired, autoFocus: PropTypes.bool, dataTest: PropTypes.string, diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js b/components/select/src/single-select-a11y/selected-value/placeholder.js similarity index 73% rename from components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js rename to components/select/src/single-select-a11y/selected-value/placeholder.js index ddd43f7b1..103a70f28 100644 --- a/components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js +++ b/components/select/src/single-select-a11y/selected-value/placeholder.js @@ -2,15 +2,7 @@ import { colors } from '@dhis2/ui-constants' import PropTypes from 'prop-types' import React from 'react' -export const SelectedValuePlaceholder = ({ - placeholder, - className, - dataTest, -}) => { - if (!placeholder) { - return null - } - +export function Placeholder({ placeholder, className, dataTest }) { return (
{placeholder} @@ -25,7 +17,7 @@ export const SelectedValuePlaceholder = ({ ) } -SelectedValuePlaceholder.propTypes = { +Placeholder.propTypes = { dataTest: PropTypes.string.isRequired, className: PropTypes.string, placeholder: PropTypes.string, diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-prefix.js b/components/select/src/single-select-a11y/selected-value/prefix.js similarity index 85% rename from components/select/src/single-select-a11y/selected-value/selected-value-prefix.js rename to components/select/src/single-select-a11y/selected-value/prefix.js index 332699147..1a71c426a 100644 --- a/components/select/src/single-select-a11y/selected-value/selected-value-prefix.js +++ b/components/select/src/single-select-a11y/selected-value/prefix.js @@ -2,7 +2,7 @@ import { colors, spacers } from '@dhis2/ui-constants' import PropTypes from 'prop-types' import React from 'react' -export function SelectedValuePrefix({ prefix, className, dataTest }) { +export function Prefix({ prefix, className, dataTest }) { return (
{prefix} @@ -19,7 +19,7 @@ export function SelectedValuePrefix({ prefix, className, dataTest }) { ) } -SelectedValuePrefix.propTypes = { +Prefix.propTypes = { dataTest: PropTypes.string.isRequired, className: PropTypes.string, prefix: PropTypes.string, diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js b/components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js deleted file mode 100644 index 06c17a702..000000000 --- a/components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js +++ /dev/null @@ -1,86 +0,0 @@ -import { colors, theme } from '@dhis2/ui-constants' -import { Tooltip } from '@dhis2-ui/tooltip' -import PropTypes from 'prop-types' -import React from 'react' -import i18n from '../../locales/index.js' - -export const ClearButton = ({ onClear, clearText, dataTest }) => ( - -) - -ClearButton.propTypes = { - clearText: PropTypes.string.isRequired, - onClear: PropTypes.func.isRequired, - dataTest: PropTypes.string, -} - -export const SelectedValueClearButton = ({ onClear, clearText, dataTest }) => { - const clearButton = ( - - ) - - if (!clearText) { - return clearButton - } - - return ( - - {clearButton} - - ) -} - -SelectedValueClearButton.propTypes = { - clearText: PropTypes.string.isRequired, - onClear: PropTypes.func.isRequired, - dataTest: PropTypes.string, -} diff --git a/components/select/src/single-select-a11y/selected-value/selected-value.js b/components/select/src/single-select-a11y/selected-value/selected-value.js index 19da6c378..19b64bd8a 100644 --- a/components/select/src/single-select-a11y/selected-value/selected-value.js +++ b/components/select/src/single-select-a11y/selected-value/selected-value.js @@ -1,16 +1,16 @@ import { IconChevronDown16 } from '@dhis2/ui-icons' import PropTypes from 'prop-types' import React from 'react' -import { SelectedValueClearButton } from './selected-value-clear-button.js' -import { SelectedValueContainer } from './selected-value-container.js' -import { SelectedValuePlaceholder } from './selected-value-placeholder.js' -import { SelectedValuePrefix } from './selected-value-prefix.js' +import { ClearButton } from './clear-button.js' +import { Container } from './container.js' +import { Placeholder } from './placeholder.js' +import { Prefix } from './prefix.js' export function SelectedValue({ clearText, comboBoxId, - idPrefix, - valueLabel, + name, + selectedLabel, onKeyDown, autoFocus, clearable, @@ -21,6 +21,7 @@ export function SelectedValue({ error, expanded, hasSelection, + inputMaxHeight, labelledBy, placeholder, prefix, @@ -32,22 +33,20 @@ export function SelectedValue({ onClick, onFocus, }) { - // @TODO - const inputMaxHeight = '300px' const dataTestPrefix = `${dataTest}-selectedvalue` return ( - {prefix && ( - + )}
- {!valueLabel && !prefix && ( - )} - {valueLabel} + {selectedLabel}
{hasSelection && clearable && !disabled && (
- - + ) } SelectedValue.propTypes = { clearText: PropTypes.string.isRequired, comboBoxId: PropTypes.string.isRequired, - idPrefix: PropTypes.string.isRequired, - valueLabel: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, + selectedLabel: PropTypes.string.isRequired, onKeyDown: PropTypes.func.isRequired, autoFocus: PropTypes.bool, clearable: PropTypes.bool, @@ -135,6 +131,7 @@ SelectedValue.propTypes = { error: PropTypes.bool, expanded: PropTypes.bool, hasSelection: PropTypes.bool, + inputMaxHeight: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), labelledBy: PropTypes.string, placeholder: PropTypes.string, prefix: PropTypes.string, diff --git a/components/select/src/single-select-a11y/single-select-a11y.e2e.stories.js b/components/select/src/single-select-a11y/single-select-a11y.e2e.stories.js index 318e68f27..b156bd10f 100644 --- a/components/select/src/single-select-a11y/single-select-a11y.e2e.stories.js +++ b/components/select/src/single-select-a11y/single-select-a11y.e2e.stories.js @@ -101,8 +101,11 @@ const fiveOptions = options.slice(0, 5) export const DefaultPosition = () => ( null} options={fiveOptions} /> @@ -111,8 +114,11 @@ export const DefaultPosition = () => ( export const FlippedPosition = () => ( <> null} options={options} /> @@ -135,8 +141,11 @@ export const FlippedPosition = () => ( export const ShiftedIntoView = () => ( <> null} options={options} /> @@ -157,7 +166,10 @@ export const ShiftedIntoView = () => ( ) export const HundretOptions = () => { - const [value, setValue] = useState('0') + const [selected, setSelected] = useState({ + value: '0', + label: `Select option 0`, + }) const [hundretOptions] = useState( Array.apply(null, Array(100)).map((x, i) => ({ value: `${i}`, @@ -167,16 +179,19 @@ export const HundretOptions = () => { return ( ) } export const HundretOptionsWithDisabled = () => { - const [value, setValue] = useState('10') + const [selected, setSelected] = useState({ + value: '10', + label: `Select option 10`, + }) const [hundretOptions] = useState( Array.apply(null, Array(100)).map((x, i) => ({ value: `${i}`, @@ -192,43 +207,10 @@ export const HundretOptionsWithDisabled = () => { return ( ) } - -export const NativeSelect = () => { - const [value, setValue] = useState('0') - const [hundretOptions] = useState( - Array.apply(null, Array(100)).map((x, i) => ({ - value: `${i}`, - label: `Select option ${i}`, - disabled: i > 19 && i < 30, - })) - ) - - return ( - <> - - - - - ) -} diff --git a/components/select/src/single-select-a11y/single-select-a11y.js b/components/select/src/single-select-a11y/single-select-a11y.js index 1fe43f274..55d9172f5 100644 --- a/components/select/src/single-select-a11y/single-select-a11y.js +++ b/components/select/src/single-select-a11y/single-select-a11y.js @@ -8,7 +8,7 @@ import { Menu } from './menu/index.js' import { SelectedValue } from './selected-value/index.js' import { optionProp } from './shared-prop-types.js' import { - useHandleKeyPress, + useHandleKeyPressOnCombobox, useHandleKeyPressOnFilterInput, } from './use-handle-key-press/index.js' @@ -53,14 +53,14 @@ function useFocussedOptionIndex({ filterable, filterValue, options }) { } export function SingleSelectA11y({ + name, options, - idPrefix, onChange, autoFocus = false, className = '', clearText: _clearText = '', clearable = false, - customOption = undefined, + optionComponent = undefined, dataTest = 'dhis2-singleselecta11y', dense = false, disabled = false, @@ -71,6 +71,7 @@ export function SingleSelectA11y({ filterPlaceholder: _filterPlaceholder = '', filterValue = '', filterable = false, + inputMaxHeight = '', labelledBy = '', loading = false, menuLoadingText: _menuLoadingText = '', @@ -79,12 +80,12 @@ export function SingleSelectA11y({ optionUpdateStrategy = 'polite', placeholder = '', prefix = '', + selected = { label: '', value: '' }, tabIndex = '0', valid = false, - value = '', warning = false, - valueLabel: _valueLabel = '', onBlur = () => undefined, + onClear = () => undefined, onEndReached = () => undefined, onFilterChange = () => undefined, onFocus = () => undefined, @@ -98,23 +99,7 @@ export function SingleSelectA11y({ const menuLoadingText = _menuLoadingText || i18n.t('Loading options') const noMatchText = _noMatchText || i18n.t('No options found') - const comboBoxId = `${idPrefix}-combo` - const valueLabel = - _valueLabel || - options.find((option) => option.value === value)?.label || - '' - - if ( - value && - !valueLabel && - options.length && - !options.find((option) => option.value === '') && - !placeholder - ) { - throw new Error( - 'You must either provide a "valueLabel" or include an empty option in the options array' - ) - } + const comboBoxId = `${name}-combo` const comboBoxRef = useRef() const listBoxRef = useRef() @@ -128,16 +113,20 @@ export function SingleSelectA11y({ const [selectRef, setSelectRef] = useState() const [expanded, setExpanded] = useState(false) const closeMenu = useCallback(() => setExpanded(false), []) + + const selectedValue = selected?.value || '' + const selectedLabel = selected?.label || '' const openMenu = useCallback(() => { const selectedOptionIndex = options.findIndex( - (option) => option.value === value + (option) => option.value === selectedValue ) if (selectedOptionIndex !== focussedOptionIndex) { setFocussedOptionIndex(selectedOptionIndex) } setExpanded(true) - }, [options, value, focussedOptionIndex, setFocussedOptionIndex]) + }, [options, selectedValue, focussedOptionIndex, setFocussedOptionIndex]) + const toggleMenu = useCallback(() => { if (expanded) { closeMenu() @@ -150,7 +139,7 @@ export function SingleSelectA11y({ const option = options[focussedOptionIndex] if (option) { - onChange(option.value) + onChange(option) } }, [focussedOptionIndex, options, onChange]) @@ -159,8 +148,8 @@ export function SingleSelectA11y({ [comboBoxRef] ) - const handleKeyDown = useHandleKeyPress({ - value, + const handleKeyDown = useHandleKeyPressOnCombobox({ + value: selectedValue, disabled, onChange, expanded, @@ -175,7 +164,7 @@ export function SingleSelectA11y({ }) const handleKeyDownOnFilterInput = useHandleKeyPressOnFilterInput({ - value, + value: selectedValue, options, loading, closeMenu, @@ -217,19 +206,18 @@ export function SingleSelectA11y({ disabled={disabled} error={error} expanded={expanded} - hasSelection={!!value} - idPrefix={idPrefix} + hasSelection={!!selectedValue} + inputMaxHeight={inputMaxHeight} labelledBy={labelledBy} - options={options} + name={name} placeholder={placeholder} prefix={prefix} tabIndex={tabIndex.toString()} - value={value} warning={warning} valid={valid} - valueLabel={valueLabel} + selectedLabel={selectedLabel} onBlur={onBlur} - onClear={() => onChange('')} + onClear={onClear} onClick={toggleMenu} onFocus={onFocus} onKeyDown={handleKeyDown} @@ -237,7 +225,7 @@ export function SingleSelectA11y({