Skip to content

Commit 87520e8

Browse files
nsdeschenesclaude
andcommitted
fix(metrics): Keep input focus in metric selector by syncing overlay with combobox state
Remove the stateGuard ref and re-entrant open/close synchronization between useOverlay and useComboBoxState. Instead, pass comboBoxState's isOpen directly to useOverlay and let each hook manage its own state with a simple bridging onOpenChange. Filter duplicate onSelectionChange fires by checking comboBoxState.isOpen. Fixes LOGS-627 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b1e0ef8 commit 87520e8

File tree

1 file changed

+32
-51
lines changed

1 file changed

+32
-51
lines changed

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

Lines changed: 32 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -242,31 +242,8 @@ export function MetricSelector({
242242
[displayedOptions]
243243
);
244244

245-
// Guards against re-entrant open/close calls between useOverlay and
246-
// useComboBoxState (which each sync into the other's state), and tracks
247-
// the last key handled by onSelectionChange to deduplicate (useComboBoxState
248-
// fires it twice: once on select, once on close).
249-
const stateGuardRef = useRef({syncing: false, lastSelection: null as string | null});
250-
251-
function handleOpenChange(open: boolean) {
252-
if (stateGuardRef.current.syncing) {
253-
return;
254-
}
255-
stateGuardRef.current.syncing = true;
256-
try {
257-
if (open) {
258-
comboBoxState.open();
259-
overlayState.open();
260-
} else {
261-
comboBoxState.close();
262-
overlayState.close();
263-
}
264-
} finally {
265-
stateGuardRef.current.syncing = false;
266-
}
267-
245+
function handleOverlayOpenChange(open: boolean) {
268246
if (open) {
269-
stateGuardRef.current.lastSelection = null;
270247
nextFrameCallback(() => updateOverlay?.());
271248
return;
272249
}
@@ -287,24 +264,6 @@ export function MetricSelector({
287264
});
288265
}
289266

290-
const {
291-
isOpen,
292-
state: overlayState,
293-
triggerProps,
294-
overlayProps,
295-
triggerRef,
296-
update: updateOverlay,
297-
} = useOverlay({
298-
type: 'listbox',
299-
position: 'bottom-start',
300-
offset: 6,
301-
isDismissable: true,
302-
isKeyboardDismissDisabled: true,
303-
shouldApplyMinWidth: true,
304-
disableTrigger: isFetching && !traceMetric.name,
305-
onOpenChange: handleOpenChange,
306-
});
307-
308267
const comboBoxState = useComboBoxState<MetricSelectOption>({
309268
children: (item: MetricSelectOption) => <Item key={item.value}>{item.label}</Item>,
310269
items: displayedOptions,
@@ -315,16 +274,9 @@ export function MetricSelector({
315274
onInputChange: setSearchInputValue,
316275
selectedKey: traceMetric.name ? traceMetricSelectValue : null,
317276
onSelectionChange: key => {
318-
if (!key) {
277+
if (!key || !comboBoxState.isOpen) {
319278
return;
320279
}
321-
// useComboBoxState fires onSelectionChange twice per selection: once
322-
// when the key is selected, and once during close via commitSelection.
323-
// Deduplicate by tracking the last key we handled.
324-
if (String(key) === stateGuardRef.current.lastSelection) {
325-
return;
326-
}
327-
stateGuardRef.current.lastSelection = String(key);
328280
const selectedOption = displayedOptionsMap.get(String(key));
329281
if (selectedOption) {
330282
onChange({
@@ -334,7 +286,36 @@ export function MetricSelector({
334286
});
335287
}
336288
},
337-
onOpenChange: handleOpenChange,
289+
onOpenChange: handleOverlayOpenChange,
290+
});
291+
292+
const {
293+
isOpen,
294+
state: overlayState,
295+
triggerProps,
296+
overlayProps,
297+
triggerRef,
298+
update: updateOverlay,
299+
} = useOverlay({
300+
type: 'listbox',
301+
position: 'bottom-start',
302+
offset: 6,
303+
isOpen: comboBoxState.isOpen,
304+
isDismissable: true,
305+
isKeyboardDismissDisabled: true,
306+
shouldApplyMinWidth: true,
307+
disableTrigger: isFetching && !traceMetric.name,
308+
onOpenChange: open => {
309+
if (open === comboBoxState.isOpen) {
310+
return;
311+
}
312+
313+
if (open) {
314+
comboBoxState.open();
315+
} else {
316+
comboBoxState.close();
317+
}
318+
},
338319
});
339320

340321
const {inputProps: comboBoxInputProps, listBoxProps} = useComboBox<MetricSelectOption>(

0 commit comments

Comments
 (0)