From 66cf27d17dad99a0d900d1bc3449c4d9460e9e15 Mon Sep 17 00:00:00 2001 From: Steven Countryman Date: Tue, 30 Apr 2024 10:30:43 -0700 Subject: [PATCH 1/3] Fixing bug where clicking an unfocused scrolled listbox item would cause the listbox scroll container to shoot to the top do to unrestrained focus logic --- .../src/components/Listbox/useListbox.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-combobox/src/components/Listbox/useListbox.ts b/packages/react-components/react-combobox/src/components/Listbox/useListbox.ts index 0d9753bbfc944..b3be3a8a2533d 100644 --- a/packages/react-components/react-combobox/src/components/Listbox/useListbox.ts +++ b/packages/react-components/react-combobox/src/components/Listbox/useListbox.ts @@ -18,6 +18,7 @@ import { useOptionCollection } from '../../utils/useOptionCollection'; import { useSelection } from '../../utils/useSelection'; import { optionClassNames } from '../Option/useOptionStyles.styles'; import { ListboxContext, useListboxContext_unstable } from '../../contexts/ListboxContext'; +import { useOnKeyboardNavigationChange } from '@fluentui/react-tabster'; // eslint-disable-next-line @typescript-eslint/naming-convention const UNSAFE_noLongerUsed = { @@ -48,6 +49,11 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref el.classList.contains(optionClassNames.root), }); + const isNavigatingWithKeyboardRef = React.useRef(false); + useOnKeyboardNavigationChange( + _isNavigatingWithKeyboard => (isNavigatingWithKeyboardRef.current = _isNavigatingWithKeyboard), + ); + const activeDescendantContext = useActiveDescendantContext(); const hasParentActiveDescendantContext = useHasParentActiveDescendantContext(); const activeDescendantController = hasParentActiveDescendantContext ? activeDescendantContext.controller : controller; @@ -90,7 +96,11 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref) => { - if (hasParentActiveDescendantContext || activeDescendantController.active()) { + if ( + hasParentActiveDescendantContext || + activeDescendantController.active() || + !isNavigatingWithKeyboardRef.current + ) { return; } From e3fe1d5f822c16694062e95dd99d191544d950d6 Mon Sep 17 00:00:00 2001 From: Steven Countryman Date: Tue, 30 Apr 2024 10:36:58 -0700 Subject: [PATCH 2/3] Adding change log --- ...eact-combobox-7c2043a7-1e96-40fa-87a9-aff3adf28a3e.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-combobox-7c2043a7-1e96-40fa-87a9-aff3adf28a3e.json diff --git a/change/@fluentui-react-combobox-7c2043a7-1e96-40fa-87a9-aff3adf28a3e.json b/change/@fluentui-react-combobox-7c2043a7-1e96-40fa-87a9-aff3adf28a3e.json new file mode 100644 index 0000000000000..659614673609b --- /dev/null +++ b/change/@fluentui-react-combobox-7c2043a7-1e96-40fa-87a9-aff3adf28a3e.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fixing bug where click or tap focusing an item in an unfocused listbox would result in the listbox scrolling all the way to the top due to unrestrained initial focus logic", + "packageName": "@fluentui/react-combobox", + "email": "stevenco@microsoft.com", + "dependentChangeType": "patch" +} From f0e112984f352173d17bab84a3c7a9706e9be998 Mon Sep 17 00:00:00 2001 From: Steven Countryman Date: Tue, 30 Apr 2024 10:50:00 -0700 Subject: [PATCH 3/3] Updating unit tests --- .../src/components/Listbox/Listbox.test.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx b/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx index 6a8a895d17969..50fa03ec21690 100644 --- a/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx +++ b/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx @@ -86,6 +86,9 @@ describe('Listbox', () => { , ); + // set initial keyboard navigating + fireEvent.keyDown(getByTestId('listbox'), { key: 'Tab' }); + const firstOption = getByTestId('firstOption'); expect(getByTestId('listbox').getAttribute('aria-activedescendant')).toBeNull(); @@ -103,6 +106,9 @@ describe('Listbox', () => { , ); + // set initial keyboard navigating + fireEvent.keyDown(getByTestId('listbox'), { key: 'Tab' }); + const selectedOption = getByTestId('firstSelectedOption'); expect(getByTestId('listbox').getAttribute('aria-activedescendant')).toBeNull(); @@ -120,6 +126,9 @@ describe('Listbox', () => { , ); + // set initial keyboard navigating + fireEvent.keyDown(getByTestId('listbox'), { key: 'Tab' }); + expect(getByTestId('listbox').getAttribute('aria-activedescendant')).toBeNull(); fireEvent.focus(getByTestId('listbox'));