Skip to content

Commit 5258df0

Browse files
kubeclaude
andcommitted
Fix stacked band rendering and stale store ref; revert frozen scales
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>
1 parent 42b7bb0 commit 5258df0

File tree

1 file changed

+32
-4
lines changed

1 file changed

+32
-4
lines changed

libs/@hashintel/petrinaut/src/views/Editor/panels/BottomPanel/subviews/simulation-timeline.tsx

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import "uplot/dist/uPlot.min.css";
66
import { SegmentGroup } from "../../../../../components/segment-group";
77
import type { SubView } from "../../../../../components/sub-view/types";
88
import { useElementSize } from "../../../../../hooks/use-element-size";
9+
import { useLatest } from "../../../../../hooks/use-latest";
910
import { useStableCallback } from "../../../../../hooks/use-stable-callback";
1011
import { PlaybackContext } from "../../../../../playback/context";
1112
import { SimulationContext } from "../../../../../simulation/context";
@@ -540,7 +541,11 @@ function drawPlayhead(u: uPlot, frameIdx: number): void {
540541
// -- uPlot options builder ----------------------------------------------------
541542

542543
interface ChartOptions {
544+
/** Store value for structural config (series, bands). */
543545
store: StreamingStore;
546+
/** Ref to the latest store — tooltip hooks read this so they always
547+
* see fresh data even if the store is replaced after a restart. */
548+
storeRef: React.RefObject<StreamingStore>;
544549
chartType: TimelineChartType;
545550
hiddenPlaces: Set<string>;
546551
size: { width: number; height: number };
@@ -552,6 +557,7 @@ interface ChartOptions {
552557
function buildUPlotOptions(opts: ChartOptions): uPlot.Options {
553558
const {
554559
store,
560+
storeRef,
555561
chartType,
556562
hiddenPlaces,
557563
size,
@@ -564,16 +570,27 @@ function buildUPlotOptions(opts: ChartOptions): uPlot.Options {
564570
let focused = -1;
565571

566572
const updateTooltip = (u: uPlot) => {
567-
const hit = resolveHoverTarget(u, store, chartType, hiddenPlaces, focused);
573+
// Read from ref so tooltip always sees the latest store, even after
574+
// simulation restart where the store object is replaced.
575+
const currentStore = storeRef.current;
576+
const hit = resolveHoverTarget(
577+
u,
578+
currentStore,
579+
chartType,
580+
hiddenPlaces,
581+
focused,
582+
);
568583
if (!hit) {
569584
t.root.style.display = "none";
570585
return;
571586
}
572587
positionTooltip(t, u, hit);
573588
};
574589

575-
// Build series config
590+
// Build series + bands config
576591
const series: uPlot.Series[] = [{ label: "Time" }];
592+
let bands: uPlot.Band[] | undefined;
593+
577594
if (chartType === "stacked") {
578595
const visible = store.places
579596
.filter((p) => !hiddenPlaces.has(p.placeId))
@@ -586,6 +603,15 @@ function buildUPlotOptions(opts: ChartOptions): uPlot.Options {
586603
width: 2,
587604
});
588605
}
606+
// Bands clip each series' fill to the region between it and the series
607+
// below, preventing overlapping semi-transparent layers from compositing
608+
// into progressively darker/muddier colors.
609+
if (visible.length > 1) {
610+
bands = [];
611+
for (let i = 1; i < visible.length; i++) {
612+
bands.push({ series: [i, i + 1] as [number, number] });
613+
}
614+
}
589615
} else {
590616
for (const p of store.places) {
591617
series.push({
@@ -601,6 +627,7 @@ function buildUPlotOptions(opts: ChartOptions): uPlot.Options {
601627
width: size.width,
602628
height: size.height,
603629
series,
630+
bands,
604631
pxAlign: false,
605632
padding: [4, 0, 0, null],
606633
cursor: {
@@ -759,6 +786,7 @@ const UPlotChart: React.FC<{
759786
const wrapperRef = useRef<HTMLDivElement>(null);
760787
const chartRef = useRef<uPlot | null>(null);
761788
const playheadFrameRef = useRef(currentFrameIndex);
789+
const storeRef = useLatest(store);
762790

763791
// -- Derived state ----------------------------------------------------------
764792

@@ -792,6 +820,7 @@ const UPlotChart: React.FC<{
792820

793821
const opts = buildUPlotOptions({
794822
store,
823+
storeRef,
795824
chartType,
796825
hiddenPlaces,
797826
size,
@@ -835,8 +864,7 @@ const UPlotChart: React.FC<{
835864
// -- Effect 3: stream new data (no chart recreation) ------------------------
836865

837866
useEffect(() => {
838-
// resetScales=false: our range functions handle bounds; skip redundant recalc
839-
chartRef.current?.setData(data, false);
867+
chartRef.current?.setData(data);
840868
}, [revision]);
841869

842870
// -- Effect 4: playhead redraw ---------------------------------------------

0 commit comments

Comments
 (0)