FE-570: Replace SVG simulation timeline with uPlot#8627
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
PR SummaryMedium Risk Overview Adds a new Updates BottomPanel/tab layout to support full-bleed tab content via a new Reviewed by Cursor Bugbot for commit 3a23cdd. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: Migrates the Simulation Timeline BottomPanel subview from a custom SVG/D3 renderer to uPlot to improve rendering quality (HiDPI/antialiasing) and enable efficient streaming updates. Changes:
Technical Notes: Legend-driven series visibility is preserved; stacked mode uses cumulative-band hit testing while run mode uses uPlot’s nearest-series focus for tooltip selection. 🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Chart destroyed and recreated on every streaming update
- I moved total frame clamping to a ref read inside onScrub so its callback identity stays stable and the chart creation effect no longer re-runs on each streaming frame increase.
Or push these changes by commenting:
@cursor push fa7f82c7d6
Preview (fa7f82c7d6)
diff --git a/libs/@hashintel/petrinaut/src/views/Editor/panels/BottomPanel/subviews/simulation-timeline.tsx b/libs/@hashintel/petrinaut/src/views/Editor/panels/BottomPanel/subviews/simulation-timeline.tsx
--- a/libs/@hashintel/petrinaut/src/views/Editor/panels/BottomPanel/subviews/simulation-timeline.tsx
+++ b/libs/@hashintel/petrinaut/src/views/Editor/panels/BottomPanel/subviews/simulation-timeline.tsx
@@ -679,12 +679,16 @@
const chartRef = useRef<uPlot | null>(null);
const playheadFrameRef = useRef(currentFrameIndex);
playheadFrameRef.current = currentFrameIndex;
+ const totalFramesRef = useRef(totalFrames);
+ totalFramesRef.current = totalFrames;
const onScrub = useCallback(
(idx: number) => {
- setCurrentViewedFrame(Math.max(0, Math.min(idx, totalFrames - 1)));
+ setCurrentViewedFrame(
+ Math.max(0, Math.min(idx, totalFramesRef.current - 1)),
+ );
},
- [setCurrentViewedFrame, totalFrames],
+ [setCurrentViewedFrame],
);
// Build data from storeThis Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
86b5878 to
7bbd005
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
5258df0 to
c0e217f
Compare
Migrate the simulation timeline chart from a custom SVG implementation to uPlot for better performance and rendering quality. - 2px antialiased lines (pxAlign: false), HiDPI-aware - Streaming: maintain uPlot columnar data directly in refs, push new frames in-place instead of copying existing data on every batch - Playhead drawn on canvas via uPlot's `draw` hook for pixel-perfect alignment with the plot area (accounts for axes/padding) - Run and stacked chart types preserved - Click/drag scrubbing via cursor bind handlers - Legend with click-to-hide Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Hover tooltip: imperative DOM mounted in u.over (no React renders per mousemove). Run mode uses uPlot's nearest-series focus; stacked mode walks cumulative bands at the cursor's y to find the hit. Edge-clamps and flips above/below the cursor so it always fits in the chart area. - Top x-axis rendered as a Logic Pro-style ruler with a separator line. Playhead is a rounded-top "pin" with a triangular tip and white border, drawn in physical pixels with dpr-correct sizes. - Ruler scrubbing: clicks/drags on the axis area scrub the playhead via native pointer events with setPointerCapture. - Cursor unlocks (lock: false) so the dotted crosshair only shows on hover. - Add SubView.noPadding flag and move BottomPanel padding from the outer container into the header and the padded content variant, so noPadding subviews are truly full-bleed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… reviews Decompose the monolithic chart effect into 4 single-responsibility effects: - Effect 1: create/destroy uPlot on structural changes only - Effect 2: sync container size via useElementSize hook - Effect 3: stream data with setData(data, false) - Effect 4: playhead redraw Derived state replaces imperative plumbing: - useElementSize(ref) replaces getBoundingClientRect + inline ResizeObserver - useStableCallback(onScrub) keeps a stable identity, fixing chart recreation on every streaming update (AI review #1) - Extract attachRulerScrubbing() helper with cached overRect AI review fixes: - #1 onScrub instability causing chart recreation — useStableCallback - #2 missing placeMeta in streaming effect deps - #3 fill color hex assumption — use color-mix(in srgb) instead - #4 y-scale [0,0] when all values zero — Math.max(1, max * 1.05) - #7 playhead line offset not dpr-scaled — tipY - 4 * dpr Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Break the 275-line monolithic options builder into single-responsibility functions: - resolveHoverTarget(): pure function that resolves which place + value is under the cursor, collapsing 7 scattered hide branches into one null-returning flow - positionTooltip(): updates tooltip content and edge-clamps position inside u.over, only called on the happy path - drawPlayhead(): self-contained canvas drawing for the Logic Pro-style pin head and vertical guide line buildUPlotOptions now takes a single ChartOptions object and is ~100 lines of config assembly with no embedded business logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AI review fixes: - #5 Stacked chart: add bands config so each series' fill is clipped to the region between it and the adjacent series below, preventing overlapping semi-transparent layers from compositing into muddy colors - #6 Stale store ref: tooltip hooks now read from a useLatest(store) ref instead of the store value captured at chart creation time, so tooltip values stay correct after simulation restart - #9 Frozen scales: revert setData(data, false) back to setData(data) since scales must recalculate as data grows (time extends, y changes) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AI review fixes: - #10 Stacked data recomputed on every render — wrap data in useMemo keyed on revision/chartType/hiddenPlaces. The component opts out of React Compiler ("use no memo"), so without manual memoization the expensive buildStackedData ran on every playback frame even though Effect 3 only consumes it on revision changes - #11 Removed redundant store.length === 0 check inside the chart creation effect. The parent (SimulationTimelineContent) already gates on store.length === 0 and renders a "No simulation data" message, so UPlotChart only mounts when data exists. Comment clarifies the contract. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No source files import d3-scale — drop both the runtime dep and its @types package. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Swap chart padding from [4, 0, 0, null] to [0, 0, 4, null] so the bottom y-axis label isn't cut off; the top doesn't need padding since the ruler axis sits there. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pass the chart-area className directly to UPlotChart and drop the inner absolute-positioned div. The defensive relative/absolute pattern is unnecessary because UPlotChart sizes its canvas in pixels via setSize, so it can't grow its parent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump uPlot's right padding from 0 to 8px so the 12px-wide playhead pin isn't clipped by the canvas edge when the current frame is at the rightmost position. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two small correctness fixes from PR review: - When the simulation restarts (totalFrames drops), bump the revision counter immediately so React re-renders to pick up the empty store. Previously the bump only happened when subsequent fetched frames were pushed, so a cancelled/empty follow-up fetch left the chart displaying stale pre-restart data. - In the drawClear hook, scale the separator line's width and its sub-pixel offset by devicePixelRatio. uPlot's canvas uses physical pixels, so the previous hardcoded 1/0.5 values rendered at 0.5/0.25 CSS pixels on HiDPI displays — thinner than uPlot's own grid lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7fa7492 to
4b6d979
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.
Reviewed by Cursor Bugbot for commit 4b6d979. Configure here.



🌟 What is the purpose of this PR?
Replaces the custom SVG simulation timeline chart with uPlot for better performance and rendering quality. Adds hover tooltip, Logic Pro-style ruler with playhead, ruler scrubbing, and full-bleed layout.
🔗 Related links
🔍 What does this change?
u.over(zero React renders per mousemove). Run mode uses nearest-series focus; stacked mode walks cumulative bands. Edge-clamps inside the chart areaSubView.noPaddingflag; BottomPanel padding moved from outer container into header/content variantsbandsconfig so fills don't overlap/darkenuseElementSizehook,useStableCallbackfor stableonScrub,useLatestfor fresh store ref in tooltip closures. Chart creation effect only fires on structural changescolor-mix(in srgb)instead of hex-appending88Math.max(1, max * 1.05)prevents [0,0] rangePre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
❓ How to test this?
🤖 Generated with Claude Code