Skip to content
Merged
Changes from 3 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
135 changes: 135 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,135 @@
import '@testing-library/jest-dom';
import userEvent from '@testing-library/user-event';
import {render, screen, fireEvent, waitFor} 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';

// Mock the CSS module so class names exist
jest.mock('../../../src/components/action-menu/action-menu.css', () => ({
expanded: 'expanded'
}));

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(() => {
mockOnClick.mockClear();
mockMoreButtonClick.mockClear();
});

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();
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.

nitpick: I think the this test case in the way that it's described and written is a bit misleading. Maybe we could do something like:

  • Focus opens menu and select the last element by default
  • Arrow down on last element cycles to the first
  • Arrow up on the first element cycles to the last

});

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);

const menuContainer = mainButton.parentElement;
expect(menuContainer).toHaveClass('expanded');
});

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);

const menuContainer = mainButton.parentElement;
expect(menuContainer).not.toHaveClass('expanded');
});

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).

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);

act(() => {
user.tab();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what's the difference between calling tab() on a user event and firing KEY.TAB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fireEvent.keyDown practically seemed to apply some .preventDefault() behavior.


await waitFor(() => {
expect(document.activeElement).toBe(afterButton);
expect(mainButton.parentElement).not.toHaveClass('expanded');
});
});

test('shift + tab closes menu and focuses previous element', async () => {
render(
<>
<button>Before Menu</button>
<ActionMenu {...defaultProps} />
<button>After Menu</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't need the after menu in this layout, since we never use it?

</>
);

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

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

const menuContainer = mainButton.parentElement;
expect(menuContainer).toHaveClass('expanded');
act(() => {
user.tab({shift: true});
});

await waitFor(() => {
expect(document.activeElement).toBe(beforeButton);
expect(mainButton.parentElement).not.toHaveClass('expanded');
});
});
});
Loading