Skip to content

Commit b88adbb

Browse files
nsdeschenesclaudeClaude Sonnet 4
authored andcommitted
fix(metrics): Prevent metric selector hover from stealing search input focus (#111292)
This PR refactors the metrics selector from a list state setup over to a combobox state setup. This resolves the issue with the input losing focus, as well as bringing the selector more in line with what we're trying to achieve with it. --------- Co-authored-by: Claude Opus 4 <noreply@anthropic.com> Co-authored-by: Claude Sonnet 4 <noreply@example.com>
1 parent 2e7f870 commit b88adbb

File tree

2 files changed

+176
-137
lines changed

2 files changed

+176
-137
lines changed

static/app/views/explore/metrics/metricToolbar/metricSelector.spec.tsx

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,25 @@ describe('MetricSelector', () => {
147147
});
148148
});
149149

150+
it('dismisses outside click without committing the current keyboard focus', async () => {
151+
const onChange = jest.fn();
152+
render(<MetricSelector traceMetric={DEFAULT_TRACE_METRIC} onChange={onChange} />, {
153+
organization,
154+
});
155+
156+
await userEvent.click(screen.getByRole('button', {name: 'bar'}));
157+
await screen.findByRole('option', {name: SORTED_METRIC_NAMES[0]!});
158+
await userEvent.keyboard('{ArrowDown}');
159+
await userEvent.keyboard('{ArrowDown}');
160+
await userEvent.click(document.body);
161+
162+
await waitFor(() => {
163+
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
164+
});
165+
166+
expect(onChange).not.toHaveBeenCalled();
167+
});
168+
150169
it('closes after selecting an option and calls onChange', async () => {
151170
const onChange = jest.fn();
152171
render(<MetricSelector traceMetric={DEFAULT_TRACE_METRIC} onChange={onChange} />, {
@@ -262,11 +281,34 @@ describe('MetricSelector', () => {
262281
expect(onChange).toHaveBeenCalledTimes(1);
263282
});
264283

265-
// NOTE: Tests for ArrowDown moving DOM focus from the search input into
266-
// the list were removed because React Aria's FocusScope `contain` prevents
267-
// `.focus()` from moving DOM focus to list items in JSDOM. The underlying
268-
// selection behaviour is still covered by 'ArrowDown followed by Enter
269-
// selects an option'.
284+
it('ArrowDown from search keeps focus in input', async () => {
285+
render(<MetricSelector traceMetric={DEFAULT_TRACE_METRIC} onChange={jest.fn()} />, {
286+
organization,
287+
});
288+
await userEvent.click(screen.getByRole('button', {name: 'bar'}));
289+
const searchInput = await screen.findByPlaceholderText('Search metrics\u2026');
290+
await userEvent.keyboard('{ArrowDown}');
291+
292+
// DOM focus stays on search input; virtual focus moves to first option
293+
expect(searchInput).toHaveFocus();
294+
});
295+
296+
it('ArrowDown twice selects second option with Enter', async () => {
297+
const onChange = jest.fn();
298+
render(<MetricSelector traceMetric={DEFAULT_TRACE_METRIC} onChange={onChange} />, {
299+
organization,
300+
});
301+
302+
await userEvent.click(screen.getByRole('button', {name: 'bar'}));
303+
await screen.findByRole('option', {name: SORTED_METRIC_NAMES[0]!});
304+
await userEvent.keyboard('{ArrowDown}');
305+
await userEvent.keyboard('{ArrowDown}');
306+
await userEvent.keyboard('{Enter}');
307+
308+
expect(onChange).toHaveBeenCalledWith(
309+
expect.objectContaining({name: SORTED_METRIC_NAMES[1]})
310+
);
311+
});
270312

271313
it('keeps keyboard selection valid when displayed options shrink', async () => {
272314
const onChange = jest.fn();

0 commit comments

Comments
 (0)