From dabe784b39b8afecc1a2f913142fd4862fa67435 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Fri, 15 Mar 2024 11:51:27 -0400 Subject: [PATCH 01/12] fix(ListBox): update focus states for listbox components --- .../components/MultiSelect/FilterableMultiSelect.js | 9 +++++++++ .../react/src/components/MultiSelect/MultiSelect.tsx | 8 +++++++- .../styles/scss/components/combo-box/_combo-box.scss | 11 +++++++++++ .../styles/scss/components/dropdown/_dropdown.scss | 4 ++++ .../styles/scss/components/list-box/_list-box.scss | 2 +- .../scss/components/multiselect/_multiselect.scss | 10 ++++++++++ 6 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index b9d50bf50265..ca26fcf857c4 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -173,6 +173,15 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( break; case stateChangeTypes.keyDownEscape: handleOnMenuChange(false); + setHighlightedIndex(0); + break; + case stateChangeTypes.changeInput: + setHighlightedIndex(0); + break; + case stateChangeTypes.keyDownEnter: + if (!isOpen) { + setHighlightedIndex(0); + } break; } } diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index db5d76f96a5f..f7991e515807 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -539,7 +539,13 @@ const MultiSelect = React.forwardRef( break; case ToggleButtonClick: setIsOpenWrapper(changes.isOpen || false); - break; + if (highlightedIndex === -1) { + return { + ...changes, + highlightedIndex: 0, + }; + } + return changes; case ToggleButtonKeyDownArrowDown: case ToggleButtonKeyDownArrowUp: case ToggleButtonKeyDownHome: diff --git a/packages/styles/scss/components/combo-box/_combo-box.scss b/packages/styles/scss/components/combo-box/_combo-box.scss index 9f246e9b552e..94cde01c0163 100644 --- a/packages/styles/scss/components/combo-box/_combo-box.scss +++ b/packages/styles/scss/components/combo-box/_combo-box.scss @@ -7,7 +7,9 @@ @use '../list-box'; @use '../../config' as *; +@use '../../motion' as *; @use '../../theme' as *; +@use '../../utilities/convert'; @use '../../utilities/focus-outline' as *; /// Combo box styles @@ -32,10 +34,19 @@ border-block-end-color: $border-subtle; } + .#{$prefix}--combo-box .#{$prefix}--text-input { + transition: outline $duration-fast-02 motion(standard, productive); + } + .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input { @include focus-outline('outline'); } + .#{$prefix}--list-box--expanded + .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input { + outline-width: convert.to-rem(1px); + } + .#{$prefix}--combo-box .#{$prefix}--list-box__field, .#{$prefix}--combo-box.#{$prefix}--list-box[data-invalid] .#{$prefix}--list-box__field, diff --git a/packages/styles/scss/components/dropdown/_dropdown.scss b/packages/styles/scss/components/dropdown/_dropdown.scss index 36e082b47e28..9005e8f05c62 100644 --- a/packages/styles/scss/components/dropdown/_dropdown.scss +++ b/packages/styles/scss/components/dropdown/_dropdown.scss @@ -93,6 +93,10 @@ border-block-end-color: $border-subtle; } + .#{$prefix}--dropdown--open .cds--list-box__field { + outline: none; + } + .#{$prefix}--dropdown--invalid { @include focus-outline('invalid'); diff --git a/packages/styles/scss/components/list-box/_list-box.scss b/packages/styles/scss/components/list-box/_list-box.scss index 60c041b09988..1113cb63e100 100644 --- a/packages/styles/scss/components/list-box/_list-box.scss +++ b/packages/styles/scss/components/list-box/_list-box.scss @@ -519,7 +519,7 @@ $list-box-menu-width: convert.to-rem(300px); inset-inline-end: 0; inset-inline-start: 0; overflow-y: auto; - transition: max-height $duration-fast-02 motion(standard, productive); + transition: max-height $duration-fast-01 motion(standard, productive); &:focus { // remove default browser focus in firefox diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss index 38f257027245..45ba3312c0e4 100644 --- a/packages/styles/scss/components/multiselect/_multiselect.scss +++ b/packages/styles/scss/components/multiselect/_multiselect.scss @@ -93,6 +93,16 @@ @include focus-outline('outline'); } + .#{$prefix}--multi-select.#{$prefix}--list-box--expanded + .#{$prefix}--list-box__field--wrapper:has( + button[aria-activedescendant]:not([aria-activedescendant='']) + ), + .#{$prefix}--multi-select--filterable.#{$prefix}--list-box--expanded:has( + input[aria-activedescendant]:not([aria-activedescendant='']) + ) { + outline-width: convert.to-rem(1px); + } + .#{$prefix}--multi-select--filterable.#{$prefix}--multi-select--selected .#{$prefix}--text-input, .#{$prefix}--multi-select.#{$prefix}--multi-select--selected From 8ae35c4bcc09e56e8b1dfef7a3b507dabc11a521 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Fri, 15 Mar 2024 12:21:02 -0400 Subject: [PATCH 02/12] fix(ListBox): update fluid listbox components --- .../styles/scss/components/combo-box/_combo-box.scss | 5 ----- .../components/fluid-combo-box/_fluid-combo-box.scss | 7 +++++++ .../components/fluid-list-box/_fluid-list-box.scss | 12 ++++++++---- .../fluid-multiselect/_fluid-multiselect.scss | 6 ++++++ .../styles/scss/components/list-box/_list-box.scss | 2 +- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/styles/scss/components/combo-box/_combo-box.scss b/packages/styles/scss/components/combo-box/_combo-box.scss index 94cde01c0163..4657e97bfd17 100644 --- a/packages/styles/scss/components/combo-box/_combo-box.scss +++ b/packages/styles/scss/components/combo-box/_combo-box.scss @@ -7,7 +7,6 @@ @use '../list-box'; @use '../../config' as *; -@use '../../motion' as *; @use '../../theme' as *; @use '../../utilities/convert'; @use '../../utilities/focus-outline' as *; @@ -34,10 +33,6 @@ border-block-end-color: $border-subtle; } - .#{$prefix}--combo-box .#{$prefix}--text-input { - transition: outline $duration-fast-02 motion(standard, productive); - } - .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input { @include focus-outline('outline'); } diff --git a/packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss b/packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss index d1496e06135f..630eb45bbdd5 100644 --- a/packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss +++ b/packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss @@ -69,4 +69,11 @@ + .#{$prefix}--list-box__invalid-icon { inset-inline-end: 1rem; } + + .#{$prefix}--list-box__wrapper--fluid + :not(.#{$prefix}--list-box--up) + .#{$prefix}--combo-box + .#{$prefix}--list-box__menu { + inset-block-start: calc(100% + convert.to-rem(1px)); + } } diff --git a/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss b/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss index 55b093c1c80e..40f4972b0e44 100644 --- a/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss +++ b/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss @@ -114,13 +114,18 @@ outline-offset: 0; } + .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has( + .#{$prefix}--list-box--expanded + ) { + outline-width: convert.to-rem(1px); + } + .#{$prefix}--list-box__wrapper--fluid .#{$prefix}--list-box__field:focus { outline: none; outline-offset: 0; } - .#{$prefix}--list-box__wrapper--fluid - :not(.#{$prefix}--list-box--up) + .#{$prefix}--list-box__wrapper--fluid:not(.#{$prefix}--list-box--up) .#{$prefix}--list-box__menu { inset-block-start: calc(100% + convert.to-rem(1px)); } @@ -235,8 +240,7 @@ } // direction - top - .#{$prefix}--list-box__wrapper--fluid - .#{$prefix}--list-box--up + .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box--up .#{$prefix}--list-box__menu { inset-block-end: $spacing-10; } diff --git a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss index 9aafa5fac6e8..47fa2c5d0a43 100644 --- a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss +++ b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss @@ -12,6 +12,7 @@ @use '../../motion' as *; @use '../../spacing' as *; @use '../../theme' as *; +@use '../../utilities/convert'; @use '../../utilities/focus-outline' as *; @use '../multiselect'; @use '../fluid-list-box'; @@ -49,4 +50,9 @@ .#{$prefix}--list-box__field { align-items: baseline; } + + .#{$prefix}--multi-select--filterable__wrapper:not(.#{$prefix}--list-box--up) + .#{$prefix}--list-box__menu { + inset-block-start: calc(100% + convert.to-rem(2px)); + } } diff --git a/packages/styles/scss/components/list-box/_list-box.scss b/packages/styles/scss/components/list-box/_list-box.scss index 1113cb63e100..60c041b09988 100644 --- a/packages/styles/scss/components/list-box/_list-box.scss +++ b/packages/styles/scss/components/list-box/_list-box.scss @@ -519,7 +519,7 @@ $list-box-menu-width: convert.to-rem(300px); inset-inline-end: 0; inset-inline-start: 0; overflow-y: auto; - transition: max-height $duration-fast-01 motion(standard, productive); + transition: max-height $duration-fast-02 motion(standard, productive); &:focus { // remove default browser focus in firefox From 616212c2337f9ebcbbc57df434c0384a31568260 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Mon, 18 Mar 2024 12:28:10 -0400 Subject: [PATCH 03/12] test(Multiselect): update keybaord nav tests --- .../ComboBox/ComboBox-test.avt.e2e.js | 12 +++---- .../Dropdown/Dropdown-test.avt.e2e.js | 4 +-- .../MultiSelect/MultiSelect-test.avt.e2e.js | 32 +++++++++---------- .../components/MultiSelect/MultiSelect.tsx | 16 ++++++---- .../MultiSelect/__tests__/MultiSelect-test.js | 1 - 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/e2e/components/ComboBox/ComboBox-test.avt.e2e.js b/e2e/components/ComboBox/ComboBox-test.avt.e2e.js index dcb32c07d176..ae3fba30cc19 100644 --- a/e2e/components/ComboBox/ComboBox-test.avt.e2e.js +++ b/e2e/components/ComboBox/ComboBox-test.avt.e2e.js @@ -38,7 +38,7 @@ test.describe('@avt ComboBox', () => { await page.keyboard.press('Tab'); await expect(combobox).toBeFocused(); await page.keyboard.press('Enter'); - await expect(page.getByRole('combobox', { expanded: true })).toBeVisible; + await expect(page.getByRole('combobox', { expanded: true })).toBeVisible(); await expect(combobox).toBeFocused(); await expect(page).toHaveNoACViolations('ComboBox-open'); @@ -65,7 +65,7 @@ test.describe('@avt ComboBox', () => { }); await expect(combobox).toBeVisible(); - await expect(clearButton).not.toBeVisible(); + await expect(clearButton).toBeHidden(); // Tab and open the ComboBox with Arrow Down await page.keyboard.press('Tab'); await expect(combobox).toBeFocused(); @@ -73,14 +73,14 @@ test.describe('@avt ComboBox', () => { await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Spacebar await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(combobox).toBeFocused(); await page.keyboard.press('Space'); await expect(menu).toBeVisible(); // Close and clear with Escape, retain focus, and open with Enter await page.keyboard.press('Escape'); await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(combobox).toBeFocused(); await page.keyboard.press('Enter'); await expect(menu).toBeVisible(); @@ -99,11 +99,11 @@ test.describe('@avt ComboBox', () => { ); // focus comes back to the toggle button after selecting await expect(combobox).toBeFocused(); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(clearButton).toBeVisible(); // should only clear selection when escape is pressed when the menu is closed await page.keyboard.press('Escape'); - await expect(clearButton).not.toBeVisible(); + await expect(clearButton).toBeHidden(); await expect(combobox).toHaveValue(''); // should highlight menu items based on text input await page.keyboard.press('2'); diff --git a/e2e/components/Dropdown/Dropdown-test.avt.e2e.js b/e2e/components/Dropdown/Dropdown-test.avt.e2e.js index 914608b6a757..70c0a24231a8 100644 --- a/e2e/components/Dropdown/Dropdown-test.avt.e2e.js +++ b/e2e/components/Dropdown/Dropdown-test.avt.e2e.js @@ -39,7 +39,7 @@ test.describe('@avt Dropdown', () => { const menu = page.getByRole('listbox'); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); - await expect(page.getByRole('combobox', { expanded: true })).toBeVisible; + await expect(page.getByRole('combobox', { expanded: true })).toBeVisible(); await expect(menu).toBeFocused(); await expect(page).toHaveNoACViolations('Dropdown-open'); @@ -68,7 +68,7 @@ test.describe('@avt Dropdown', () => { await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Enter await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); // Should focus on selected item by default diff --git a/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js b/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js index 8f8047ef3ae0..afa6502e0444 100644 --- a/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js +++ b/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js @@ -51,7 +51,7 @@ test.describe('@avt MultiSelect', () => { await page.keyboard.press('Tab'); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); - await expect(page.getByRole('combobox', { expanded: true })).toBeVisible; + await expect(page.getByRole('combobox', { expanded: true })).toBeVisible(); await expect(page).toHaveNoACViolations('MultiSelect-open'); }); @@ -72,7 +72,7 @@ test.describe('@avt MultiSelect', () => { await page.keyboard.press('Tab'); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); - await expect(page.getByRole('combobox', { expanded: true })).toBeVisible; + await expect(page.getByRole('combobox', { expanded: true })).toBeVisible(); await expect(page).toHaveNoACViolations('MultiSelect-open'); }); @@ -94,7 +94,7 @@ test.describe('@avt MultiSelect', () => { const menu = page.getByRole('listbox'); await expect(toggleButton).toBeVisible(); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); // Tab and open the MultiSelect with Arrow Down await page.keyboard.press('Tab'); await expect(toggleButton).toBeFocused(); @@ -102,19 +102,18 @@ test.describe('@avt MultiSelect', () => { await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Enter await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Spacebar await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Space'); await expect(menu).toBeVisible(); // Navigation inside the menu - // move to first option - await page.keyboard.press('ArrowDown'); + // Focus on first element by default await expect( page.getByRole('option', { name: 'An example option that is really long to show what should be done to handle long text', @@ -165,11 +164,11 @@ test.describe('@avt MultiSelect', () => { await page.keyboard.press('Escape'); await expect(toggleButton).toBeFocused(); // should show count of selected items when closed - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(selection).toBeVisible(); // should only clear selection when escape is pressed when the menu is closed await page.keyboard.press('Escape'); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); }); test.slow('@avt-keyboard-nav filterable multiselect', async ({ page }) => { @@ -187,7 +186,7 @@ test.describe('@avt MultiSelect', () => { const menu = page.getByRole('listbox'); await expect(toggleButton).toBeVisible(); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); // Tab and open the MultiSelect with Arrow Down await page.keyboard.press('Tab'); await expect(toggleButton).toBeFocused(); @@ -195,19 +194,18 @@ test.describe('@avt MultiSelect', () => { await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Enter await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Spacebar await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Space'); await expect(menu).toBeVisible(); // Navigation inside the menu - // move to first option - await page.keyboard.press('ArrowDown'); + // Focus on first element by default await expect( page.getByRole('option', { name: 'An example option that is really long to show what should be done to handle long text', @@ -244,11 +242,11 @@ test.describe('@avt MultiSelect', () => { await page.keyboard.press('Escape'); await expect(toggleButton).toBeFocused(); // should show count of selected items when closed - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(selection).toBeVisible(); // should only clear selection when escape is pressed when the menu is closed await page.keyboard.press('Escape'); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); // should filter menu items based on text input await page.keyboard.press('2'); @@ -264,6 +262,6 @@ test.describe('@avt MultiSelect', () => { name: 'Option 1', selected: false, }) - ).not.toBeVisible(); + ).toBeHidden(); }); }); diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index f7991e515807..2dbe0997cfed 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -48,6 +48,7 @@ const { ToggleButtonClick, ToggleButtonKeyDownHome, ToggleButtonKeyDownEnd, + FunctionSetHighlightedIndex, } = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & { ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick; }; @@ -400,6 +401,7 @@ const MultiSelect = React.forwardRef( getItemProps, selectedItem, highlightedIndex, + setHighlightedIndex, } = useSelect(selectProps); const toggleButtonProps = getToggleButtonProps({ @@ -426,6 +428,7 @@ const MultiSelect = React.forwardRef( match(e, keys.Enter)) && !isOpen ) { + setHighlightedIndex(0); setItemsCleared(false); setIsOpenWrapper(true); } @@ -539,13 +542,14 @@ const MultiSelect = React.forwardRef( break; case ToggleButtonClick: setIsOpenWrapper(changes.isOpen || false); - if (highlightedIndex === -1) { - return { - ...changes, - highlightedIndex: 0, - }; + break; + + case FunctionSetHighlightedIndex: + if (!isOpen) { + console.log('test'); + setHighlightedIndex(0); } - return changes; + break; case ToggleButtonKeyDownArrowDown: case ToggleButtonKeyDownArrowUp: case ToggleButtonKeyDownHome: diff --git a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js index 96859ac0ae93..721cc7ffb47f 100644 --- a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js +++ b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js @@ -206,7 +206,6 @@ describe('MultiSelect', () => { expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'false'); - await userEvent.keyboard('[ArrowDown]'); await userEvent.keyboard('[Enter]'); expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'true'); From d3aec93b394776f5ec641f52bc5b780df3a6cb55 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Tue, 19 Mar 2024 09:00:50 -0400 Subject: [PATCH 04/12] fix(Multiselect): remove console.log --- packages/react/src/components/MultiSelect/MultiSelect.tsx | 1 - yarn.lock | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 2dbe0997cfed..0ed90cab52cd 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -546,7 +546,6 @@ const MultiSelect = React.forwardRef( case FunctionSetHighlightedIndex: if (!isOpen) { - console.log('test'); setHighlightedIndex(0); } break; diff --git a/yarn.lock b/yarn.lock index cc3d4f414c83..9870b43c77c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7789,7 +7789,7 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/utils@npm:5.59.6, @typescript-eslint/utils@npm:^5.10.0, @typescript-eslint/utils@npm:^5.58.0": +"@typescript-eslint/utils@npm:5.59.6": version: 5.59.6 resolution: "@typescript-eslint/utils@npm:5.59.6" dependencies: @@ -7807,7 +7807,7 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/utils@npm:^5.62.0": +"@typescript-eslint/utils@npm:^5.10.0, @typescript-eslint/utils@npm:^5.58.0, @typescript-eslint/utils@npm:^5.62.0": version: 5.62.0 resolution: "@typescript-eslint/utils@npm:5.62.0" dependencies: From ca98e05d5aa1312089dc071ecf0522eecf99edad Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Tue, 19 Mar 2024 10:32:50 -0400 Subject: [PATCH 05/12] test(FluidMultiselect): update tests --- .../FluidMultiSelect-test.avt.e2e.js | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js b/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js index 5327284e23af..9ceaae2dce03 100644 --- a/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js +++ b/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js @@ -63,7 +63,7 @@ test.describe('@avt FluidMultiSelect', () => { const menu = page.getByRole('listbox'); await expect(toggleButton).toBeVisible(); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); // Tab and open the MultiSelect with Arrow Down await page.keyboard.press('Tab'); await expect(toggleButton).toBeFocused(); @@ -71,19 +71,18 @@ test.describe('@avt FluidMultiSelect', () => { await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Enter await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Spacebar await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Space'); await expect(menu).toBeVisible(); // Navigation inside the menu - // move to first option - await page.keyboard.press('ArrowDown'); + // Focus on first element by default await expect( page.getByRole('option', { name: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.', @@ -134,11 +133,11 @@ test.describe('@avt FluidMultiSelect', () => { await page.keyboard.press('Escape'); await expect(toggleButton).toBeFocused(); // should show count of selected items when closed - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(selection).toBeVisible(); // should only clear selection when escape is pressed when the menu is closed await page.keyboard.press('Escape'); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); }); test('@avt-keyboard-nav FluidMultiSelect condensed', async ({ page }) => { @@ -158,7 +157,7 @@ test.describe('@avt FluidMultiSelect', () => { const menu = page.getByRole('listbox'); await expect(toggleButton).toBeVisible(); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); // Tab and open the MultiSelect with Arrow Down await page.keyboard.press('Tab'); await expect(toggleButton).toBeFocused(); @@ -166,19 +165,18 @@ test.describe('@avt FluidMultiSelect', () => { await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Enter await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Enter'); await expect(menu).toBeVisible(); // Close with Escape, retain focus, and open with Spacebar await page.keyboard.press('Escape'); - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(toggleButton).toBeFocused(); await page.keyboard.press('Space'); await expect(menu).toBeVisible(); // Navigation inside the menu - // move to first option - await page.keyboard.press('ArrowDown'); + // Focus on first element by default await expect( page.getByRole('option', { name: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.', @@ -229,10 +227,10 @@ test.describe('@avt FluidMultiSelect', () => { await page.keyboard.press('Escape'); await expect(toggleButton).toBeFocused(); // should show count of selected items when closed - await expect(menu).not.toBeVisible(); + await expect(menu).toBeHidden(); await expect(selection).toBeVisible(); // should only clear selection when escape is pressed when the menu is closed await page.keyboard.press('Escape'); - await expect(selection).not.toBeVisible(); + await expect(selection).toBeHidden(); }); }); From f04eb5e9d06cb10d97a320ef6c22c9db715ed371 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 11:23:10 -0400 Subject: [PATCH 06/12] fix(ComboBox): adjust mouse interactions --- .../react/src/components/ComboBox/ComboBox.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index 8d962e31bf4f..bfb3ef36481d 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -53,9 +53,11 @@ const { keyDownArrowUp, keyDownEscape, clickButton, + clickItem, blurButton, changeInput, blurInput, + unknown, } = Downshift.stateChangeTypes; const defaultItemToString = (item: ItemType | null) => { @@ -450,15 +452,16 @@ const ComboBox = forwardRef( switch (type) { case keyDownArrowDown: case keyDownArrowUp: - setHighlightedIndex(changes.highlightedIndex); + if (changes.isOpen) { + updateHighlightedIndex(getHighlightedIndex(changes)); + } else { + setHighlightedIndex(changes.highlightedIndex); + } break; case blurButton: case keyDownEscape: setHighlightedIndex(changes.highlightedIndex); break; - case clickButton: - setHighlightedIndex(changes.highlightedIndex); - break; case changeInput: updateHighlightedIndex(getHighlightedIndex(changes)); break; @@ -470,6 +473,11 @@ const ComboBox = forwardRef( } } break; + case clickButton: + case clickItem: + case unknown: + setHighlightedIndex(getHighlightedIndex(changes)); + break; } }; From 94d583e8247cc7e54351919e5c4cf9dcf0c819e7 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 11:31:47 -0400 Subject: [PATCH 07/12] fix(Dropdown): adjust focus state on mouse leave --- packages/react/src/components/Dropdown/Dropdown.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index bcb38b43a921..66740e6a8ca8 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -49,6 +49,7 @@ const { ToggleButtonKeyDownHome, ToggleButtonKeyDownEnd, ItemMouseMove, + MenuMouseLeave, } = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & { ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick; }; @@ -290,6 +291,8 @@ const Dropdown = React.forwardRef( const { changes, props, type } = actionAndChanges; const { highlightedIndex } = changes; + console.log(type); + switch (type) { case ToggleButtonKeyDownArrowDown: case ToggleButtonKeyDownArrowUp: @@ -303,6 +306,7 @@ const Dropdown = React.forwardRef( } return changes; case ItemMouseMove: + case MenuMouseLeave: return { ...changes, highlightedIndex: state.highlightedIndex }; } return changes; From 7452da1b3c17bbe02f79c232a2a6fb5538f74631 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 12:31:31 -0400 Subject: [PATCH 08/12] fix(MultiSelect): match mouse focus to keyboard, home/end interaction --- .../MultiSelect/MultiSelect.stories.js | 8 +++++ .../components/MultiSelect/MultiSelect.tsx | 31 +++++++++++++------ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.stories.js b/packages/react/src/components/MultiSelect/MultiSelect.stories.js index dcd6b3f52253..4e280bbc42e1 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.stories.js +++ b/packages/react/src/components/MultiSelect/MultiSelect.stories.js @@ -111,6 +111,14 @@ const items = [ id: 'downshift-1-item-5', text: 'Option 5', }, + { + id: 'downshift-1-item-6', + text: 'Option 6', + }, + { + id: 'downshift-1-item-7', + text: 'Option 7', + }, ]; export const Playground = (args) => { diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 0ed90cab52cd..db1a8548aa12 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -45,9 +45,10 @@ const { ToggleButtonKeyDownEscape, ToggleButtonKeyDownSpaceButton, ItemMouseMove, + MenuMouseLeave, ToggleButtonClick, - ToggleButtonKeyDownHome, - ToggleButtonKeyDownEnd, + ToggleButtonKeyDownPageDown, + ToggleButtonKeyDownPageUp, FunctionSetHighlightedIndex, } = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & { ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick; @@ -525,7 +526,6 @@ const MultiSelect = React.forwardRef( } switch (type) { - case ItemClick: case ToggleButtonKeyDownSpaceButton: case ToggleButtonKeyDownEnter: if (changes.selectedItem === undefined) { @@ -542,17 +542,30 @@ const MultiSelect = React.forwardRef( break; case ToggleButtonClick: setIsOpenWrapper(changes.isOpen || false); - break; - + return { ...changes, highlightedIndex: 0 }; + case ItemClick: + setHighlightedIndex(changes.selectedItem); + onItemChange(changes.selectedItem); + return { ...changes, highlightedIndex: state.highlightedIndex }; + case MenuMouseLeave: + return { ...changes, highlightedIndex: state.highlightedIndex }; case FunctionSetHighlightedIndex: if (!isOpen) { - setHighlightedIndex(0); + return { + ...changes, + highlightedIndex: 0, + }; + } else { + console.log(' open'); + return { + ...changes, + highlightedIndex: props.items.indexOf(highlightedIndex), + }; } - break; case ToggleButtonKeyDownArrowDown: case ToggleButtonKeyDownArrowUp: - case ToggleButtonKeyDownHome: - case ToggleButtonKeyDownEnd: + case ToggleButtonKeyDownPageDown: + case ToggleButtonKeyDownPageUp: if (highlightedIndex > -1) { const itemArray = document.querySelectorAll( `li.${prefix}--list-box__menu-item[role="option"]` From db0aa716a287624f3ca21aacc75b35067b8c3e63 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 13:45:00 -0400 Subject: [PATCH 09/12] fix(FilterableMultiselect): align keyboard, mouse focus interaction --- .../MultiSelect/FilterableMultiSelect.js | 23 +++++++++++++++++++ .../MultiSelect/MultiSelect.stories.js | 8 ------- .../components/MultiSelect/MultiSelect.tsx | 1 - 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index ca26fcf857c4..3f2265012a1a 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -149,6 +149,10 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( if (onMenuChange) { onMenuChange(nextIsOpen); } + + if (!isOpen) { + setHighlightedIndex(0); + } } function handleOnOuterClick() { @@ -183,6 +187,25 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( setHighlightedIndex(0); } break; + case stateChangeTypes.clickItem: + if (isOpen) { + const sortedItems = sortItems( + filterItems(items, { itemToString, inputValue }), + { + selectedItems: { + top: changes.selectedItems, + fixed: [], + 'top-after-reopen': topItems, + }[selectionFeedback], + itemToString, + compareItems, + locale, + } + ); + const sortedSelectedIndex = sortedItems.indexOf(changes.selectedItem); + setHighlightedIndex(sortedSelectedIndex); + } + break; } } diff --git a/packages/react/src/components/MultiSelect/MultiSelect.stories.js b/packages/react/src/components/MultiSelect/MultiSelect.stories.js index 4e280bbc42e1..dcd6b3f52253 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.stories.js +++ b/packages/react/src/components/MultiSelect/MultiSelect.stories.js @@ -111,14 +111,6 @@ const items = [ id: 'downshift-1-item-5', text: 'Option 5', }, - { - id: 'downshift-1-item-6', - text: 'Option 6', - }, - { - id: 'downshift-1-item-7', - text: 'Option 7', - }, ]; export const Playground = (args) => { diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index db1a8548aa12..685b3e7226ac 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -556,7 +556,6 @@ const MultiSelect = React.forwardRef( highlightedIndex: 0, }; } else { - console.log(' open'); return { ...changes, highlightedIndex: props.items.indexOf(highlightedIndex), From a7bd4150a75f74beb86d1e23d6c8d3be4ad790f2 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 14:01:32 -0400 Subject: [PATCH 10/12] style(ListBox): adjust outline offset when outline is only 1px --- packages/react/src/components/MultiSelect/MultiSelect.tsx | 2 +- packages/styles/scss/components/combo-box/_combo-box.scss | 1 + .../scss/components/fluid-list-box/_fluid-list-box.scss | 2 +- .../components/fluid-multiselect/_fluid-multiselect.scss | 6 ++++-- .../styles/scss/components/multiselect/_multiselect.scss | 1 + 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 685b3e7226ac..8e6e7cbd8e5a 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -357,7 +357,7 @@ const MultiSelect = React.forwardRef( const { isFluid } = useContext(FormContext); const { current: multiSelectInstanceId } = useRef(getInstanceId()); const [isFocused, setIsFocused] = useState(false); - const [inputFocused, setInputFocused] = useState(false); + const [inputFocused, setInputFocused] = useState(true); const [isOpen, setIsOpen] = useState(open || false); const [prevOpenProp, setPrevOpenProp] = useState(open); const [topItems, setTopItems] = useState([]); diff --git a/packages/styles/scss/components/combo-box/_combo-box.scss b/packages/styles/scss/components/combo-box/_combo-box.scss index 4657e97bfd17..8a9cedec8bbf 100644 --- a/packages/styles/scss/components/combo-box/_combo-box.scss +++ b/packages/styles/scss/components/combo-box/_combo-box.scss @@ -39,6 +39,7 @@ .#{$prefix}--list-box--expanded .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input { + outline-offset: convert.to-rem(-1px); outline-width: convert.to-rem(1px); } diff --git a/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss b/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss index 40f4972b0e44..6e73fe1f550e 100644 --- a/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss +++ b/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss @@ -127,7 +127,7 @@ .#{$prefix}--list-box__wrapper--fluid:not(.#{$prefix}--list-box--up) .#{$prefix}--list-box__menu { - inset-block-start: calc(100% + convert.to-rem(1px)); + inset-block-start: calc(100%); } // Invalid / Warning styles diff --git a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss index 47fa2c5d0a43..d0c44f5cecce 100644 --- a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss +++ b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss @@ -51,8 +51,10 @@ align-items: baseline; } - .#{$prefix}--multi-select--filterable__wrapper:not(.#{$prefix}--list-box--up) + .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--multi-select--filterable__wrapper:not( + .#{$prefix}--list-box--up + ) .#{$prefix}--list-box__menu { - inset-block-start: calc(100% + convert.to-rem(2px)); + inset-block-start: calc(100% + convert.to-rem(1px)); } } diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss index 45ba3312c0e4..bc89f28a9054 100644 --- a/packages/styles/scss/components/multiselect/_multiselect.scss +++ b/packages/styles/scss/components/multiselect/_multiselect.scss @@ -100,6 +100,7 @@ .#{$prefix}--multi-select--filterable.#{$prefix}--list-box--expanded:has( input[aria-activedescendant]:not([aria-activedescendant='']) ) { + outline-offset: convert.to-rem(-1px); outline-width: convert.to-rem(1px); } From 50ab48ebbd02482297b2f2cfb5896f2855280cc0 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 14:50:53 -0400 Subject: [PATCH 11/12] chore(console): remove console.log --- packages/react/src/components/Dropdown/Dropdown.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index 66740e6a8ca8..b0dcd08ee12d 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -291,8 +291,6 @@ const Dropdown = React.forwardRef( const { changes, props, type } = actionAndChanges; const { highlightedIndex } = changes; - console.log(type); - switch (type) { case ToggleButtonKeyDownArrowDown: case ToggleButtonKeyDownArrowUp: From f20a2610839560b25f5bd458f67da3473a3e6617 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 28 Mar 2024 15:15:21 -0400 Subject: [PATCH 12/12] chore(Multiselect): revert testing default state --- packages/react/src/components/MultiSelect/MultiSelect.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 8e6e7cbd8e5a..685b3e7226ac 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -357,7 +357,7 @@ const MultiSelect = React.forwardRef( const { isFluid } = useContext(FormContext); const { current: multiSelectInstanceId } = useRef(getInstanceId()); const [isFocused, setIsFocused] = useState(false); - const [inputFocused, setInputFocused] = useState(true); + const [inputFocused, setInputFocused] = useState(false); const [isOpen, setIsOpen] = useState(open || false); const [prevOpenProp, setPrevOpenProp] = useState(open); const [topItems, setTopItems] = useState([]);