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

esanuandra · 2025-08-14T13:31:27Z

Jira ticket - The keyboard handling for the revision dropdown should be done differently

This PR should make the navigation using the keyboard possible for searching revisions in Search component. Part of the implementation was made with the help of Cursor.

netlify · 2025-08-14T13:31:32Z

✅ Deploy Preview for mozilla-perfcompare ready!

Name	Link
🔨 Latest commit	`208f150`
🔍 Latest deploy log	https://app.netlify.com/projects/mozilla-perfcompare/deploys/692815f97a999f0008a0010f
😎 Deploy Preview	https://deploy-preview-926--mozilla-perfcompare.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

esanuandra · 2025-11-06T16:23:12Z

I need some help with the Snackbar tests, fetchRevisionByID and fetchRevisionsBySearch.
@kala-moz would you mind looking over them, please?

gmierz · 2025-11-27T17:09:46Z

src/components/Search/SearchInputAndResults.tsx

+    const activeColor =
+      theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark;
+    const captionStyle =
+      theme === 'light' ? captionStylesLight : captionStylesDark;


I'd much rather if we stick to using the light/dark mode selection tooling that Julien setup here: https://github.com/mozilla/perfcompare/blob/main/src/theme/protocolTheme.ts

Can you change your code to move the color setup for light/dark to there? That should clean up your code a good bit too.

Oh wait, it looks like these were just copied from here:

perfcompare/src/components/Search/SearchResultsList.tsx

Lines 35 to 43 in d6ed322

