Skip to content

Commit

Permalink
fix(react): fix listbox focus issue when listbox options have changed (
Browse files Browse the repository at this point in the history
  • Loading branch information
scurker authored Dec 20, 2024
1 parent 0a00d24 commit 55d370c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
5 changes: 4 additions & 1 deletion packages/react/src/components/Listbox/Listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,10 @@ const Listbox = forwardRef<

const handleFocus = useCallback(
(event: React.FocusEvent<HTMLElement>) => {
if (!activeOption) {
if (
!activeOption ||
!options.some((option) => option.element === activeOption.element)
) {
const firstOption = options.find(
(option) => !isDisabledOption(option)
);
Expand Down
40 changes: 38 additions & 2 deletions packages/react/src/components/Listbox/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import Listbox from './';
import { ListboxGroup, ListboxOption } from './';
import axe from '../../axe';
Expand Down Expand Up @@ -202,7 +202,7 @@ test('should set the first non-disabled option as active on focus', () => {
</Listbox>
);

fireEvent.focus(screen.getByRole('option', { name: 'Banana' }));
fireEvent.focus(screen.getByRole('listbox'));
expect(screen.getByRole('option', { name: 'Banana' })).toHaveClass(
'ListboxOption--active'
);
Expand All @@ -212,6 +212,42 @@ test('should set the first non-disabled option as active on focus', () => {
);
});

test('should set the first non-disabled option as active on focus when the options have changed', () => {
const { rerender } = render(
<Listbox>
<ListboxOption disabled>Apple</ListboxOption>
<ListboxOption>Banana</ListboxOption>
<ListboxOption>Cantaloupe</ListboxOption>
</Listbox>
);

waitFor(() => {
fireEvent.focus(screen.getByRole('listbox'));
expect(screen.getByRole('listbox')).toHaveFocus();
});

rerender(
<Listbox>
<ListboxOption disabled>Dragon Fruit</ListboxOption>
<ListboxOption>Elderberry</ListboxOption>
<ListboxOption>Fig</ListboxOption>
</Listbox>
);

waitFor(() => {
fireEvent.focus(screen.getByRole('listbox'));
expect(screen.getByRole('listbox')).toHaveFocus();
});

expect(screen.getByRole('option', { name: 'Elderberry' })).toHaveClass(
'ListboxOption--active'
);
expect(screen.getByRole('listbox')).toHaveAttribute(
'aria-activedescendant',
screen.getByRole('option', { name: 'Elderberry' }).getAttribute('id')
);
});

test('should set selected value with "value" prop when listbox option only has text label', () => {
render(
<Listbox value="Banana">
Expand Down

0 comments on commit 55d370c

Please sign in to comment.