Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(react-listbox): Fixing click/tap focus scroll bug introduced with recent Listbox standalone changes #31235

Closed
wants to merge 7 commits into from
Closed

fix(react-listbox): Fixing click/tap focus scroll bug introduced with recent Listbox standalone changes #31235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
66cf27d
Fixing bug where clicking an unfocused scrolled listbox item would ca…
stevencomsft Apr 30, 2024
e3fe1d5
Adding change log
stevencomsft Apr 30, 2024
f0e1129
Updating unit tests
stevencomsft Apr 30, 2024
ebd3345
Merge remote-tracking branch 'origin/master' into stevenco/listboxFoc…
stevencomsft May 1, 2024
2268357
Merge branch 'master' into stevenco/listboxFocusOnClickBug
stevencomsft May 6, 2024
59be767
Merge branch 'master' into stevenco/listboxFocusOnClickBug
stevencomsft May 6, 2024
9fdc840
Merge branch 'microsoft:master' into stevenco/listboxFocusOnClickBug
stevencomsft May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@fluentui-react-combobox-7c2043a7-1e96-40fa-87a9-aff3adf28a3e.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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": "[email protected]",
"dependentChangeType": "patch"
}
9 changes: 9 additions & 0 deletions packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ describe('Listbox', () => {
</Listbox>,
);

// set initial keyboard navigating
fireEvent.keyDown(getByTestId('listbox'), { key: 'Tab' });

const firstOption = getByTestId('firstOption');
expect(getByTestId('listbox').getAttribute('aria-activedescendant')).toBeNull();

Expand All @@ -103,6 +106,9 @@ describe('Listbox', () => {
</Listbox>,
);

// set initial keyboard navigating
fireEvent.keyDown(getByTestId('listbox'), { key: 'Tab' });

const selectedOption = getByTestId('firstSelectedOption');
expect(getByTestId('listbox').getAttribute('aria-activedescendant')).toBeNull();

Expand All @@ -120,6 +126,9 @@ describe('Listbox', () => {
</Listbox>,
);

// set initial keyboard navigating
fireEvent.keyDown(getByTestId('listbox'), { key: 'Tab' });

expect(getByTestId('listbox').getAttribute('aria-activedescendant')).toBeNull();

fireEvent.focus(getByTestId('listbox'));
Expand Down
12 changes: 11 additions & 1 deletion packages/react-components/react-combobox/src/components/Listbox/useListbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,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 = {
Expand Down Expand Up @@ -75,6 +76,11 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
const hasParentActiveDescendantContext = useHasParentActiveDescendantContext();
const activeDescendantController = hasParentActiveDescendantContext ? activeDescendantContext.controller : controller;

const isNavigatingWithKeyboardRef = React.useRef(false);
useOnKeyboardNavigationChange(
_isNavigatingWithKeyboard => (isNavigatingWithKeyboardRef.current = _isNavigatingWithKeyboard),
);

const { clearSelection, selectedOptions, selectOption } = useSelection(props);

const onKeyDown = (event: React.KeyboardEvent<HTMLElement>) => {
Expand Down Expand Up @@ -113,7 +119,11 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
};

const onFocus = (_event: React.FocusEvent<HTMLElement>) => {
if (hasParentActiveDescendantContext || activeDescendantController.active()) {
if (
hasParentActiveDescendantContext ||
activeDescendantController.active() ||
!isNavigatingWithKeyboardRef.current
) {
return;
}

Expand Down
Loading