From c1a9e4fbe6b9dc765c288d759dbc33e7729e0e69 Mon Sep 17 00:00:00 2001 From: Ian Lapham Date: Tue, 11 Aug 2026 13:44:17 -0400 Subject: [PATCH] fix: clamp vertical marker stacks at the canvas edge Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 8 ++ docs/guides/markers-and-trades.mdx | 5 ++ .../src/components/CustomMarkerOverlay.tsx | 6 +- .../src/components/MarkerOverlay.tsx | 6 +- .../src/hooks/useMarkers.ts | 6 +- .../src/math/markerCluster.ts | 16 +++- .../tests/math/markerCluster.test.ts | 83 +++++++++++++++++++ 7 files changed, 126 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d222b58d..c41eb1a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Vertical marker stacks stay on the plot.** A `"vertical"` marker column + whose anchor sits near the top or bottom of the canvas no longer climbs off + the chart: the column is now also capped where the next glyph would cross the + canvas edge, hiding the overflow the same way `maxVisible` does. The base + glyph always draws. + ## [4.17.0] - 2026-08-10 ### Added diff --git a/docs/guides/markers-and-trades.mdx b/docs/guides/markers-and-trades.mdx index 6186e667..43ede340 100644 --- a/docs/guides/markers-and-trades.mdx +++ b/docs/guides/markers-and-trades.mdx @@ -73,6 +73,11 @@ tall column doesn't collapse to a count badge early. markerCluster={{ direction: "vertical", overlap: 0.6, maxBeforeGroup: 20 }} ``` +A column whose anchor sits near the top or bottom of the canvas is clamped at +the canvas edge automatically: glyphs that would land off the plot are hidden +(the base glyph always draws), the same overflow treatment as `maxVisible` +below. + #### Cap a vertical column `maxVisible` limits a **vertical** column to its oldest glyphs, keeping the diff --git a/packages/react-native-livechart/src/components/CustomMarkerOverlay.tsx b/packages/react-native-livechart/src/components/CustomMarkerOverlay.tsx index 295ea5ab..42a4e96b 100644 --- a/packages/react-native-livechart/src/components/CustomMarkerOverlay.tsx +++ b/packages/react-native-livechart/src/components/CustomMarkerOverlay.tsx @@ -199,7 +199,11 @@ export function CustomMarkerOverlay({ lineData: lineData?.get(), lineLinear, }); - clusterMarkers(ms, buf, { config: cluster }); + clusterMarkers(ms, buf, { + config: cluster, + minY: 0, + maxY: engine.canvasHeight.get(), + }); const map: Record = {}; for (let i = 0; i < ms.length; i++) { if (customIds[ms[i].id]) map[ms[i].id] = buf[i]; diff --git a/packages/react-native-livechart/src/components/MarkerOverlay.tsx b/packages/react-native-livechart/src/components/MarkerOverlay.tsx index 9f309752..20ce34de 100644 --- a/packages/react-native-livechart/src/components/MarkerOverlay.tsx +++ b/packages/react-native-livechart/src/components/MarkerOverlay.tsx @@ -388,7 +388,11 @@ export function MarkerOverlay({ lineData: lineData?.get(), lineLinear, }); - clusterMarkers(ms, buf, { config: cluster }); + clusterMarkers(ms, buf, { + config: cluster, + minY: 0, + maxY: engine.canvasHeight.get(), + }); const atlasFrames = atlasFrameRef.current!; atlasFrames.tick = !atlasFrames.tick; const frame = atlasFrames.tick ? atlasFrames.a : atlasFrames.b; diff --git a/packages/react-native-livechart/src/hooks/useMarkers.ts b/packages/react-native-livechart/src/hooks/useMarkers.ts index 73a75f85..01cf00b7 100644 --- a/packages/react-native-livechart/src/hooks/useMarkers.ts +++ b/packages/react-native-livechart/src/hooks/useMarkers.ts @@ -102,7 +102,11 @@ export function useMarkers( lineData: lineData?.get(), lineLinear, }); - clusterMarkers(markers.get(), buf, { config: cluster }); + clusterMarkers(markers.get(), buf, { + config: cluster, + minY: 0, + maxY: engine.canvasHeight.get(), + }); projected.set(buf); }, autostart, diff --git a/packages/react-native-livechart/src/math/markerCluster.ts b/packages/react-native-livechart/src/math/markerCluster.ts index 059f7530..76adc0e1 100644 --- a/packages/react-native-livechart/src/math/markerCluster.ts +++ b/packages/react-native-livechart/src/math/markerCluster.ts @@ -34,6 +34,12 @@ export interface ResolvedMarkerCluster { export interface ClusterMarkersOpts { config: ResolvedMarkerCluster; + /** Canvas-space y bounds (typically `0` / canvas height). When set, a + * `"vertical"` column is additionally capped where the next glyph would + * cross a bound, so a tall stack can't climb off the chart when its anchor + * is already near the edge. The base slot always draws. */ + minY?: number; + maxY?: number; } /** Glyph box used when `marker.size` is unset — mirrors `markerAtlas.DEFAULT_ICON_SIZE`. */ @@ -138,7 +144,15 @@ function layoutBucket( // `maxVisible` caps the column: the oldest glyphs keep their slots and the // newest overflow is simply hidden. const dir = side === "below" ? 1 : -1; - const cap = opts.config.maxVisible; + let cap = opts.config.maxVisible; + // Bounds clamp: slot j sits at `base + dir * j * step`; keep only slots + // whose glyph box stays inside [minY, maxY]. + if (opts.minY !== undefined && opts.maxY !== undefined && step > 0) { + const base = anchorY + sideDy; + const room = dir === -1 ? base - h / 2 - opts.minY : opts.maxY - h / 2 - base; + const fit = 1 + Math.floor(room / step); + if (fit < cap) cap = Math.max(1, fit); + } for (let j = 0; j < count; j++) { const p = proj[idx[s + j]]; if (j >= cap) { diff --git a/packages/react-native-livechart/tests/math/markerCluster.test.ts b/packages/react-native-livechart/tests/math/markerCluster.test.ts index ab4856f8..28a2f389 100644 --- a/packages/react-native-livechart/tests/math/markerCluster.test.ts +++ b/packages/react-native-livechart/tests/math/markerCluster.test.ts @@ -236,3 +236,86 @@ describe("clusterMarkers — stacked vertical", () => { expect(proj.slice(0, 5).every((p) => p.hidden && p.groupRep === 5)).toBe(true); }); }); + +describe("clusterMarkers — stacked vertical with canvas bounds", () => { + const STEP = 16 * (1 - 0.6); // glyphHeight(trade) * (1 - overlap) = 6.4 + + it("clamps an `above` column where the next glyph would cross minY", () => { + // Base at 25 - (16/2 + 2) = 15; room above = 15 - 8 = 7 → 1 + floor(7/6.4) = 2 slots. + const markers = Array.from({ length: 4 }, (_, i) => trade(`m${i}`, i + 1, "above")); + const proj = markers.map(() => pm(100, 25)); + clusterMarkers(markers, proj, { config: STACKED_VERTICAL, minY: 0, maxY: 200 }); + expect(proj[0].hidden).toBe(false); + expect(proj[0].y).toBeCloseTo(15); + expect(proj[1].hidden).toBe(false); + expect(proj[1].y).toBeCloseTo(15 - STEP); + expect(proj[2].hidden).toBe(true); // would sit at 2.2 with its top at -5.8 + expect(proj[3].hidden).toBe(true); + }); + + it("clamps a `below` column where the next glyph would cross maxY", () => { + // Base at 175 + (16/2 + 2) = 185; room below = 200 - 8 - 185 = 7 → 2 slots. + const markers = Array.from({ length: 4 }, (_, i) => trade(`m${i}`, i + 1, "below")); + const proj = markers.map(() => pm(100, 175)); + clusterMarkers(markers, proj, { config: STACKED_VERTICAL, minY: 0, maxY: 200 }); + expect(proj[0].hidden).toBe(false); + expect(proj[0].y).toBeCloseTo(185); + expect(proj[1].hidden).toBe(false); + expect(proj[1].y).toBeCloseTo(185 + STEP); + expect(proj[2].hidden).toBe(true); + expect(proj[3].hidden).toBe(true); + }); + + it("always draws the base slot even when it already overflows the bounds", () => { + // Base at 5 - 10 = -5 is above minY entirely (negative room) — cap floors at 1. + const markers = Array.from({ length: 3 }, (_, i) => trade(`m${i}`, i + 1, "above")); + const proj = markers.map(() => pm(100, 5)); + clusterMarkers(markers, proj, { config: STACKED_VERTICAL, minY: 0, maxY: 200 }); + expect(proj[0].hidden).toBe(false); + expect(proj[0].y).toBeCloseTo(-5); + expect(proj[1].hidden).toBe(true); + expect(proj[2].hidden).toBe(true); + }); + + it("applies the smaller of maxVisible and the bounds cap", () => { + const markers = Array.from({ length: 4 }, (_, i) => trade(`m${i}`, i + 1, "above")); + // Plenty of room (base 140, minY 0 fits 20+): maxVisible = 2 wins. + const roomy = markers.map(() => pm(100, 150)); + clusterMarkers(markers, roomy, { + config: { ...STACKED_VERTICAL, maxVisible: 2 }, + minY: 0, + maxY: 200, + }); + expect(roomy.map((p) => p.hidden)).toEqual([false, false, true, true]); + // Tight room (base 15 fits 2): the bounds cap wins over maxVisible = 3. + const tight = markers.map(() => pm(100, 25)); + clusterMarkers(markers, tight, { + config: { ...STACKED_VERTICAL, maxVisible: 3 }, + minY: 0, + maxY: 200, + }); + expect(tight.map((p) => p.hidden)).toEqual([false, false, true, true]); + }); + + it("ignores bounds unless both minY and maxY are provided", () => { + const markers = Array.from({ length: 4 }, (_, i) => trade(`m${i}`, i + 1, "above")); + const proj = markers.map(() => pm(100, 25)); + clusterMarkers(markers, proj, { config: STACKED_VERTICAL, minY: 0 }); + // Pass-through: the full column lays out, even past the would-be bound. + expect(proj.every((p) => !p.hidden)).toBe(true); + expect(proj[3].y).toBeCloseTo(15 - 3 * STEP); // top glyph center at -4.2 + }); + + it("skips the bounds clamp when the fan step is 0 (overlap 1)", () => { + // step = h * (1 - 1) = 0: all glyphs share the base slot; no division by 0. + const markers = Array.from({ length: 3 }, (_, i) => trade(`m${i}`, i + 1, "above")); + const proj = markers.map(() => pm(100, 25)); + clusterMarkers(markers, proj, { + config: { ...STACKED_VERTICAL, overlap: 1 }, + minY: 0, + maxY: 200, + }); + expect(proj.every((p) => !p.hidden)).toBe(true); + expect(proj.every((p) => p.y === 15)).toBe(true); + }); +});