Skip to content

Commit e2c11bb

Browse files
committed
Add "/" and "f" keyboard shortcuts to focus the panel filter box
"/" now focuses the filter input on any panel that has one (Call Tree, Flame Graph, Stack Chart, Marker Chart, Marker Table, Network Chart). The shortcut is discoverable through the input placeholder, which now reads "Enter filter terms (/)". "f" is an additional undocumented shortcut with the same effect, but only on panels where "f" is not already used as a call node transform shortcut (i.e. Marker Chart, Marker Table and Network Chart). It is opt-in via a new alsoFocusOnF prop on PanelSearch. Both shortcuts are no-ops when the user is already typing in an input, textarea or contenteditable element, and when modifier keys are held.
1 parent e775e55 commit e2c11bb

15 files changed

Lines changed: 209 additions & 31 deletions

locales/en-GB/app.ftl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ Home--chrome-extension-recording-instructions =
393393
## The component that is used for all the search inputs in the application.
394394

395395
IdleSearchField--search-input =
396-
.placeholder = Enter filter terms
396+
.placeholder = Enter filter terms (/)
397397
398398
## JsTracerSettings
399399
## JSTracer is an experimental feature and it's currently disabled. See Bug 1565788.

locales/en-US/app.ftl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ Home--chrome-extension-recording-instructions = Once installed, use the extensio
422422
## The component that is used for all the search inputs in the application.
423423

424424
IdleSearchField--search-input =
425-
.placeholder = Enter filter terms
425+
.placeholder = Enter filter terms (/)
426426
427427
## JsTracerSettings
428428
## JSTracer is an experimental feature and it's currently disabled. See Bug 1565788.

