feat(tracemetrics): Add metric detail side panel to dropdown#110343
feat(tracemetrics): Add metric detail side panel to dropdown#110343nsdeschenes merged 21 commits intomasterfrom
Conversation
|
@sentry review |
|
@cursor review |
static/app/views/explore/metrics/metricToolbar/metricSelector.tsx
Outdated
Show resolved
Hide resolved
|
@sentry review |
|
@cursor review |
static/app/views/explore/metrics/metricToolbar/metricSelector.tsx
Outdated
Show resolved
Hide resolved
49e4e3b to
e4cc6b8
Compare
static/app/views/explore/metrics/metricToolbar/metricSelector.tsx
Outdated
Show resolved
Hide resolved
e2c3856 to
7fade0f
Compare
dd616be to
97d9dd0
Compare
k-fish
left a comment
There was a problem hiding this comment.
Need to fix the merge conflict, but looks fine, tested it locally, it works.
static/app/views/explore/metrics/metricToolbar/metricSelector.tsx
Outdated
Show resolved
Hide resolved
static/app/views/explore/metrics/metricToolbar/metricSelector.tsx
Outdated
Show resolved
Hide resolved
| const onKeyDown = useCallback( | ||
| (e: React.KeyboardEvent) => { | ||
| switch (e.key) { | ||
| case 'ArrowDown': { | ||
| e.preventDefault(); | ||
| if (displayedOptions.length === 0) { | ||
| break; |
There was a problem hiding this comment.
Do we have anywhere else that does list keyboard nav? Seems weird we're the only select doing it
There was a problem hiding this comment.
Yea there's a couple places, the scraps selects support this, as well as the search query builder comboboxes
There was a problem hiding this comment.
Merge impls? Later? This seems like repeated logic
There was a problem hiding this comment.
I've found somethings i'm not fully happy with, so i'll push a refactor up after this fixing those up.
Replace the CompactSelect-based metric selector with a custom overlay that includes a side detail panel, virtualized list with MenuListItem, checkmark indicators, content-driven width sizing, and responsive layout that stacks vertically on narrow screens. Also replace manual lodash debounce with useDebouncedValue hook and add text truncation to the trigger button. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Made-with: Cursor
… in MetricSelector Migrate MetricSelector from styled components to Container, Flex, and Text primitives. Also pass missing booleanAttributes prop to LogsInfiniteTable. Co-Authored-By: Claude Opus <noreply@anthropic.com> Made-with: Cursor
Replace Flex direction="column" with Stack, use Container for virtual list layout, and remove unnecessary Fragment around LeadWrap. Co-Authored-By: Claude Opus <noreply@anthropic.com> Made-with: Cursor
Add canUseMetricsSidePanelUI flag for tracemetrics-attributes-dropdown-side-panel. MetricSelector shows the detail panel only when the org has the feature enabled. Co-Authored-By: Claude Opus <noreply@anthropic.com> Made-with: Cursor
…elector Fall back to rendering all items when the virtualizer has no virtual items (e.g. in test environments where DOM layout dimensions are zero). Also add role, aria-label, and aria-selected attributes to option rows for accessibility, and update the empty placeholder text from 'Select a metric' to 'None'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add unit tests covering dropdown open/close, keyboard navigation, search input, empty state, metric type badges, unit badges, side panel feature flag, auto-selection, and aria-selected state. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
When a user navigated to an option then searched, focusedIndex could exceed displayedOptions.length. This caused the highlight to vanish and required multiple ArrowUp presses to recover. Clamp the index whenever the list shrinks. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Replace the manual searchRef + requestAnimationFrame focus hack with FocusScope's built-in autoFocus prop, which automatically focuses the first focusable element (the search input) when the overlay mounts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Left-align the metric selector button text and disable the trigger while the initial metric options are being fetched. The button is only disabled when no metric is selected yet, so background refetches do not block interaction. Add tests for the disabled/enabled loading states and for search input auto-focus on dropdown open. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
React requires state updaters to be pure functions with no side effects. virtualizer.scrollToIndex was being called inside setFocusedIndex updaters, which violates this contract and causes double invocation in Strict Mode. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Pressing ArrowUp/ArrowDown when displayedOptions is empty would call virtualizer.scrollToIndex with an out-of-bounds index and set focusedIndex to 0 for a nonexistent item. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
…overlay Move trailing items rendering (MetricTypeBadge and unit Tag) to use the trailingItems property from the option object rather than duplicating the JSX inline in the custom overlay. This consolidates the rendering logic in the metricOptions useMemo where it is already defined. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract the repeated trailing items rendering logic (type badge and unit tag) into a shared MetricOptionTrailingItems component. This also fixes a bug where the selected metric's list item was missing its trailing badges when filtered out of search results, because the optionFromTraceMetric object was created without trailingItems. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
The longestOption variable and its hidden rendered element prevent the overlay from resizing as the user scrolls through the virtualized list. This isn't immediately obvious, so add comments at both locations. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Replace all JSX conditional rendering using && with explicit ternaries (condition ? <Component /> : null) for consistency and to avoid potential falsy value rendering issues. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Document the non-obvious patterns in MetricSelector: overlay reset on close, optionFromTraceMetric fallback, option merging strategy, auto-select on initial load, stale options during fetches, focused index clamping, and virtualizer test fallback. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
getTotalSize() returns a number, but the inline style needs an explicit px suffix for the height to be applied correctly.
Switch from default import to named import for LoadingIndicator to match the module's export style. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Centralize the metric unit visibility check so the dropdown and detail panel share the same rules. This keeps the JSX shorter and avoids duplicating the NONE_UNIT and '-' guards. Co-Authored-By: GPT-5.4 <noreply@openai.com> Made-with: Cursor
Add coverage for the none-unit metric so the selector tests lock in that unit badges stay hidden even when the units UI is enabled. Refs LOGS-595 Co-Authored-By: GPT-5.4 <noreply@openai.com> Made-with: Cursor
97d9dd0 to
65dbabf
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Replace the CompactSelect metric picker with a custom overlay that includes a side panel showing metric details (name, type, unit, description). When a metric is selected or focused, the panel displays its metadata.
The new UI is gated behind the
tracemetrics-attributes-dropdown-side-panelfeature flag. Migrated from styled components to design system primitives (Container, Flex, Stack). Increased virtualizer overscan for smoother scrolling.Refs LOGS-595