Skip to content

PCF-391 The keyboard handling for the revision dropdown should be done differently #926

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

Open
esanuandra wants to merge 3 commits into
base: main
Choose a base branch
from
Open

PCF-391 The keyboard handling for the revision dropdown should be done differently #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c33f8f
Add autocomplete component
esanuandra Jul 21, 2025
0458490
fix tests
esanuandra Aug 27, 2025
7785cde
fix multiple select revision checkbox
esanuandra Dec 18, 2025
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
452 changes: 310 additions & 142 deletions src/__tests__/CompareResults/__snapshots__/ResultsTable.test.tsx.snap
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions src/__tests__/CompareResults/__snapshots__/ResultsView.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`Results View Should display Base, New and Common graphs with replicates
<section
aria-label="Revision Row Details"
class="MuiBox-root css-13s8wtg"
id="_r_af_"
id="_r_b7_"
>
<div
class="MuiStack-root css-1714cpj-MuiStack-root"
Expand Down Expand Up @@ -317,7 +317,7 @@ exports[`Results View Should display Base, New and Common graphs with tooltips 1
<section
aria-label="Revision Row Details"
class="MuiBox-root css-13s8wtg"
id="_r_93_"
id="_r_9n_"
>
<div
class="MuiStack-root css-1714cpj-MuiStack-root"
Expand Down Expand Up @@ -743,7 +743,7 @@ exports[`Results View The table should match snapshot and other elements should
aria-invalid="false"
aria-label="Search by title, platform, revision or options"
class="MuiInputBase-input MuiOutlinedInput-input MuiInputBase-inputSizeSmall MuiInputBase-inputAdornedStart MuiInputBase-inputAdornedEnd css-3v3un6-MuiInputBase-input-MuiOutlinedInput-input"
id="_r_g_"
id="_r_k_"
placeholder="Filter results"
type="search"
value=""
Expand Down Expand Up @@ -1515,7 +1515,7 @@ exports[`Results View The table should match snapshot and other elements should
data-testid="expand-revision-button"
>
<button
aria-controls="_r_13_"
aria-controls="_r_17_"
aria-expanded="false"
class="MuiButtonBase-root MuiIconButton-root MuiIconButton-colorPrimary MuiIconButton-sizeSmall css-1pgb59k-MuiButtonBase-root-MuiIconButton-root"
tabindex="0"
Expand Down Expand Up @@ -1726,7 +1726,7 @@ exports[`Results View The table should match snapshot and other elements should
data-testid="expand-revision-button"
>
<button
aria-controls="_r_1a_"
aria-controls="_r_1e_"
aria-expanded="false"
class="MuiButtonBase-root MuiIconButton-root MuiIconButton-colorPrimary MuiIconButton-sizeSmall css-1pgb59k-MuiButtonBase-root-MuiIconButton-root"
tabindex="0"
Expand Down Expand Up @@ -1915,7 +1915,7 @@ exports[`Results View The table should match snapshot and other elements should
data-testid="expand-revision-button"
>
<button
aria-controls="_r_1h_"
aria-controls="_r_1l_"
aria-expanded="false"
class="MuiButtonBase-root MuiIconButton-root MuiIconButton-colorPrimary MuiIconButton-sizeSmall css-1pgb59k-MuiButtonBase-root-MuiIconButton-root"
tabindex="0"
Expand Down Expand Up @@ -2115,7 +2115,7 @@ exports[`Results View The table should match snapshot and other elements should
data-testid="expand-revision-button"
>
<button
aria-controls="_r_1o_"
aria-controls="_r_1s_"
aria-expanded="false"
class="MuiButtonBase-root MuiIconButton-root MuiIconButton-colorPrimary MuiIconButton-sizeSmall css-1pgb59k-MuiButtonBase-root-MuiIconButton-root"
tabindex="0"
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/CompareResults/__snapshots__/SearchInput.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ exports[`Search by title/test name should filter results after a timeout or imme
aria-invalid="false"
aria-label="Search by title, platform, revision or options"
class="MuiInputBase-input MuiOutlinedInput-input MuiInputBase-inputSizeSmall MuiInputBase-inputAdornedStart MuiInputBase-inputAdornedEnd css-3v3un6-MuiInputBase-input-MuiOutlinedInput-input"
id="_r_i_"
id="_r_m_"
placeholder="Filter results"
type="search"
value=""
Expand Down
33 changes: 25 additions & 8 deletions src/__tests__/Search/CompareOverTime.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import {
waitFor,
} from '../utils/test-utils';

const searchRevisionPlaceholder =
Strings.components.searchDefault.base.collapsed.base.inputPlaceholder;

function setUpTestData() {
const { testData } = getTestData();
fetchMock
Expand Down Expand Up @@ -259,7 +262,11 @@ describe('Compare Over Time', () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

// Click inside the input box to show search results.
const searchInput = screen.getByRole('textbox');

// focus input to show results
const searchInput = screen.getAllByPlaceholderText(
searchRevisionPlaceholder,
)[2];
await user.click(searchInput);

const comment = await screen.findAllByText("you've got no arms left!");
Expand All @@ -281,7 +288,9 @@ describe('Compare Over Time', () => {

expect(within(formElement).getByText(/Time range/)).toBeInTheDocument();

const searchInput = screen.getByRole('textbox');
const searchInput = screen.getAllByPlaceholderText(
searchRevisionPlaceholder,
)[2];
await user.click(searchInput);
const checkbox = await screen.findByTestId('checkbox-0');
await user.click(checkbox);
Expand Down Expand Up @@ -310,7 +319,10 @@ describe('Compare Over Time', () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

// focus input to show results
const searchInput = screen.getByRole('textbox');
const searchInput = screen.getAllByPlaceholderText(
searchRevisionPlaceholder,
)[2];

await user.click(searchInput);

const noArmsLeft = await screen.findByText(/no arms left/);
Expand Down Expand Up @@ -368,8 +380,10 @@ describe('Compare Over Time', () => {
).toBeInTheDocument();

// focus first input to show results
const inputs = screen.getAllByRole('textbox');
await user.click(inputs[0]);
const searchInputs = screen.getAllByPlaceholderText(
searchRevisionPlaceholder,
);
await user.click(searchInputs[2]);

// Select a rev
const items = await screen.findAllByText("you've got no arms left!");
Expand Down Expand Up @@ -534,14 +548,17 @@ describe('Compare Over Time', () => {
expect(formElement).toMatchSnapshot('After clicking edit button');
expect(editButton).not.toBeVisible();

//add a new revision
const searchInput = within(formElement).getByRole('textbox');
// add a new revision
const searchInput = screen.getAllByPlaceholderText(
searchRevisionPlaceholder,
)[0];

await user.click(searchInput);
const alvesOfCoconut = await screen.findByText(/alves of coconuts/);
await user.click(alvesOfCoconut);
expect(checkboxForText(alvesOfCoconut)).toHaveClass('Mui-checked');

//change time range
// change time range
const timeRangeDropdown = screen.getByRole('combobox', {
name: /Time range/i,
});
Expand Down
8 changes: 7 additions & 1 deletion src/__tests__/Search/CompareWithBase.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import { Strings } from '../../resources/Strings';
import getTestData from '../utils/fixtures';
import { screen, renderWithRouter, within, waitFor } from '../utils/test-utils';

const searchRevisionPlaceholder =
Strings.components.searchDefault.base.collapsed.base.inputPlaceholder;

function setUpTestData() {
const { testData } = getTestData();
fetchMock
Expand Down Expand Up @@ -165,7 +168,10 @@ describe('Compare With Base', () => {
// set delay to null to prevent test time-out due to useFakeTimers
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

const searchInput = screen.getAllByRole('textbox')[0];
// focus input to show results
const searchInput = screen.getAllByPlaceholderText(
searchRevisionPlaceholder,
)[1];
await user.click(searchInput);
const checkbox = (await screen.findAllByTestId('checkbox-0'))[0];
await user.click(checkbox);
Expand Down
80 changes: 58 additions & 22 deletions src/__tests__/Search/SearchResultsList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { loader } from '../../components/Search/loader';
import SearchView from '../../components/Search/SearchView';
import { Strings } from '../../resources/Strings';
import getTestData from '../utils/fixtures';
import { screen, within, renderWithRouter } from '../utils/test-utils';
import { screen, renderWithRouter, fireEvent } from '../utils/test-utils';

async function renderComponent() {
renderWithRouter(<SearchView title={Strings.metaData.pageTitle.search} />, {
Expand All @@ -32,8 +32,12 @@ describe('SearchResultsList', () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

await renderComponent();

const placeholder =
Strings.components.searchDefault.base.collapsed.base.inputPlaceholder;

// focus input to show results
const searchInput = screen.getAllByRole('textbox')[0];
const searchInput = screen.getAllByPlaceholderText(placeholder)[0];
await user.click(searchInput);
await screen.findByText(/flesh wound/);
expect(document.body).toMatchSnapshot();
Expand All @@ -44,11 +48,20 @@ describe('SearchResultsList', () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

await renderComponent();
// focus input to show results
const searchInput = screen.getAllByRole('textbox')[0];
await user.click(searchInput);

const fleshWound = await screen.findAllByText("it's just a flesh wound");
const autocomplete = screen.getAllByTestId('autocomplete')[0];

// open dropdown using keyboard arrow down
fireEvent.keyDown(autocomplete, {
key: 'ArrowDown',
code: 'ArrowDown',
charCode: 40,
});

// Wait for the dropdown to open and results to load
await screen.findAllByRole('textbox');

const fleshWound = await screen.findAllByText(/it's just a flesh wound/);

await user.click(fleshWound[0]);
expect(screen.getAllByTestId('checkbox-1')[0]).toHaveClass('Mui-checked');
Expand All @@ -60,23 +73,29 @@ describe('SearchResultsList', () => {

await renderComponent();

// focus input to show results
const searchInput = screen.getAllByRole('textbox')[0];
await user.click(searchInput);
const autocomplete = screen.getAllByTestId('autocomplete')[0];

const fleshWound = await screen.findByRole('button', {
name: /it's just a flesh wound/,
// open dropdown using keyboard arrow down
fireEvent.keyDown(autocomplete, {
key: 'ArrowDown',
code: 'ArrowDown',
charCode: 40,
});
const fleshWoundCheckbox = within(fleshWound).getByRole('radio');

// Wait for the dropdown to open and results to load
await screen.findAllByRole('textbox');

const autocompleteOptions = await screen.findAllByTestId(
'autocomplete-option',
);
const fleshWound = autocompleteOptions[0];

await user.click(fleshWound);
expect(fleshWound).toHaveClass('item-selected');
expect(fleshWoundCheckbox).toBeChecked();
expect(fleshWound.querySelector('.Mui-checked')).toBeInTheDocument();

await user.click(fleshWound);
expect(fleshWound).not.toHaveClass('item-selected');
expect(fleshWoundCheckbox).not.toBeChecked();
expect(fleshWound.querySelector('.Mui-checked')).toBeNull();
});

Expand All @@ -85,9 +104,15 @@ describe('SearchResultsList', () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

await renderComponent();
// focus input to show results
const searchInput = screen.getAllByRole('textbox')[0];
await user.click(searchInput);
const autocomplete = screen.getAllByTestId('autocomplete')[0];

// open dropdown using keyboard arrow down
fireEvent.keyDown(autocomplete, {
key: 'ArrowDown',
code: 'ArrowDown',
charCode: 40,
});

await user.click((await screen.findAllByTestId('checkbox-0'))[0]);
await user.click(screen.getAllByTestId('checkbox-1')[0]);

Expand All @@ -111,8 +136,12 @@ describe('SearchResultsList', () => {
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });

await renderComponent();

const placeholder =
Strings.components.searchDefault.base.collapsed.base.inputPlaceholder;

// focus input to show results
const searchInput = screen.getAllByRole('textbox')[1];
const searchInput = screen.getAllByPlaceholderText(placeholder)[1];
await user.click(searchInput);

const noArmsLeft = await screen.findByText(/no arms left/);
Expand Down Expand Up @@ -146,10 +175,17 @@ describe('SearchResultsList', () => {

await user.click(darkModeToggle);
expect(screen.getByLabelText('Light mode')).toBeInTheDocument();
const searchInput = screen.getAllByRole('textbox')[0];
await user.click(searchInput);
const resultsList = screen.getByTestId('list-mode');
expect(resultsList).toMatchSnapshot('after toggling dark mode');
const autocomplete = screen.getAllByTestId('autocomplete')[0];

// open dropdown using keyboard arrow down
fireEvent.keyDown(autocomplete, {
key: 'ArrowDown',
code: 'ArrowDown',
charCode: 40,
});

const resultsList = screen.getByRole('listbox');
expect(resultsList).toMatchSnapshot('after toggling dark mode 1');
expect(resultsList).toHaveClass('results-list-dark');
});
});
Loading