src/components/shared/IdleSearchField.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class IdleSearchField extends PureComponent<Props, State> {
126126
<input
127127
type="search"
128128
name="search"
129-
placeholder="Enter filter terms"
129+
placeholder="Enter filter terms (/)"
130130
className="idleSearchFieldInput photon-input"
131131
required={true}
132132
title={title ?? undefined}

src/components/shared/MarkerSettings.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ class MarkerSettingsImpl extends PureComponent<Props, State> {
227227
title="Only display markers that match a certain name"
228228
currentSearchString={searchString}
229229
onSearch={this._onSearch}
230+
alsoFocusOnF={true}
230231
/>
231232
</Localized>
232233
<Localized id="MarkerSettings--marker-filters" attrs={{ title: true }}>

src/components/shared/NetworkSettings.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class NetworkSettingsImpl extends PureComponent<Props> {
4444
title="Only display network requests that match a certain name"
4545
currentSearchString={searchString}
4646
onSearch={this._onSearch}
47+
alsoFocusOnF={true}
4748
/>
4849
</Localized>
4950
</div>

src/components/shared/PanelSearch.tsx

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,18 @@ type Props = {
1414
readonly title: string;
1515
readonly currentSearchString: string;
1616
readonly onSearch: (param: string) => void;
17+
// When true, the "f" key (in addition to "/") will also focus the search
18+
// field. This is opt-in because some panels (e.g. those showing frames) use
19+
// "f" as a call node transform shortcut.
20+
readonly alsoFocusOnF?: boolean;
1721
};
1822

1923
type State = { searchFieldFocused: boolean };
2024

2125
export class PanelSearch extends React.PureComponent<Props, State> {
2226
override state = { searchFieldFocused: false };
27+
_searchFieldWrapper = React.createRef<HTMLDivElement>();
28+
2329
_onSearchFieldIdleAfterChange = (value: string) => {
2430
this.props.onSearch(value);
2531
};
@@ -32,6 +38,55 @@ export class PanelSearch extends React.PureComponent<Props, State> {
3238
this.setState(() => ({ searchFieldFocused: false }));
3339
};
3440

41+
override componentDidMount() {
42+
window.addEventListener('keydown', this._handleGlobalKeyDown);
43+
}
44+
45+
override componentWillUnmount() {
46+
window.removeEventListener('keydown', this._handleGlobalKeyDown);
47+
}
48+
49+
_handleGlobalKeyDown = (event: KeyboardEvent) => {
50+
// Ignore key combinations involving modifier keys, so we don't interfere
51+
// with browser or OS shortcuts.
52+
if (event.ctrlKey || event.altKey || event.metaKey) {
53+
return;
54+
}
55+
56+
const isSlash = event.key === '/';
57+
const isF = this.props.alsoFocusOnF && event.key === 'f';
58+
if (!isSlash && !isF) {
59+
return;
60+
}
61+
62+
// Don't steal the key when the user is already typing in a text input,
63+
// textarea, contenteditable element, or interacting with a select.
64+
const target = event.target;
65+
if (target instanceof HTMLElement) {
66+
const tagName = target.tagName;
67+
if (
68+
tagName === 'INPUT' ||
69+
tagName === 'TEXTAREA' ||
70+
tagName === 'SELECT' ||
71+
target.isContentEditable
72+
) {
73+
return;
74+
}
75+
}
76+
77+
const wrapper = this._searchFieldWrapper.current;
78+
if (!wrapper) {
79+
return;
80+
}
81+
const input = wrapper.querySelector<HTMLInputElement>(
82+
'input[type="search"]'
83+
);
84+
if (input) {
85+
event.preventDefault();
86+
input.focus();
87+
}
88+
};
89+
3590
override render() {
3691
const { label, title, currentSearchString, className } = this.props;
3792
const { searchFieldFocused } = this.state;
@@ -40,7 +95,10 @@ export class PanelSearch extends React.PureComponent<Props, State> {
4095
currentSearchString &&
4196
!currentSearchString.includes(',');
4297
return (
43-
<div className={classNames('panelSearchField', className)}>
98+
<div
99+
className={classNames('panelSearchField', className)}
100+
ref={this._searchFieldWrapper}
101+
>
44102
<label className="panelSearchFieldLabel">
45103
{label + ' '}
46104
<IdleSearchField
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import { fireEvent, screen } from '@testing-library/react';
6+
7+
import { render } from 'firefox-profiler/test/fixtures/testing-library';
8+
import { PanelSearch } from 'firefox-profiler/components/shared/PanelSearch';
9+
import { fireFullKeyPress } from 'firefox-profiler/test/fixtures/utils';
10+
import { coerce } from 'firefox-profiler/utils/types';
11+
12+
describe('shared/PanelSearch', function () {
13+
function setup({ alsoFocusOnF = false }: { alsoFocusOnF?: boolean } = {}) {
14+
const onSearch = jest.fn();
15+
const renderResult = render(
16+
<PanelSearch
17+
className="testPanelSearch"
18+
label="Filter:"
19+
title="Filter description"
20+
currentSearchString=""
21+
onSearch={onSearch}
22+
alsoFocusOnF={alsoFocusOnF}
23+
/>
24+
);
25+
return { ...renderResult, onSearch };
26+
}
27+
28+
function getSearchInput(): HTMLInputElement {
29+
return screen.getByRole('searchbox') as HTMLInputElement;
30+
}
31+
32+
it('focuses the search input when the / key is pressed outside any input', () => {
33+
setup();
34+
const input = getSearchInput();
35+
expect(input).not.toHaveFocus();
36+
37+
fireFullKeyPress(coerce<Window, HTMLElement>(window), { key: '/' });
38+
39+
expect(input).toHaveFocus();
40+
});
41+
42+
it('does not steal the / key when focus is already inside an input', () => {
43+
setup();
44+
const input = getSearchInput();
45+
46+
// Add a separate input that will hold focus.
47+
const otherInput = document.createElement('input');
48+
otherInput.type = 'text';
49+
document.body.appendChild(otherInput);
50+
otherInput.focus();
51+
expect(otherInput).toHaveFocus();
52+
53+
// Fire the keydown event from the focused input. The listener should
54+
// bail out and leave focus where it was.
55+
fireEvent.keyDown(otherInput, { key: '/' });
56+
57+
expect(otherInput).toHaveFocus();
58+
expect(input).not.toHaveFocus();
59+
60+
document.body.removeChild(otherInput);
61+
});
62+
63+
it('does not react to / combined with a modifier key', () => {
64+
setup();
65+
const input = getSearchInput();
66+
expect(input).not.toHaveFocus();
67+
68+
fireEvent.keyDown(window, { key: '/', ctrlKey: true });
69+
expect(input).not.toHaveFocus();
70+
71+
fireEvent.keyDown(window, { key: '/', metaKey: true });
72+
expect(input).not.toHaveFocus();
73+
74+
fireEvent.keyDown(window, { key: '/', altKey: true });
75+
expect(input).not.toHaveFocus();
76+
});
77+
78+
it('focuses the search input when the f key is pressed and alsoFocusOnF is true', () => {
79+
setup({ alsoFocusOnF: true });
80+
const input = getSearchInput();
81+
expect(input).not.toHaveFocus();
82+
83+
fireFullKeyPress(coerce<Window, HTMLElement>(window), { key: 'f' });
84+
85+
expect(input).toHaveFocus();
86+
});
87+
88+
it('does not focus the search input on the f key when alsoFocusOnF is not set', () => {
89+
setup();
90+
const input = getSearchInput();
91+
expect(input).not.toHaveFocus();
92+
93+
fireFullKeyPress(coerce<Window, HTMLElement>(window), { key: 'f' });
94+
95+
expect(input).not.toHaveFocus();
96+
});
97+
98+
it('still uses / for focus even when alsoFocusOnF is true', () => {
99+
setup({ alsoFocusOnF: true });
100+
const input = getSearchInput();
101+
expect(input).not.toHaveFocus();
102+
103+
fireFullKeyPress(coerce<Window, HTMLElement>(window), { key: '/' });
104+
105+
expect(input).toHaveFocus();
106+
});
107+
108+
it('removes its global key listener on unmount', () => {
109+
const { unmount } = setup();
110+
const input = getSearchInput();
111+
unmount();
112+
113+
// After unmount, pressing / should be a no-op (no focus jump back).
114+
fireFullKeyPress(coerce<Window, HTMLElement>(window), { key: '/' });
115+
expect(input).not.toHaveFocus();
116+
});
117+
});

src/test/components/__snapshots__/FlameGraph.test.tsx.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ exports[`FlameGraph matches the snapshot 1`] = `
500500
<input
501501
class="idleSearchFieldInput photon-input"
502502
name="search"
503-
placeholder="Enter filter terms"
503+
placeholder="Enter filter terms (/)"
504504
required=""
505505
title="Only display stacks which contain a function whose name matches this substring"
506506
type="search"

src/test/components/__snapshots__/MarkerChart.test.tsx.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ exports[`MarkerChart renders the normal marker chart and matches the snapshot 1`
516516
<input
517517
class="idleSearchFieldInput photon-input"
518518
name="search"
519-
placeholder="Enter filter terms"
519+
placeholder="Enter filter terms (/)"
520520
required=""
521521
title="Only display markers that match a certain name"
522522
type="search"

src/test/components/__snapshots__/MarkerTable.test.tsx.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ exports[`MarkerTable renders some basic markers and updates when needed 1`] = `
127127
<input
128128
class="idleSearchFieldInput photon-input"
129129
name="search"
130-
placeholder="Enter filter terms"
130+
placeholder="Enter filter terms (/)"
131131
required=""
132132
title="Only display markers that match a certain name"
133133
type="search"

0 commit comments

Comments
 (0)