From 5fc1a56384ad2d57c9957856f1b323266f84ccd0 Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 3 Sep 2023 02:03:13 +0200 Subject: [PATCH 1/9] Correct keyboard navigation with multiple disabled options --- .../src/useAutocomplete/useAutocomplete.js | 28 +-- .../useAutocomplete/useAutocomplete.test.js | 203 ++++++++++++++++++ 2 files changed, 219 insertions(+), 12 deletions(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js index f81827e684d00f..3786579a608454 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js @@ -300,14 +300,6 @@ export function useAutocomplete(props) { let nextFocus = index; while (true) { - // Out of range - if ( - (direction === 'next' && nextFocus === filteredOptions.length) || - (direction === 'previous' && nextFocus === -1) - ) { - return -1; - } - const option = listboxRef.current.querySelector(`[data-option-index="${nextFocus}"]`); // Same logic as MenuList.js @@ -315,12 +307,24 @@ export function useAutocomplete(props) { ? false : !option || option.disabled || option.getAttribute('aria-disabled') === 'true'; - if ((option && !option.hasAttribute('tabindex')) || nextFocusDisabled) { - // Move to the next element. - nextFocus += direction === 'next' ? 1 : -1; - } else { + if (!(option && !option.hasAttribute('tabindex')) && !nextFocusDisabled) { + // The next option is available return nextFocus; } + + // The next option is disabled, move to the next element. + // with looped index + if (direction === 'next') { + nextFocus = (nextFocus + 1) % filteredOptions.length; + } else { + nextFocus = (nextFocus - 1 + filteredOptions.length) % filteredOptions.length; + } + + // We end up with initial index, that means we don't have avalable options. + // All of them are disabled + if (nextFocus === index) { + return -1; + } } } diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js index 5ae4fc1a476188..a774bb95d039f7 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js @@ -49,6 +49,209 @@ describe('useAutocomplete', () => { expect(barOptionAsFirst).to.equal(barOptionAsSecond); }); + describe('Navigation via keyboard', () => { + function Test(props) { + const [value, setValue] = React.useState(); + const { options } = props; + const { + groupedOptions, + getRootProps, + getInputLabelProps, + getInputProps, + getListboxProps, + getOptionProps, + } = useAutocomplete({ + options, + open: true, + value, + onChange: (_, option) => setValue(option.value), + isOptionEqualToValue: (option, _value) => option.value === _value, + getOptionDisabled: (option) => option.disabled, + }); + + return ( +
+
+ + +
+ {groupedOptions.length > 0 ? ( + + ) : null} +
+ ); + } + + it('Should navigate in cicle', () => { + const options = [ + { label: 'foo', value: 1, disabled: false }, + { label: 'bar', value: 2, disabled: false }, + { label: 'baz', value: 3, disabled: false }, + ]; + + const { getByRole } = render(); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + const optionElements = screen.getAllByRole('option'); + expect(optionElements[0]).to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + }); + + it('Should skip disabled options', () => { + const options = [ + { label: 'foo', value: 1, disabled: false }, + { label: 'bar', value: 2, disabled: true }, + { label: 'baz', value: 3, disabled: false }, + ]; + + const { getByRole } = render(); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + const optionElements = screen.getAllByRole('option'); + expect(optionElements[0]).to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + }); + + it('Should skip disabled options in the end of list', () => { + const options = [ + { label: 'foo', value: 1, disabled: false }, + { label: 'bar', value: 2, disabled: false }, + { label: 'baz', value: 3, disabled: true }, + { label: 'qux', value: 4, disabled: true }, + ]; + + const { getByRole } = render(); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + const optionElements = screen.getAllByRole('option'); + expect(optionElements[0]).to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + }); + + it('Should skip first disabled option and in the end of list', () => { + const options = [ + { label: 'foo', value: 1, disabled: true }, + { label: 'bar', value: 2, disabled: false }, + { label: 'baz', value: 3, disabled: false }, + { label: 'qux', value: 4, disabled: true }, + ]; + + const { getByRole } = render(); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + const optionElements = screen.getAllByRole('option'); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowUp' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).to.have.class('Mui-focused'); + expect(optionElements[3]).not.to.have.class('Mui-focused'); + }); + + it('Should ignore the list with disabled options', () => { + const options = [ + { label: 'foo', value: 1, disabled: true }, + { label: 'bar', value: 2, disabled: true }, + { label: 'baz', value: 3, disabled: true }, + ]; + + const { getByRole } = render(); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + const optionElements = screen.getAllByRole('option'); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + + fireEvent.keyDown(textbox, { key: 'ArrowUp' }); + expect(optionElements[0]).not.to.have.class('Mui-focused'); + expect(optionElements[1]).not.to.have.class('Mui-focused'); + expect(optionElements[2]).not.to.have.class('Mui-focused'); + }); + }); + describe('createFilterOptions', () => { it('defaults to getOptionLabel for text filtering', () => { const filterOptions = createFilterOptions(); From b9530def2b9b076310655339fdc937190cea26d3 Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 3 Sep 2023 15:04:06 +0200 Subject: [PATCH 2/9] fix a typo in the test case --- packages/mui-base/src/useAutocomplete/useAutocomplete.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js index a774bb95d039f7..aff4561158f650 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js @@ -184,7 +184,7 @@ describe('useAutocomplete', () => { expect(optionElements[3]).not.to.have.class('Mui-focused'); }); - it('Should skip first disabled option and in the end of list', () => { + it('Should skip disabled options in the start and in the end of the list', () => { const options = [ { label: 'foo', value: 1, disabled: true }, { label: 'bar', value: 2, disabled: false }, From 8871dac2b1fb1f7c820c51a173eea4901b86a55d Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 3 Sep 2023 17:33:22 +0200 Subject: [PATCH 3/9] Fix unit tests, checking correct initial index --- packages/mui-base/src/useAutocomplete/useAutocomplete.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js index 3786579a608454..7dc184f7f95ad7 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js @@ -293,7 +293,7 @@ export function useAutocomplete(props) { }, [value, multiple, focusedTag, focusTag]); function validOptionIndex(index, direction) { - if (!listboxRef.current || index === -1) { + if (!listboxRef.current || index < 0 || index >= filteredOptions.length) { return -1; } From c238fa3e6e17e9479c245e9a65a659b26101932b Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 3 Sep 2023 17:34:08 +0200 Subject: [PATCH 4/9] Refactor, fixed condition --- packages/mui-base/src/useAutocomplete/useAutocomplete.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js index 7dc184f7f95ad7..e2ca22f9d628fa 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js @@ -307,7 +307,7 @@ export function useAutocomplete(props) { ? false : !option || option.disabled || option.getAttribute('aria-disabled') === 'true'; - if (!(option && !option.hasAttribute('tabindex')) && !nextFocusDisabled) { + if ((option && option.hasAttribute('tabindex')) && !nextFocusDisabled) { // The next option is available return nextFocus; } From 92eb07582b872b55a17c3d5a59a1803c1e093faa Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 3 Sep 2023 17:52:36 +0200 Subject: [PATCH 5/9] fixed code formatting --- packages/mui-base/src/useAutocomplete/useAutocomplete.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js index e2ca22f9d628fa..e886114be8db78 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js @@ -307,7 +307,7 @@ export function useAutocomplete(props) { ? false : !option || option.disabled || option.getAttribute('aria-disabled') === 'true'; - if ((option && option.hasAttribute('tabindex')) && !nextFocusDisabled) { + if (option && option.hasAttribute('tabindex') && !nextFocusDisabled) { // The next option is available return nextFocus; } From ae5ff944a355b01a6b51437a62f6ae035e11a922 Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 8 Oct 2023 23:24:51 +0200 Subject: [PATCH 6/9] Moving tests to the Autocomplete component --- .../useAutocomplete/useAutocomplete.test.js | 203 ------------------ .../src/Autocomplete/Autocomplete.test.js | 89 ++++++++ 2 files changed, 89 insertions(+), 203 deletions(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js index 59a3893aa82554..0386731e59eeb8 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.test.js @@ -49,209 +49,6 @@ describe('useAutocomplete', () => { expect(barOptionAsFirst).to.equal(barOptionAsSecond); }); - describe('Navigation via keyboard', () => { - function Test(props) { - const [value, setValue] = React.useState(); - const { options } = props; - const { - groupedOptions, - getRootProps, - getInputLabelProps, - getInputProps, - getListboxProps, - getOptionProps, - } = useAutocomplete({ - options, - open: true, - value, - onChange: (_, option) => setValue(option.value), - isOptionEqualToValue: (option, _value) => option.value === _value, - getOptionDisabled: (option) => option.disabled, - }); - - return ( -
-
- - -
- {groupedOptions.length > 0 ? ( -
    - {groupedOptions.map((option, index) => { - return
  • {option.label}
    • ; - })} -
- ) : null} -
- ); - } - - it('Should navigate in cicle', () => { - const options = [ - { label: 'foo', value: 1, disabled: false }, - { label: 'bar', value: 2, disabled: false }, - { label: 'baz', value: 3, disabled: false }, - ]; - - const { getByRole } = render(); - const textbox = getByRole('combobox'); - - act(() => { - textbox.focus(); - }); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - const optionElements = screen.getAllByRole('option'); - expect(optionElements[0]).to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - }); - - it('Should skip disabled options', () => { - const options = [ - { label: 'foo', value: 1, disabled: false }, - { label: 'bar', value: 2, disabled: true }, - { label: 'baz', value: 3, disabled: false }, - ]; - - const { getByRole } = render(); - const textbox = getByRole('combobox'); - - act(() => { - textbox.focus(); - }); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - const optionElements = screen.getAllByRole('option'); - expect(optionElements[0]).to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - }); - - it('Should skip disabled options in the end of list', () => { - const options = [ - { label: 'foo', value: 1, disabled: false }, - { label: 'bar', value: 2, disabled: false }, - { label: 'baz', value: 3, disabled: true }, - { label: 'qux', value: 4, disabled: true }, - ]; - - const { getByRole } = render(); - const textbox = getByRole('combobox'); - - act(() => { - textbox.focus(); - }); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - const optionElements = screen.getAllByRole('option'); - expect(optionElements[0]).to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - }); - - it('Should skip disabled options in the start and in the end of the list', () => { - const options = [ - { label: 'foo', value: 1, disabled: true }, - { label: 'bar', value: 2, disabled: false }, - { label: 'baz', value: 3, disabled: false }, - { label: 'qux', value: 4, disabled: true }, - ]; - - const { getByRole } = render(); - const textbox = getByRole('combobox'); - - act(() => { - textbox.focus(); - }); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - const optionElements = screen.getAllByRole('option'); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowUp' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).to.have.class('Mui-focused'); - expect(optionElements[3]).not.to.have.class('Mui-focused'); - }); - - it('Should ignore the list with disabled options', () => { - const options = [ - { label: 'foo', value: 1, disabled: true }, - { label: 'bar', value: 2, disabled: true }, - { label: 'baz', value: 3, disabled: true }, - ]; - - const { getByRole } = render(); - const textbox = getByRole('combobox'); - - act(() => { - textbox.focus(); - }); - - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); - const optionElements = screen.getAllByRole('option'); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - - fireEvent.keyDown(textbox, { key: 'ArrowUp' }); - expect(optionElements[0]).not.to.have.class('Mui-focused'); - expect(optionElements[1]).not.to.have.class('Mui-focused'); - expect(optionElements[2]).not.to.have.class('Mui-focused'); - }); - }); - describe('createFilterOptions', () => { it('defaults to getOptionLabel for text filtering', () => { const filterOptions = createFilterOptions(); diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index c2a42e332e7cf3..b8cdc8567f1179 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -823,6 +823,95 @@ describe('', () => { expect(handleSubmit.callCount).to.equal(0); expect(handleChange.callCount).to.equal(1); }); + + it('Should skip disabled options when navigatin via keyboard', () => { + const { getByRole } = render( + option === 'two'} + openOnFocus + options={['one', 'two', 'three']} + />, + ); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'one'); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'three'); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'one'); + }); + + it('Should skip disabled options at the end of the list when navigatin via keyboard', () => { + const { getByRole } = render( + option === 'three' || option === 'four'} + openOnFocus + options={['one', 'two', 'three', 'four']} + />, + ); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'one'); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'two'); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'one'); + }); + + it('Should skip the first disabled option and disabled options at the end of the list when navigatin via keyboard', () => { + const { getByRole } = render( + option === 'one' || option === 'five'} + openOnFocus + options={['one', 'two', 'three', 'four', 'five']} + />, + ); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'two'); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'four'); + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), 'two'); + fireEvent.keyDown(textbox, { key: 'ArrowUp' }); + checkHighlightIs(getByRole('listbox'), 'four'); + }); + + it("Shouldn't focus any option when all options are disabled", () => { + const { getByRole } = render( + true} + openOnFocus + options={['one', 'two', 'three']} + />, + ); + const textbox = getByRole('combobox'); + + act(() => { + textbox.focus(); + }); + + fireEvent.keyDown(textbox, { key: 'ArrowDown' }); + checkHighlightIs(getByRole('listbox'), null); + fireEvent.keyDown(textbox, { key: 'ArrowUp' }); + checkHighlightIs(getByRole('listbox'), null); + }); }); describe('WAI-ARIA conforming markup', () => { From 111a9d5b1dbecf7959b4d021cccbece0e0c826af Mon Sep 17 00:00:00 2001 From: Vadim Date: Sun, 8 Oct 2023 23:28:45 +0200 Subject: [PATCH 7/9] Fixing typos --- packages/mui-material/src/Autocomplete/Autocomplete.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index b8cdc8567f1179..34898508914483 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -824,7 +824,7 @@ describe('', () => { expect(handleChange.callCount).to.equal(1); }); - it('Should skip disabled options when navigatin via keyboard', () => { + it('Should skip disabled options when navigating via keyboard', () => { const { getByRole } = render( option === 'two'} @@ -846,7 +846,7 @@ describe('', () => { checkHighlightIs(getByRole('listbox'), 'one'); }); - it('Should skip disabled options at the end of the list when navigatin via keyboard', () => { + it('Should skip disabled options at the end of the list when navigating via keyboard', () => { const { getByRole } = render( option === 'three' || option === 'four'} @@ -868,7 +868,7 @@ describe('', () => { checkHighlightIs(getByRole('listbox'), 'one'); }); - it('Should skip the first disabled option and disabled options at the end of the list when navigatin via keyboard', () => { + it('Should skip the first disabled option and disabled options at the end of the list when navigating via keyboard', () => { const { getByRole } = render( option === 'one' || option === 'five'} From 6f7ea300dda690be385ce08283630ef088cc4a14 Mon Sep 17 00:00:00 2001 From: Vadim Date: Sat, 14 Oct 2023 19:13:00 +0200 Subject: [PATCH 8/9] Added required prop in tests components --- packages/mui-material/src/Autocomplete/Autocomplete.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index 34898508914483..94271a84ee5e10 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -830,6 +830,7 @@ describe('', () => { getOptionDisabled={(option) => option === 'two'} openOnFocus options={['one', 'two', 'three']} + renderInput={(props) => } />, ); const textbox = getByRole('combobox'); @@ -852,6 +853,7 @@ describe('', () => { getOptionDisabled={(option) => option === 'three' || option === 'four'} openOnFocus options={['one', 'two', 'three', 'four']} + renderInput={(props) => } />, ); const textbox = getByRole('combobox'); @@ -874,6 +876,7 @@ describe('', () => { getOptionDisabled={(option) => option === 'one' || option === 'five'} openOnFocus options={['one', 'two', 'three', 'four', 'five']} + renderInput={(props) => } />, ); const textbox = getByRole('combobox'); @@ -899,6 +902,7 @@ describe('', () => { getOptionDisabled={() => true} openOnFocus options={['one', 'two', 'three']} + renderInput={(props) => } />, ); const textbox = getByRole('combobox'); From 04c378287910b97f8431f47a34beaea7c3e6fba9 Mon Sep 17 00:00:00 2001 From: Vadim Date: Tue, 17 Oct 2023 21:16:02 +0200 Subject: [PATCH 9/9] Tests fixes after code review --- .../src/useAutocomplete/useAutocomplete.js | 2 +- .../src/Autocomplete/Autocomplete.test.js | 24 ++++--------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js index ba1c699d2eb7a0..ce64b6a3a6ba6c 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js @@ -320,7 +320,7 @@ export function useAutocomplete(props) { nextFocus = (nextFocus - 1 + filteredOptions.length) % filteredOptions.length; } - // We end up with initial index, that means we don't have avalable options. + // We end up with initial index, that means we don't have available options. // All of them are disabled if (nextFocus === index) { return -1; diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index 94271a84ee5e10..bad04f03fc7fb0 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -824,7 +824,7 @@ describe('', () => { expect(handleChange.callCount).to.equal(1); }); - it('Should skip disabled options when navigating via keyboard', () => { + it('should skip disabled options when navigating via keyboard', () => { const { getByRole } = render( option === 'two'} @@ -835,10 +835,6 @@ describe('', () => { ); const textbox = getByRole('combobox'); - act(() => { - textbox.focus(); - }); - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); checkHighlightIs(getByRole('listbox'), 'one'); fireEvent.keyDown(textbox, { key: 'ArrowDown' }); @@ -847,7 +843,7 @@ describe('', () => { checkHighlightIs(getByRole('listbox'), 'one'); }); - it('Should skip disabled options at the end of the list when navigating via keyboard', () => { + it('should skip disabled options at the end of the list when navigating via keyboard', () => { const { getByRole } = render( option === 'three' || option === 'four'} @@ -858,10 +854,6 @@ describe('', () => { ); const textbox = getByRole('combobox'); - act(() => { - textbox.focus(); - }); - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); checkHighlightIs(getByRole('listbox'), 'one'); fireEvent.keyDown(textbox, { key: 'ArrowDown' }); @@ -870,7 +862,7 @@ describe('', () => { checkHighlightIs(getByRole('listbox'), 'one'); }); - it('Should skip the first disabled option and disabled options at the end of the list when navigating via keyboard', () => { + it('should skip the first and last disabled options in the list when navigating via keyboard', () => { const { getByRole } = render( option === 'one' || option === 'five'} @@ -881,10 +873,6 @@ describe('', () => { ); const textbox = getByRole('combobox'); - act(() => { - textbox.focus(); - }); - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); checkHighlightIs(getByRole('listbox'), 'two'); fireEvent.keyDown(textbox, { key: 'ArrowDown' }); @@ -896,7 +884,7 @@ describe('', () => { checkHighlightIs(getByRole('listbox'), 'four'); }); - it("Shouldn't focus any option when all options are disabled", () => { + it('should not focus any option when all the options are disabled', () => { const { getByRole } = render( true} @@ -907,10 +895,6 @@ describe('', () => { ); const textbox = getByRole('combobox'); - act(() => { - textbox.focus(); - }); - fireEvent.keyDown(textbox, { key: 'ArrowDown' }); checkHighlightIs(getByRole('listbox'), null); fireEvent.keyDown(textbox, { key: 'ArrowUp' });