Skip to content
Merged
Changes from 1 commit
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
118 changes: 118 additions & 0 deletions packages/scratch-gui/test/unit/components/action-menu.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import '@testing-library/jest-dom';
import userEvent from '@testing-library/user-event';
import {render, screen, fireEvent} from '@testing-library/react';
import ActionMenu from '../../../src/components/action-menu/action-menu.jsx';
import {KEY} from '../../../src/lib/navigation-keys';
import React, {act} from 'react';

/**
* Wrap a callback in act and wait for a given time (ms) afterwards.
* @param {() => void} callback - the function that triggers state updates
* @param {number} waitTime - milliseconds to wait after act
*/
export const actWithDelay = async (callback, waitTime = 200) => {
await act(async () => {
callback();
jest.advanceTimersByTime(waitTime);
await new Promise(resolve => setTimeout(resolve, waitTime));
});
};

describe('ActionMenu keyboard navigation', () => {
const mockOnClick = jest.fn();
const mockMoreButtonClick = jest.fn();

const defaultProps = {
title: 'Main Button',
img: 'main-icon.svg',
onClick: mockOnClick,
moreButtons: [
{title: 'Button 1', img: 'icon1.svg', onClick: mockMoreButtonClick},
{title: 'Button 2', img: 'icon2.svg', onClick: mockMoreButtonClick},
{title: 'Button 3', img: 'icon3.svg', onClick: mockMoreButtonClick}
]
};

beforeEach(() => {
jest.useFakeTimers();
mockOnClick.mockClear();
mockMoreButtonClick.mockClear();
});

afterEach(() => {
jest.runOnlyPendingTimers();
jest.useRealTimers();
});

test('focus + arrow_down opens menu and arrow_up cycles to last', () => {
render(<ActionMenu {...defaultProps} />);

const mainButton = screen.getByRole('button', {name: 'Main Button'});
act(() => {
mainButton.focus();
});

act(() => {
fireEvent.keyDown(mainButton, {key: KEY.ARROW_DOWN});
});
const firstItem = screen.getByRole('button', {name: 'Button 1'});
expect(document.activeElement).toBe(firstItem);

act(() => {
fireEvent.keyDown(firstItem, {key: KEY.ARROW_UP});
});
const lastItem = screen.getByRole('button', {name: 'Button 3'});
expect(document.activeElement).toBe(lastItem);
});

test('escape closes menu and returns focus to main button', () => {
render(<ActionMenu {...defaultProps} />);

const mainButton = screen.getByRole('button', {name: 'Main Button'});
act(() => {
mainButton.focus();
fireEvent.keyDown(mainButton, {key: KEY.ARROW_DOWN});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need ARROW_DOWN to open the menu? Is it just so that we'd go to the first element? I wonder if that's relevant to the current test case, maybe it'd make more sense to keep it simple and avoid doing extra navigations? Same question for the other test cases where we do it.

});

const firstItem = screen.getByRole('button', {name: 'Button 1'});
expect(document.activeElement).toBe(firstItem);

act(() => {
fireEvent.keyDown(firstItem, {key: KEY.ESCAPE});
});
expect(document.activeElement).toBe(mainButton);
});

test('tab closes menu and focuses next element', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add a test for shift + tab as well

Copy link
Contributor Author

@kbangelov kbangelov Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I did add one, it kept failing if I entered the menu before shift + tab-ing and I couldn't figure out why. Perhaps it's some difference between the user testing library and the real thing. Not sure if it's ok to leave it like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you ended up adding a test for that. Did it start working consistently?

Copy link
Contributor Author

@kbangelov kbangelov Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one I left, yes. If we add arrow_down to get inside the menu it doesn't perform correctly. Since there were some changes to the base branch since then, I will wait for that one to get merged first and then try again with this test (and check the other ones too).

jest.useRealTimers();

render(
<>
<ActionMenu {...defaultProps} />
<button>After Menu</button>
</>
);

const mainButton = screen.getByRole('button', {name: 'Main Button'});
const afterButton = screen.getByRole('button', {name: 'After Menu'});
const user = userEvent.setup();

act(() => {
mainButton.focus();
fireEvent.keyDown(mainButton, {key: KEY.ARROW_DOWN});
});

const firstItem = screen.getByRole('button', {name: 'Button 1'});
expect(document.activeElement).toBe(firstItem);

await act(async () => {
await user.tab();
});

// Wait 1 second for any menu close animations or timeouts
await new Promise(resolve => setTimeout(resolve, 1000));
expect(document.activeElement).toBe(afterButton);

jest.useFakeTimers();
});
});
Loading