Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 55 additions & 0 deletions packages/mui-material/test/integration/MenuList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,61 @@ describe('<MenuList> integration', () => {
expect(menu).toHaveFocus();
});

it('should not infinite loop on keyboard navigation when there are no children', () => {
render(<MenuList autoFocus />);

const menu = screen.getByRole('menu');

fireEvent.keyDown(menu, { key: 'ArrowDown' });
expect(menu).toHaveFocus();
fireEvent.keyDown(menu, { key: 'ArrowUp' });
expect(menu).toHaveFocus();
fireEvent.keyDown(menu, { key: 'Home' });
expect(menu).toHaveFocus();
fireEvent.keyDown(menu, { key: 'End' });
expect(menu).toHaveFocus();
});

it('should not infinite loop on keyboard navigation when children are removed', () => {
function DynamicMenuList() {
const [items, setItems] = React.useState(['Item 1', 'Item 2']);

return (
<React.Fragment>
<button data-testid="clear" onClick={() => setItems([])}>
Clear
</button>
<MenuList autoFocus>
{items.map((item) => (
<MenuItem key={item}>{item}</MenuItem>
))}
</MenuList>
</React.Fragment>
);
}

render(<DynamicMenuList />);

const menu = screen.getByRole('menu');
const menuitems = screen.getAllByRole('menuitem');

fireEvent.keyDown(menu, { key: 'ArrowDown' });
expect(menuitems[0]).toHaveFocus();

// Remove all children
fireEvent.click(screen.getByTestId('clear'));

act(() => {
menu.focus();
});

// Should not hang
fireEvent.keyDown(menu, { key: 'ArrowDown' });
expect(menu).toHaveFocus();
fireEvent.keyDown(menu, { key: 'ArrowUp' });
expect(menu).toHaveFocus();
});

it('should allow focus on disabled items when disabledItemsFocusable=true', () => {
render(
<MenuList autoFocus disabledItemsFocusable>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,71 @@ describe('useRovingTabIndex', () => {
expect(screen.getByTestId('container').getAttribute('tabindex')).to.equal('-1');
});

test('should not infinite loop when focusNext is called with no children', () => {
let focusNextResult: number | undefined;

function EmptyContainer() {
const { getContainerProps, focusNext: focusNextFn } = useRovingTabIndex({
orientation: 'horizontal',
});

focusNext = focusNextFn;

return <div data-testid="container" tabIndex={-1} {...getContainerProps()} />;
}

render(<EmptyContainer />);

act(() => {
focusNextResult = focusNext();
});

expect(focusNextResult).to.equal(-1);
});

test('should not infinite loop on arrow key navigation with no children', async () => {
function EmptyContainer() {
const { getContainerProps } = useRovingTabIndex({
orientation: 'horizontal',
});

return <div data-testid="container" tabIndex={-1} {...getContainerProps()} />;
}

const { user } = render(<EmptyContainer />);

const container = screen.getByTestId('container');
container.focus();

// These would hang if the bug is present
await user.keyboard('{ArrowRight}');
await user.keyboard('{ArrowLeft}');
await user.keyboard('{Home}');
await user.keyboard('{End}');

expect(container).toHaveFocus();
});

test('should not infinite loop on arrow key navigation with no children (vertical)', async () => {
function EmptyContainer() {
const { getContainerProps } = useRovingTabIndex({
orientation: 'vertical',
});

return <div data-testid="container" tabIndex={-1} {...getContainerProps()} />;
}

const { user } = render(<EmptyContainer />);

const container = screen.getByTestId('container');
container.focus();

await user.keyboard('{ArrowDown}');
await user.keyboard('{ArrowUp}');

expect(container).toHaveFocus();
});

test('should make the controlled prop take precedence over internal state', async () => {
const focusableIndex = 1;

Expand Down
4 changes: 4 additions & 0 deletions packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ function focusNext(
wrap: boolean,
shouldFocus: (element: HTMLElement | null) => boolean,
): number {
if (elementsRef.current.length === 0) {
return -1;
}

const lastIndex = elementsRef.current.length - 1;
let wrappedOnce = false;
let nextIndex = getNextIndex(currentIndex, lastIndex, direction, wrap);
Expand Down
Loading