function getStyles(theme: string) {

const backgroundColor =

theme === 'light' ? Colors.Background300 : Colors.Background300Dark;

const hoverColor =

theme === 'light' ? Colors.SecondaryHover : Colors.SecondaryHoverDark;

const activeColor =

theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark;

const captionStyle =

theme === 'light' ? captionStylesLight : captionStylesDark;

Still, I think it would be good to make the light/dark mode changes now since we're already modifying this.

@gmierz This isn't a simple ask, and beyond the scope of this PR. It isn't simple as adding
sx={{ background: 'background.default', }} to the Box and Autocomplete components

In order to implement it the way above and delete the old way, Andra would have to redo the caption styles in the styles folder because they're based on the old dark/light styling system.

I will create a meta ticket and assign it to myself to update all the dark/light styles to Julien's new, simpler system, but for now I highly recommend leaving as is to get the proper keyboard handing checked in.

I agree with the caption style one, but the other three seem very easy to change. I checked it out locally, and everything seems to be working fine from my testing with these changes:

+ import { useTheme } from '@mui/material/styles'; import { style } from 'typestyle'; import AutocompleteInput from './AutocompleteInput'; @@ -47,6 +48,7 @@ export default function SearchInputAndResults({ listItemComponent, }: Props) { const mode = useAppSelector((state) => state.theme.mode); + const theme = useTheme(); const [recentRevisions, setRecentRevisions] = useState( null as null | Changeset[], @@ -76,39 +78,33 @@ export default function SearchInputAndResults({ zIndex: 100, }; - const getListStyles = (theme: string) => { - const backgroundColor = - theme === 'light' ? Colors.Background300 : Colors.Background300Dark; - const hoverColor = - theme === 'light' ? Colors.SecondaryHover : Colors.SecondaryHoverDark; - const activeColor = - theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark; + const getListStyles = (themeMode: string) => { const captionStyle = - theme === 'light' ? captionStylesLight : captionStylesDark; + themeMode === 'light' ? captionStylesLight : captionStylesDark; return style({ - backgroundColor, + backgroundColor: theme.palette.searchDropdown.background, position: 'relative', ...sharedSelectStyles, $nest: { // Autocomplete option highlighting 'li[aria-selected="true"]': { - backgroundColor: `${hoverColor} !important`, + backgroundColor: `${theme.palette.searchDropdown.hover} !important`, borderRadius: '4px', paddingTop: `${Spacing.xSmall}px`, }, 'li:hover': { - backgroundColor: `${hoverColor} !important`, + backgroundColor: `${theme.palette.searchDropdown.hover} !important`, borderRadius: '4px', paddingTop: `${Spacing.xSmall}px`, }, 'li[data-focus="true"]': { - backgroundColor: `${hoverColor} !important`, + backgroundColor: `${theme.palette.searchDropdown.hover} !important`, borderRadius: '4px', paddingTop: `${Spacing.xSmall}px`, }, 'li.Mui-focused': { - backgroundColor: `${hoverColor} !important`, + backgroundColor: `${theme.palette.searchDropdown.hover} !important`, borderRadius: '4px', paddingTop: `${Spacing.xSmall}px`, }, @@ -121,17 +117,17 @@ export default function SearchInputAndResults({ padding: `${Spacing.xSmall}px ${Spacing.Small}px`, $nest: { '&:hover': { - backgroundColor: hoverColor, + backgroundColor: theme.palette.searchDropdown.hover, borderRadius: '4px', }, '&:active': { - backgroundColor: activeColor, + backgroundColor: theme.palette.searchDropdown.active, borderRadius: '4px', }, }, }, '.item-selected': { - backgroundColor: hoverColor, + backgroundColor: theme.palette.searchDropdown.hover, borderRadius: '4px', }, '.revision-hash': { diff --git a/src/theme/protocolTheme.ts b/src/theme/protocolTheme.ts index 0befd83..e418b1f 100644 --- a/src/theme/protocolTheme.ts +++ b/src/theme/protocolTheme.ts @@ -41,6 +41,11 @@ const lightMode = { expandedRow: { background: Colors.SecondaryDefault, }, + searchDropdown: { + background: Colors.Background300, + hover: Colors.SecondaryHover, + active: Colors.SecondaryActive, + }, }; const darkMode = { @@ -77,6 +82,11 @@ const darkMode = { expandedRow: { background: Colors.Background100Dark, }, + searchDropdown: { + background: Colors.Background300Dark, + hover: Colors.SecondaryHoverDark, + active: Colors.SecondaryActiveDark, + }, };

gmierz · 2025-11-27T17:54:40Z

src/components/Search/SearchInputAndResults.tsx

+import { style } from 'typestyle';

-import SearchInput from './SearchInput';
-import SearchResultsList from './SearchResultsList';


Is the SearchResultsList still being used somewhere after these changes? I'm not able to find any other instances of it being used apart from this file. There's also SearchResultsListItem which I can't find any usages of after these changes.

We should remove them if they're unused now.

gmierz · 2025-11-27T17:59:39Z

src/components/Search/SearchInputAndResults.tsx

+    const activeColor =
+      theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark;
+    const captionStyle =
+      theme === 'light' ? captionStylesLight : captionStylesDark;


Oh wait, it looks like these were just copied from here:

perfcompare/src/components/Search/SearchResultsList.tsx

Lines 35 to 43 in d6ed322

function getStyles(theme: string) {

const backgroundColor =

theme === 'light' ? Colors.Background300 : Colors.Background300Dark;

const hoverColor =

theme === 'light' ? Colors.SecondaryHover : Colors.SecondaryHoverDark;

const activeColor =

theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark;

const captionStyle =

theme === 'light' ? captionStylesLight : captionStylesDark;

Still, I think it would be good to make the light/dark mode changes now since we're already modifying this.

gmierz · 2025-11-28T14:23:50Z

src/components/Search/AutocompleteOption.tsx

+  }),
+};
+
+function AutocompleteOption({


Do we need to call these new components Autocomplete* for any reason? If possible, I'd prefer it if we give them more descriptive names related to what they'll be used for, e.g. SearchRevisionOption.

Or maybe something like RevisionAutocompleteOption? I'm not 100% sure about what name to use here, but I think there are better options since this component is not generic, and is specific to revision options.

gmierz · 2025-11-28T14:29:09Z

src/components/Search/AutocompleteInput.tsx

+  compact: boolean;
+}
+
+function AutocompleteInput({


The same is true here. RevisionInput or RevisionAutocompleteInput would be along the lines of something better.

gmierz · 2025-11-28T14:38:55Z

src/components/Search/SearchInputAndResults.tsx

+        getOptionLabel={(options) => options.revision}
+        isOptionEqualToValue={(option, value) => option.id === value.id}
+        multiple={listItemComponent === 'checkbox'}
+        disableCloseOnSelect={listItemComponent === 'checkbox'}


I don't think this is working properly (I may be pointing to the wrong code that needs a fix). When I select something for the "new" revision when I'm using the keyboard by pressing enter, it closes the revision search dropdown instead of allowing me to continue making selections.

Andra I suggest trying this:

multiple={listItemComponent !== 'radio'} disableCloseOnSelect={listItemComponent !== 'radio'}

gmierz · 2025-11-28T14:44:56Z

src/components/Search/SearchInputAndResults.tsx

+            }
+          }
+        }}
+        renderInput={renderInput}


I'm not sure where this issue is coming from yet, but when I hit enter on a revision selection for the new one, the revision input field gets populated with the string of the revision that was selected. This is a bit annoying in combination with the issue above because if I want to select multiple revisions, I have to delete the revision input that was added, then I'm able to continue searching for more revisions to add.

kala-moz

Added my comments and suggestions to the PR.

kala-moz · 2025-12-01T21:51:05Z

src/components/Search/SearchInputAndResults.tsx

+    const activeColor =
+      theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark;
+    const captionStyle =
+      theme === 'light' ? captionStylesLight : captionStylesDark;


@gmierz This isn't a simple ask, and beyond the scope of this PR. It isn't simple as adding
sx={{ background: 'background.default', }} to the Box and Autocomplete components

In order to implement it the way above and delete the old way, Andra would have to redo the caption styles in the styles folder because they're based on the old dark/light styling system.

I will create a meta ticket and assign it to myself to update all the dark/light styles to Julien's new, simpler system, but for now I highly recommend leaving as is to get the proper keyboard handing checked in.

kala-moz · 2025-12-01T22:19:21Z

src/components/Search/SearchInputAndResults.tsx

+        getOptionLabel={(options) => options.revision}
+        isOptionEqualToValue={(option, value) => option.id === value.id}
+        multiple={listItemComponent === 'checkbox'}
+        disableCloseOnSelect={listItemComponent === 'checkbox'}


Andra I suggest trying this:

multiple={listItemComponent !== 'radio'} disableCloseOnSelect={listItemComponent !== 'radio'}

esanuandra added the 🚧 WIP 🚧 Work in progress: do not merge label Aug 14, 2025

esanuandra force-pushed the keyboard-handling-for-revision-dropdown branch 3 times, most recently from 90de22d to d5beba4 Compare August 14, 2025 13:44

esanuandra force-pushed the keyboard-handling-for-revision-dropdown branch from d5beba4 to d19522f Compare October 2, 2025 15:06

esanuandra force-pushed the keyboard-handling-for-revision-dropdown branch 6 times, most recently from fdde6ef to 54ddcf0 Compare October 16, 2025 14:50

esanuandra force-pushed the keyboard-handling-for-revision-dropdown branch 5 times, most recently from b6922d9 to 031a978 Compare October 21, 2025 16:12

esanuandra force-pushed the keyboard-handling-for-revision-dropdown branch 5 times, most recently from 13e39ea to 7c2d3b9 Compare November 6, 2025 16:14

esanuandra requested a review from kala-moz November 6, 2025 16:23

esanuandra added Ready For Review and removed 🚧 WIP 🚧 Work in progress: do not merge labels Nov 6, 2025

esanuandra added 2 commits November 27, 2025 11:12

Add autocomplete component

aee7f51

fix tests

208f150

esanuandra force-pushed the keyboard-handling-for-revision-dropdown branch from 7c2d3b9 to 208f150 Compare November 27, 2025 09:12

gmierz reviewed Nov 27, 2025

View reviewed changes

gmierz reviewed Nov 28, 2025

View reviewed changes

kala-moz requested changes Dec 1, 2025

View reviewed changes

esanuandra removed the Ready For Review label Dec 5, 2025

	function getStyles(theme: string) {
	const backgroundColor =
	theme === 'light' ? Colors.Background300 : Colors.Background300Dark;
	const hoverColor =
	theme === 'light' ? Colors.SecondaryHover : Colors.SecondaryHoverDark;
	const activeColor =
	theme === 'light' ? Colors.SecondaryActive : Colors.SecondaryActiveDark;
	const captionStyle =
	theme === 'light' ? captionStylesLight : captionStylesDark;

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

esanuandra commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for mozilla-perfcompare ready!

Uh oh!

esanuandra commented Nov 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kala-moz Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmierz Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmierz Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmierz Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kala-moz left a comment

Choose a reason for hiding this comment

Uh oh!

kala-moz Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

esanuandra commented Aug 14, 2025 •

edited

Loading

netlify bot commented Aug 14, 2025 •

edited

Loading

kala-moz Dec 1, 2025 •

edited

Loading

gmierz Dec 2, 2025 •

edited

Loading

gmierz Nov 28, 2025 •

edited

Loading

gmierz Nov 28, 2025 •

edited

Loading

kala-moz Dec 1, 2025 •

edited

Loading