feat: Wire heatmap chart into dashboard editor and tile rendering#2107
feat: Wire heatmap chart into dashboard editor and tile rendering#2107alex-fedotyev wants to merge 33 commits intomainfrom
Conversation
- Add Heatmap tab to chart editor form with IconGrid3x3 icon - Create HeatmapSeriesEditor with value expression (Y-axis) and count expression (intensity) inputs - Add heatmap display type to displayTypeToActiveTab mapping - Add heatmap preview rendering in ChartPreviewPanel - Add heatmap tile rendering in DBDashboardPage - Add heatmap validation: single series, required value expression - Constrain heatmap to single series (no Add Series / ratio) Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 7509050 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
E2E Test Results✅ All tests passed • 146 passed • 3 skipped • 1144s
Tests ran across 4 shards in parallel. |
…ormat, reduce chart whitespace - When switching to Heatmap with a trace source, auto-fill value expression with getDurationMsExpression and set numberFormat to duration with factor 0.001 for proper Y-axis labels (4ms, 100ms, 1.2s) - Pass numberFormat from config through to DBHeatmapChart in both ChartPreviewPanel and DBDashboardPage for correct tick formatting - Reduce Stack gap in ChartContainer from default to 4px to minimize whitespace above the chart area Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
…ctivity in editor/dashboard - Add effect to populate duration defaults when source changes while already in heatmap mode (not just when switching to heatmap tab) - Disable drag-to-select and hide 'Click & Drag' prompt in Heatmap when onFilter is not provided (chart editor and dashboard contexts) - Keep hover tooltip with Y/Count values for read-only inspection Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
…ngs button for heatmap - Fill Value field with getDurationMsExpression and Count field with count() when switching to Heatmap with a trace source (visible text, not just behind the scenes) - Add Display Settings button below heatmap editor, same pattern as other chart types, opening ChartDisplaySettingsDrawer for number format configuration Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
…hboards - Extract HeatmapSettingsDrawer into shared component used by both search Event Deltas and chart editor/dashboard contexts - Remove Value/Count fields from the main heatmap editor form — move them into the Heatmap Settings drawer (Scale, Value, Count) - Replace 'Display Settings' button with 'Heatmap Settings' button that opens the same drawer as search Event Deltas gear icon - Pass scaleType (log/linear) from form state through to DBHeatmapChart in preview panel and dashboard tiles - Pre-populate Value with getDurationMsExpression and Count with count() from data source config, editable via Heatmap Settings Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
…artContainer gap - Add padding: [8,0,0,0] to uPlot options to reduce whitespace above the topmost Y-axis tick in heatmap charts - Set lockScroll=false on HeatmapSettingsDrawer to prevent layout shift when opening settings inside a dashboard edit modal - Restore ChartContainer gap to 'xs' (was incorrectly set to 4px affecting all chart types) Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
…ChartContainer changes - Wrap HeatmapSettingsDrawer in Portal so it doesn't participate in the flex layout of the editor form, preventing content shift when opening/closing settings in dashboard edit modal - Revert ChartContainer gap/margin changes to preserve existing spacing for all chart types Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
Pass autoRun to EditTimeChartForm in the dashboard edit modal so saved charts (including heatmaps) render their preview immediately when opened for editing, matching the Chart Explorer behavior. Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
… layout shift Move the HeatmapSettingsDrawer out of HeatmapSeriesEditor and render it at the EditTimeChartForm top level (same pattern as ChartDisplaySettingsDrawer). This prevents any DOM insertion from affecting the flex layout between the settings button and run button when the drawer opens/closes. Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
PR Review
|
…le form, revert ChartContainer - Extract toHeatmapChartConfig() helper into DBHeatmapChart.tsx, eliminating duplicated config construction and unsafe casts in DBDashboardPage and ChartPreviewPanel - Export HeatmapChartConfig and HeatmapSelectExtras types for typed access to countExpression and heatmapScaleType on select items - Add useEffect to sync HeatmapSettingsDrawer form when defaultValues change (prevents stale closure when source changes while drawer open) - Fully revert ChartContainer to original (no gap change) Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
alex-fedotyev
requested review from
a team and
pulpdrew
and removed request for
a team
pulpdrew
left a comment
pulpdrew
left a comment
There was a problem hiding this comment.
Cool stuff, but I have some suggestions.
Also, do we have a followup ticket to update the external API implementation+docs+tests to cover this new visualization type?
| const traceSource = | ||
| tableSource?.kind === SourceKind.Trace && | ||
| tableSource.durationExpression | ||
| ? tableSource | ||
| : undefined; |
There was a problem hiding this comment.
I think we need some sort of messaging or improved error messages here, or some way to make this work by default for non-trace sources.
I was very confused why this just errors out by default for non-Trace sources:
There was a problem hiding this comment.
Resolved by restricting the source picker to trace sources only (commit a587ca2). Non-trace sources no longer appear in the dropdown, so the error state isn't reachable. Defaults for trace sources (Duration / 1e6 + count()) auto-populate on tab/source change.
| <HeatmapSettingsDrawer | ||
| opened={heatmapSettingsOpened} | ||
| onClose={closeHeatmapSettings} | ||
| connection={tableConnection} | ||
| parentRef={parentRef} | ||
| defaultValues={heatmapSettingsDefaults} | ||
| scaleType={heatmapScaleType} | ||
| onScaleTypeChange={handleHeatmapScaleTypeChange} | ||
| onSubmit={handleUpdateHeatmapSettings} | ||
| /> |
There was a problem hiding this comment.
I understand that we're probably doing it this way because the Event Deltas heatmap uses a drawer for the settings. And that makes sense because it is restricted to Trace sources and a particular use case, which have good defaults.
In the context of the chart editor, the point is to be able to edit the chart, and I think it makes less sense to hide the heatmap settings in a drawer. Particularly because for metric and log sources, the defaults error out and it's not at all clear where I would set a 'value expression' if I'm not already aware that the settings are hidden in display settings.
I'd also suggest that the value and count expressions are not display settings but are just as central to this chart as the series are a part of the line chart or table chart - they should be front and center IMO
There was a problem hiding this comment.
Keeping as "advanced settings" in the drawer for now — heatmap is now restricted to trace sources (see the first thread), so value/count auto-populate from the source's duration expression and count(). The original pain (metric/log sources erroring out with no clear place to fix them) is gone.
Agreed this is a design call rather than a hard answer. We'll revisit based on user feedback; if the drawer turns out to be too buried we'll promote them into the main panel.
| } | ||
|
|
||
| // Validate number and pie charts only have one series | ||
| // Validate number, pie, and heatmap charts only have one series |
There was a problem hiding this comment.
Please update the unit tests to cover the changes in this file
There was a problem hiding this comment.
Added tests in packages/app/src/components/ChartEditor/__tests__/utils.test.ts:
validateChartForm— valid heatmap with trace source, multi-series rejection, missingvalueExpressionrejection- Round-trip:
countExpressionandheatmapScaleTypesurviveconvertFormStateToSavedChartConfig→convertSavedChartConfigToFormState
| chartConfigForExplanations?: ChartConfigWithOptTimestamp; | ||
| onSubmit: (suppressErrorNotification?: boolean) => void; | ||
| openDisplaySettings: () => void; | ||
| openHeatmapSettings?: () => void; |
There was a problem hiding this comment.
Does this need to be optional? Looks like it is always provided
| openHeatmapSettings?: () => void; | |
| openHeatmapSettings: () => void; |
There was a problem hiding this comment.
Done — (no longer optional).
There was a problem hiding this comment.
(Apologies, previous reply lost the code snippet to shell escaping.)
Done — openHeatmapSettings: () => void (no ?). Fallback to openDisplaySettings also removed per the next comment.
| setValue={setValue} | ||
| tableSource={tableSource} | ||
| onSubmit={onSubmit} | ||
| onOpenDisplaySettings={openHeatmapSettings ?? openDisplaySettings} |
There was a problem hiding this comment.
Would we ever want to open the display settings here?
| onOpenDisplaySettings={openHeatmapSettings ?? openDisplaySettings} | |
| onOpenHeatmapSettings={openHeatmapSettings} |
There was a problem hiding this comment.
Done — onOpenDisplaySettings={openHeatmapSettings} (no fallback).
| import { SQLPreview } from './ChartSQLPreview'; | ||
|
|
||
| /** Compact duration labels for axis ticks — fewer decimals, shorter units. */ | ||
| function formatDurationMsCompact(ms: number): string { |
There was a problem hiding this comment.
Thoughts on making this a shared util alongside formatDurationMs in packages/app/src/utils.ts?
There was a problem hiding this comment.
It would also be great if we could add some unit tests here.
There was a problem hiding this comment.
Done — moved to packages/app/src/utils.ts alongside formatDurationMs and exported. DBHeatmapChart.tsx now imports it from @/utils.
There was a problem hiding this comment.
Added 8 tests in packages/app/src/__tests__/utils.test.ts covering: zero, negative, ns (< 0.001ms), µs (< 1ms), ms (< 1000ms), seconds (< 2min), minutes (< 1h), and hours.
| ]; | ||
| setValue('select', heatmapSeries); | ||
| setValue('series', heatmapSeries); | ||
| setValue('series.0.countExpression' as any, 'count()'); |
There was a problem hiding this comment.
We should add countExpression to the SelectItem schema/type so that we don't need a cast here
There was a problem hiding this comment.
Done — see the reply to the HeatmapSelectExtras thread above. countExpression is now part of DerivedColumnSchema and the cast is gone.
| setValue('series.0.countExpression' as any, 'count()'); | ||
| if (traceSource) { | ||
| setValue('numberFormat', { | ||
| output: 'duration' as any, |
There was a problem hiding this comment.
This is not needed. Casting as any will hide any future type errors, and should be avoided when possible.
| output: 'duration' as any, | |
| output: 'duration', |
There was a problem hiding this comment.
Done — the cast wasn't needed since 'duration' is a valid value in NumberFormatSchema. Now just output: 'duration'.
| setValue('select', heatmapSeries); | ||
| setValue('series', heatmapSeries); | ||
| setValue('series.0.countExpression' as any, 'count()'); | ||
| setValue('numberFormat', { output: 'duration' as any, factor: 0.001 }); |
There was a problem hiding this comment.
This cast is not needed
| setValue('numberFormat', { output: 'duration' as any, factor: 0.001 }); | |
| setValue('numberFormat', { output: 'duration', factor: 0.001 }); |
There was a problem hiding this comment.
Done — no cast, just setValue('numberFormat', { output: 'duration', factor: 0.001 }).
| defaultValues: HeatmapSettingsValues; | ||
| scaleType: HeatmapScaleType; | ||
| onScaleTypeChange: (v: HeatmapScaleType) => void; | ||
| onSubmit: (v: HeatmapSettingsValues) => void; |
There was a problem hiding this comment.
Would it make sense to combine scaleType into HeatmapSettingsValues instead of treating them separately here and having to handle both handleHeatmapScaleTypeChange and handleUpdateHeatmapSettings in the parent component?
There was a problem hiding this comment.
Good call — done. scaleType is now part of HeatmapSettingsSchema:
export const HeatmapSettingsSchema = z.object({
value: z.string().trim().min(1),
count: z.string().trim().optional(),
scaleType: z.enum(['log', 'linear']).default('log'),
});The drawer manages scaleType via useWatch/setValue like the other fields, and onSubmit now receives all three values together. The parent no longer needs separate scaleType / onScaleTypeChange props or a second handler.
Heatmap charts require trace data (duration expression) to work properly. Non-trace sources would error out with no guidance. Instead of adding complex error messaging, filter the source dropdown to only show trace sources when heatmap display type is selected.
|
Pushed a587ca2 — restricts the data source picker to trace sources only when heatmap display type is selected. This addresses:
The Still working through the remaining review items (removing |
…editor-dashboard-5801 # Conflicts: # packages/app/src/components/ChartEditor/utils.ts
… formatted tick labels The static opt.axes[1].size function was measuring raw values from a prior uPlot cycle, not the actual formatted labels produced by the values callback (tickFormatter). This caused the Y-axis to be too narrow for longer labels like '1.67min'. Moving size into the options useMemo alongside the values callback ensures it always measures the same formatted strings that are actually rendered.
…ent Deltas Drag-select is intercepted by react-grid-layout when the heatmap is inside a dashboard tile, making the existing drag-based drill-down unreachable. Replace it with a click-popover pattern matching the line chart (DBTimeChart): clicking anywhere on the heatmap opens a small popover with a 'View in Event Deltas' link that preserves source, where clause, filters, and time range via buildEventsSearchUrl + mode=delta.
.claude/settings.json contains user-local tool permission preferences and should not be in the repo.
…editor-dashboard-5801
- Deduplicate heatmap defaults setup (previously inlined in two useEffects) into applyHeatmapDefaults() - Add changeset describing the feature for the next release
Review round-upReplied to each thread individually. Quick summary: Drew's comments
Claude Code review bot
New in this round
|
…n' clicks The options useMemo depended on the onFilter function reference. Since DBSearchHeatmapChart passes an inline arrow to onFilter, the reference changed on every parent render — so after clicking 'Filter by Selection' (which calls setFields() and triggers a re-render), the options object recomputed, uPlot re-initialized, and its internal u.select state (the rectangle) was wiped. Depend on a boolean (`hasFilter = !!onFilter`) instead of the function itself. The drag-enabled state is what the options actually care about, and the boolean only flips when onFilter goes from undefined to defined (which is a context switch, not a per-render change).
Before: drag → rectangle + 'Filter by Selection' button → click button → filter applies. After: drag → filter applies immediately; click anywhere on chart clears selection + exits comparison mode. Rationale: the button was an extra step for the only action that made sense after a drag. Applying on drag-end matches how selection works in most analytics tools and removes the 'how do I commit this?' discoverability gap. - Remove selectingInfo state and the 'Filter by Selection' floating button - setSelect hook now fires onFilter directly (with log→linear y conversion and time-unit scaling that the button did previously) - Add onClearFilter prop; container's onClick calls u.setSelect(zero) and onClearFilter, guarded by a justDraggedAtRef timestamp to ignore the synthetic click that follows mouseup after a drag - Restore uPlot instance capture via onCreate/onDelete for programmatic u.select clearing on outside-click - DBSearchHeatmapChart passes onClearFilter that resets xMin/xMax/yMin/yMax URL params (exits comparison mode) - Tooltip hint: 'Click & Drag to Select Data' → 'Drag to Compare · Click to Clear'
Two related cases where the selection rectangle went away but the URL xMin/xMax/yMin/yMax params lingered, leaving the delta chart stuck in comparison mode against stale coordinates: 1. Time range change (time picker) — data refetches, uPlot re-initializes (rectangle gone), but the xMin/xMax/yMin/yMax URL params remained and the delta chart kept running its comparison against the new time range. Added a useEffect that tracks dateRange and clears selection on change. 2. Display settings change (value/count/scaleType via drawer) — a rectangle drawn against the old y-axis semantics doesn't map cleanly to the new one. The drawer's onSubmit now clears selection alongside the new values.
|
Hi @pulpdrew — while fixing a regression where the heatmap selection rectangle disappeared after clicking "Filter by Selection" (turned out to be a dep-array issue I introduced in an earlier commit), we went a step further and rethought the drag interaction. Would appreciate a second pair of eyes on this UX change before we call the PR done. What changedBefore: drag a rectangle → rectangle + floating "Filter by Selection" button appears → click the button → delta comparison runs. After: drag a rectangle → delta comparison runs immediately; click anywhere on the chart to clear + exit comparison mode. Why
Also cleaned up
Open question for youConcern I'd flag: "drag to immediately run a query" means a slightly-off drag triggers work. The Commits for reference: 1f2b4fed (the UX change) and 75090506 (time/settings-change clears). |
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
3 participants
What
Wires the existing
DBHeatmapChartcomponent andDisplayType.Heatmapenum into the chart editor, dashboard tile rendering, and unifies display settings across all heatmap contexts (search Event Deltas, chart editor, dashboards).Why
Users should be able to create and view heatmap charts from the chart editor and dashboards, alongside existing chart types. Heatmaps visualize distributions over time (e.g., latency distributions) and were previously only available in the search Event Deltas view.
Changes
Shared Display Settings Drawer (
HeatmapSettingsDrawer.tsx):DBSearchHeatmapChart.tsxinto a reusable componentdefaultValuesmemoized at call sites to prevent unnecessary form resetsChart Editor (
EditTimeChartForm,ChartEditorControls,HeatmapSeriesEditor):IconGrid3x3icongetDurationMsExpression(source)andcount()when a Trace source is selected (on tab switch or source change)numberFormat: { output: 'duration', factor: 0.001 }for proper Y-axis labelsHeatmapContainer)Dashboard Tiles (
DBDashboardPage):HeatmapTilecomponent rendersDBHeatmapChartviatoHeatmapChartConfig()helperautoRunpassed toEditTimeChartFormin dashboard edit modal so saved charts preview immediatelynumberFormatandscaleTypepassed through from saved configChart Preview (
ChartPreviewPanel):HeatmapPreviewcomponent renders heatmap in the chart editor preview areaHeatmap UX improvements (
DBHeatmapChart.tsx):ctx.measureText()— measures actual formatted tick labelsformatDurationMsCompact):minstead ofmin, values under 2min stay as seconds, fewer decimalsgap: 10+space: 60prevents label overlap in narrow containerspadding: [8, 8, 0, 4]prevents label clipping at edgesonFilteris not provided (editor/dashboard); hover tooltip still shows for inspectiononFilteris wiredConfig helper (
toHeatmapChartConfig):HeatmapChartConfigformatHeatmapChartConfigtype fromDBHeatmapChart.tsx