From abfc97e97ce5cbffe7d5bc9264527bb0c0528af5 Mon Sep 17 00:00:00 2001 From: Aleksei Vesnin Date: Fri, 25 Sep 2026 13:07:01 +0300 Subject: [PATCH 1/3] fix(dives): the profile chart fits a phone, and its panel labels clear each other Co-Authored-By: Claude Opus 5.5 --- .../dives/dive-profile-chart.browser.test.tsx | 120 ++++ src/components/dives/dive-profile-chart.tsx | 613 +++++++++--------- src/hooks/useChartWidth.ts | 40 ++ src/hooks/useKeepInside.ts | 38 ++ src/lib/chart-scale.test.ts | 38 +- src/lib/chart-scale.ts | 27 + src/lib/dive-profile.ts | 14 +- src/test/chart-layout.ts | 45 ++ 8 files changed, 627 insertions(+), 308 deletions(-) create mode 100644 src/components/dives/dive-profile-chart.browser.test.tsx create mode 100644 src/hooks/useChartWidth.ts create mode 100644 src/hooks/useKeepInside.ts create mode 100644 src/test/chart-layout.ts diff --git a/src/components/dives/dive-profile-chart.browser.test.tsx b/src/components/dives/dive-profile-chart.browser.test.tsx new file mode 100644 index 00000000..00aca6a5 --- /dev/null +++ b/src/components/dives/dive-profile-chart.browser.test.tsx @@ -0,0 +1,120 @@ +import { describe, expect, it, vi } from "vitest"; +import { fireEvent, render } from "@testing-library/react"; + +import { DiveProfileChart } from "./dive-profile-chart"; +import type { DiveProfile } from "@/lib/api/dives"; +import { CHART_FULL_WIDTH_PX } from "@/lib/chart-scale"; +import { + overlappingLabels, + sidewaysScrollers, + withinSides, +} from "@/test/chart-layout"; + +// Load-bearing: the browser project loads none of this app's Tailwind, and +// without it the scroll wrapper this file guards against has no `min-width` to +// overflow with, so every assertion below would pass against it. The first test +// fails loudly if this import goes. +import "@/app/globals.css"; + +vi.mock("@/contexts/AuthContext", () => ({ + useAuth: () => ({ user: { uuid: "user-1", units: "metric" } }), +})); + +// A 19 m dive with a full deco panel. Depth's axis ends on 20 and the NDL row's +// begins at 100 min, which is the pair that touched; the ppO₂ and percent rows +// put two more row boundaries under it, and temperature labels the right edge. +const times = Array.from({ length: 11 }, (_, index) => index * 300_000); +const PROFILE: DiveProfile = { + duration: 3_000_000, + depth: { + times, + values: [0, 1200, 1900, 1900, 1800, 1700, 1500, 1200, 800, 500, 0], + }, + temperature: { + times, + values: [260, 250, 240, 235, 232, 230, 230, 232, 238, 245, 252], + }, + pressures: [], + ndl: { + times, + values: [5940, 3600, 1800, 1500, 1500, 1600, 1900, 2400, 3600, 5940, 5940], + }, + tts: { times, values: [0, 60, 120, 180, 180, 120, 60, 60, 0, 0, 0] }, + ppo2: { times, values: [21, 45, 61, 61, 59, 57, 53, 46, 38, 31, 21] }, + cns: { times, values: [0, 2, 4, 6, 8, 9, 10, 11, 12, 12, 12] }, + gradient_factor: { + times, + values: [0, 10, 25, 38, 45, 52, 58, 60, 55, 40, 20], + }, + events: [], +}; + +// The chart's width on a dive page at a 375 px and a 320 px viewport, and at +// 1024 px, where the page gets its sidebar. +const WIDTHS = { phone: 293, smallPhone: 238, desktop: 582 }; + +function renderAt(width: number) { + const { container } = render( +
+ +
, + ); + const frame = container.firstElementChild as HTMLElement; + const svg = frame.querySelector("svg") as SVGSVGElement; + return { frame, svg }; +} + +describe("the dive profile chart's layout", () => { + it("is laid out by the app's stylesheet", () => { + // What makes the rest of this file mean anything - see the import above. + const { svg } = renderAt(WIDTHS.phone); + + expect(getComputedStyle(svg.parentElement as Element).position).toBe( + "relative", + ); + }); + + it.each(Object.entries(WIDTHS))( + "draws no label over another at %s width", + (_, width) => { + const { svg } = renderAt(width); + + expect(overlappingLabels(svg)).toEqual([]); + }, + ); + + it.each(Object.entries(WIDTHS))( + "fits %s width without scrolling sideways", + (_, width) => { + const { frame, svg } = renderAt(width); + + expect(sidewaysScrollers(svg, frame)).toEqual([]); + }, + ); + + it("keeps the type at the size it has at the full-width breakpoint", () => { + // Fitting a phone by shrinking the whole drawing would pass both tests + // above with labels a third of their size. + const { svg } = renderAt(WIDTHS.phone); + const scale = + svg.getBoundingClientRect().width / svg.viewBox.baseVal.width; + + expect(scale).toBeCloseTo(CHART_FULL_WIDTH_PX / 720); + }); + + it("keeps the hover card inside the chart wherever the crosshair is", () => { + const { frame, svg } = renderAt(WIDTHS.phone); + const target = svg.querySelector('rect[fill="transparent"]') as Element; + const box = target.getBoundingClientRect(); + + for (let step = 0; step <= 50; step++) { + fireEvent.mouseMove(target, { + clientX: box.left + (box.width * step) / 50, + clientY: box.top + box.height / 3, + }); + const card = frame.querySelector('[role="presentation"]') as Element; + + expect(withinSides(card, frame), `at ${step * 2}%`).toBe(true); + } + }); +}); diff --git a/src/components/dives/dive-profile-chart.tsx b/src/components/dives/dive-profile-chart.tsx index a48dfe3b..1ae199d0 100644 --- a/src/components/dives/dive-profile-chart.tsx +++ b/src/components/dives/dive-profile-chart.tsx @@ -4,6 +4,7 @@ import { useEffect, useId, useMemo, + useRef, useState, useSyncExternalStore, } from "react"; @@ -12,7 +13,12 @@ import type { DiveProfileEvent, DiveProfileEventType, } from "@/lib/api/dives"; -import { axisTicks, niceDomain, type Domain } from "@/lib/chart-scale"; +import { + axisTicks, + labelCapacity, + niceDomain, + type Domain, +} from "@/lib/chart-scale"; import { buildAreaPath } from "@/lib/chart-path"; import { type ChannelSeries, @@ -21,6 +27,7 @@ import { type ProfileChannelKey, type ProfileSeriesKey, type ProfileViewKey, + ELAPSED_TICK_TARGET, EVENTS_LABEL, PANEL_AXES, PROFILE_CHANNELS, @@ -56,6 +63,8 @@ import { writeSeriesVisibility, } from "@/lib/chart-series-view"; import { cn } from "@/lib/utils"; +import { useChartWidth } from "@/hooks/useChartWidth"; +import { useKeepInside } from "@/hooks/useKeepInside"; import { useUnits } from "@/hooks/useUnits"; import type { UnitSystem } from "@/lib/units"; @@ -73,16 +82,22 @@ import type { UnitSystem } from "@/lib/units"; // grey in dark, which left the gas chart's trend line barely visible. // The viewBox coordinate space. Not pixels: the SVG scales to its container, so -// these are only ever ratios to each other. +// these are only ever ratios to each other. This is the design width; a phone +// draws into a narrower one (see `useChartWidth`), so only the heights below are +// fixed. const WIDTH = 720; // Wider on both sides than the gas chart: depth is on the left and temperature // and pressure share the right, so both margins carry axis labels. const PADDING = { top: 14, right: 46, bottom: 28, left: 44 }; -const PLOT_WIDTH = WIDTH - PADDING.left - PADDING.right; const PLOT_HEIGHT = 238; const PLOT_BOTTOM = PADDING.top + PLOT_HEIGHT; +// The room each elapsed-time label gets on the x axis: "999:00" is 37.5 units +// wide in 11-unit Inter, plus air. At the design width the axis is never short of +// it, so this only thins the labels on a phone. +const ELAPSED_LABEL_SPACING = 44; + // The deco panel: one short plot per unit the depth plot's two edges cannot // carry, stacked under it and sharing its elapsed-time axis. See // `profileScalePlacement` for why the split is forced rather than chosen. @@ -91,7 +106,10 @@ const PLOT_BOTTOM = PADDING.top + PLOT_HEIGHT; // edges are untouched by anything the diver switches on down here, and every // curve on the chart is drawn against numbers that belong to it. const PANEL_HEIGHT = 46; -const PANEL_GAP = 12; +// The labels either side of a gap each sit centred on their own rule, so the gap +// has to hold half of each: 11- and 10-unit type is about 1.2 em tall, 12.6 units +// between them, and 16 leaves a few units of air. +const PANEL_GAP = 16; const panelTop = (index: number) => PLOT_BOTTOM + PANEL_GAP + index * (PANEL_HEIGHT + PANEL_GAP); @@ -232,6 +250,8 @@ export function DiveProfileChart({ profile }: DiveProfileChartProps) { // fragment that has to resolve it. What survives the strip is still the part // that differs between two ids on one page. const clipPrefix = useId().replace(/[^a-zA-Z0-9_-]/g, ""); + const [chartRef, width] = useChartWidth(WIDTH); + const plotWidth = width - PADDING.left - PADDING.right; // One hovered *time*, not one hovered sample, and one piece of state for the // whole chart - the same call `GasUseChart` makes, for the same reason. It // can't be an index here: the channels are independently sampled and don't @@ -282,7 +302,7 @@ export function DiveProfileChart({ profile }: DiveProfileChartProps) { const duration = profile.duration; const x = (at: number) => - PADDING.left + (duration > 0 ? at / duration : 0) * PLOT_WIDTH; + PADDING.left + (duration > 0 ? at / duration : 0) * plotWidth; // Markers that land inside the plot, which is this chart's job rather than the // API's and is stated as such at the other end: `_rebase_events` clamps the low @@ -748,7 +768,7 @@ export function DiveProfileChart({ profile }: DiveProfileChartProps) { : nearestEvent( events, hoveredMs, - (duration / PLOT_WIDTH) * EVENT_HOVER_UNITS, + (duration / plotWidth) * EVENT_HOVER_UNITS, ); // One binary search per channel, not one shared index lookup: the channels are @@ -800,43 +820,28 @@ export function DiveProfileChart({ profile }: DiveProfileChartProps) { return (
- {/* Wide content scrolls in its own container rather than shrinking the - whole chart to phone width, where three axes' labels would become - unreadable - the same treatment the gas chart and the gas mixtures - table get. The legend deliberately sits *outside* it: it's text, so it - should wrap to the screen rather than scroll sideways with the plot, - and on a phone the container's own horizontal scrollbar is drawn - across the bottom of whatever it contains - straight through the - legend. */} - {/* `relative` at the *viewport's* width rather than the plot's, which is - what the empty-plot message below is positioned against. Centring it on - the 560-unit plot box instead puts it at x≈280 of a box that is wider - than a phone, so on a 375 px screen the sentence starts near the right - edge and runs off it - and the one thing that has to be readable - without scrolling is the sentence explaining why there is nothing to - scroll to. */} -
-
- {/* Sized to exactly the chart, and the positioning context the - tooltip's percentage offsets are resolved against. */} -
-
+ - - {hoveredMs !== null && (readouts.length > 0 || hoveredEvent) && ( - 0 - ? Math.min(...dots.map((readout) => readout.cy)) - : PLOT_BOTTOM - } - /> - )} -
-
+ { + const bounds = event.currentTarget.getBoundingClientRect(); + const ratio = (event.clientX - bounds.left) / bounds.width; + setHoveredMs(Math.min(duration, Math.max(0, ratio * duration))); + }} + onMouseLeave={() => setHoveredMs(null)} + /> + + + {hoveredMs !== null && (readouts.length > 0 || hoveredEvent) && ( + 0 + ? Math.min(...dots.map((readout) => readout.cy)) + : PLOT_BOTTOM + } + /> + )} {/* Switching the last channel off used to return a bare sentence in place of the whole chart, which collapsed the card to two lines and @@ -1176,17 +1184,12 @@ export function DiveProfileChart({ profile }: DiveProfileChartProps) { the middle of it. Nothing moves, and the toggle that undid this is still under the cursor that clicked it. - A sibling of the scroll container rather than a child of the plot - box, so it centres on what the diver can see - see the note on the - `relative` wrapper above. - Not shown while the markers are up, even with every curve hidden: the plot has content then, and "pick one below to plot it" printed across a row of markers describes a chart nobody is looking at. `pointer-events-none` so the hit target underneath still tracks the - crosshair, which markers are still worth hovering for - and so the - plot underneath can still be scrolled sideways. */} + crosshair, which markers are still worth hovering for. */} {shown.length === 0 && !eventsShown && (

@@ -1395,6 +1398,7 @@ function ProfileTooltip({ readouts, event, cx, + chartWidth, chartHeight, topmostY, }: { @@ -1402,39 +1406,42 @@ function ProfileTooltip({ readouts: Readout[]; event: DiveProfileEvent | null; cx: number; + chartWidth: number; chartHeight: number; topmostY: number; }) { - // The card always lands inside the chart box, in both axes. It has to: the - // scroll container around it clips (setting `overflow-x` to `auto` makes - // `overflow-y` compute to `auto` as well), so anything hanging past an edge is - // cut off or adds a stray scrollbar. + // The card always lands inside the chart box, in both axes: past an edge it + // would cover whatever sits beside the chart, or on a phone scroll the page + // sideways. // // Vertically that is guaranteed by anchoring to the plot's own top or bottom // edge rather than offsetting from a data point - see `tooltipVerticalAnchor` // for why offsetting from a point cannot be made safe here. Horizontally the // card is centred on the crosshair and flips to hug whichever edge it is near, - // which is safe because its width is bounded by `whitespace-nowrap` on short - // readouts. + // and `useKeepInside` pulls back in whatever a phone-width chart still leaves + // hanging over. + const cardRef = useRef(null); + useKeepInside(cardRef); const { y, translateY } = tooltipVerticalAnchor( topmostY, PADDING.top, PLOT_BOTTOM, ); const translateX = - cx < WIDTH * 0.2 + cx < chartWidth * 0.2 ? "-12px" - : cx > WIDTH * 0.8 + : cx > chartWidth * 0.8 ? "calc(-100% + 12px)" : "-50%"; return (

{describeEvent(event)} diff --git a/src/hooks/useChartWidth.ts b/src/hooks/useChartWidth.ts new file mode 100644 index 00000000..4024834b --- /dev/null +++ b/src/hooks/useChartWidth.ts @@ -0,0 +1,40 @@ +"use client"; + +import { type RefCallback, useCallback, useState } from "react"; +import { fittedChartWidth } from "@/lib/chart-scale"; + +/** + * The viewBox width for a chart designed `width` units wide, fitted to the + * element the returned ref is on - see `fittedChartWidth` for the rule. + * + * Read as the ref attaches, which is during commit, so the first paint already + * draws at the fitted width instead of flashing the design width at phone size; + * a `ResizeObserver` follows the element after that. `offsetWidth` in both + * places, so the two readings of one layout agree and cost no second render. + * + * @example + * const [chartRef, chartWidth] = useChartWidth(720); + * return ( + *
+ * + *
+ * ); + */ +export function useChartWidth( + width: number, +): [RefCallback, number] { + const [containerPx, setContainerPx] = useState(null); + + const ref = useCallback((element: HTMLElement | null) => { + if (!element) return; + + setContainerPx(element.offsetWidth); + const observer = new ResizeObserver(() => + setContainerPx(element.offsetWidth), + ); + observer.observe(element); + return () => observer.disconnect(); + }, []); + + return [ref, fittedChartWidth(width, containerPx)]; +} diff --git a/src/hooks/useKeepInside.ts b/src/hooks/useKeepInside.ts new file mode 100644 index 00000000..48f2067f --- /dev/null +++ b/src/hooks/useKeepInside.ts @@ -0,0 +1,38 @@ +"use client"; + +import { type RefObject, useLayoutEffect } from "react"; + +/** + * Nudges an absolutely positioned element sideways, after every render and + * before paint, so it stays inside the box it is positioned against (its + * `offsetParent`). One wider than that box is aligned to its left edge. + * + * For the charts' hover cards, which are placed at the point they describe and + * are as wide as what they say. At phone width a card near either edge would + * otherwise hang past the chart and scroll the page sideways. + * + * Written to the `translate` property, which composes with the `transform` the + * card's own placement sets instead of replacing it, and which React leaves + * alone because no `style` prop names it. + * + * @example + * const cardRef = useRef(null); + * useKeepInside(cardRef); + * return
; + */ +export function useKeepInside(ref: RefObject): void { + useLayoutEffect(() => { + const element = ref.current; + const frame = element?.offsetParent; + if (!element || !frame) return; + + element.style.translate = ""; + const outer = frame.getBoundingClientRect(); + const inner = element.getBoundingClientRect(); + const shift = Math.max( + outer.left - inner.left, + Math.min(0, outer.right - inner.right), + ); + if (shift !== 0) element.style.translate = `${shift}px`; + }); +} diff --git a/src/lib/chart-scale.test.ts b/src/lib/chart-scale.test.ts index 13bc5af5..205abadd 100644 --- a/src/lib/chart-scale.test.ts +++ b/src/lib/chart-scale.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from "vitest"; -import { axisTicks, countDomain, niceDomain } from "@/lib/chart-scale"; +import { + CHART_FULL_WIDTH_PX, + axisTicks, + countDomain, + fittedChartWidth, + labelCapacity, + niceDomain, +} from "@/lib/chart-scale"; // Moved verbatim from `dive-gas.test.ts` along with the functions themselves; // the examples are still phrased in RMV because that is the series they were @@ -96,3 +103,32 @@ describe("countDomain", () => { expect(countDomain(0)).toEqual({ min: 0, max: 1, step: 1 }); }); }); + +describe("fittedChartWidth", () => { + it("keeps the design width from the full-width container up", () => { + expect(fittedChartWidth(720, CHART_FULL_WIDTH_PX)).toBe(720); + expect(fittedChartWidth(720, 1100)).toBe(720); + }); + + it("narrows in step with a narrower container, so the scale holds", () => { + // A 375px phone's card leaves the chart 293px. + const width = fittedChartWidth(720, 293); + + expect(293 / width).toBeCloseTo(CHART_FULL_WIDTH_PX / 720); + }); + + it("uses the design width for a container it has not measured", () => { + expect(fittedChartWidth(720, null)).toBe(720); + expect(fittedChartWidth(720, 0)).toBe(720); + }); +}); + +describe("labelCapacity", () => { + it("counts whole labels only", () => { + expect(labelCapacity(664, 33)).toBe(20); + }); + + it("never offers fewer than one", () => { + expect(labelCapacity(10, 33)).toBe(1); + }); +}); diff --git a/src/lib/chart-scale.ts b/src/lib/chart-scale.ts index f8f72f7a..ff0be5e1 100644 --- a/src/lib/chart-scale.ts +++ b/src/lib/chart-scale.ts @@ -101,6 +101,33 @@ export function countDomain(highest: number, targetTicks = 5): Domain { return { min: 0, max: top, step: top }; } +// The narrowest container, in CSS pixels, a chart is drawn into at its full +// viewBox width. Its 11-unit axis type renders at 8.6px there, which is as small +// as any of these charts ever drew it: they used to hold this as a minimum width +// and scroll sideways below it. +export const CHART_FULL_WIDTH_PX = 560; + +// The viewBox width for a chart designed `width` units wide, in a container +// `containerPx` wide. From `CHART_FULL_WIDTH_PX` up it is the design width, so a +// desktop layout draws exactly as designed. Below it the viewBox narrows in step +// with the container, which keeps the scale - and so the type size - the chart +// has at that width: a phone gets a narrower plot, not smaller labels and not a +// scrollbar. Unmeasured (`null` or `0`, which is also what jsdom reports) gets +// the design width. +export function fittedChartWidth( + width: number, + containerPx: number | null, +): number { + if (!containerPx || containerPx >= CHART_FULL_WIDTH_PX) return width; + return (width * containerPx) / CHART_FULL_WIDTH_PX; +} + +// How many labels fit along `plotWidth` viewBox units when each needs `spacing` +// of them, never fewer than one. +export function labelCapacity(plotWidth: number, spacing: number): number { + return Math.max(1, Math.floor(plotWidth / spacing)); +} + // The gridline values for a domain, inclusive of both ends. Built by counting // steps rather than by accumulating `+= step`, which drifts on fractional steps // (0.1 + 0.2 territory) and produces labels like "12.499999999999998". diff --git a/src/lib/dive-profile.ts b/src/lib/dive-profile.ts index 9cd2b81d..b4ad7e97 100644 --- a/src/lib/dive-profile.ts +++ b/src/lib/dive-profile.ts @@ -1000,9 +1000,15 @@ const ELAPSED_STEPS_MS = [1, 2, 5, 10, 15, 30, 60, 120].map( (minutes) => minutes * 60 * MILLISECONDS_PER_SECOND, ); +// How many elapsed-time steps the axis aims for at the chart's design width. +export const ELAPSED_TICK_TARGET = 6; + // The elapsed-time gridlines for a profile spanning `durationMs`, in // milliseconds, starting at 0 and never running past the end of the dive. -export function elapsedTicks(durationMs: number, targetTicks = 6): number[] { +export function elapsedTicks( + durationMs: number, + targetTicks = ELAPSED_TICK_TARGET, +): number[] { if (durationMs <= 0) return [0]; const step = @@ -1032,9 +1038,9 @@ export function elapsedTicks(durationMs: number, targetTicks = 6): number[] { // card is, and here you don't: its height depends on how many channels the // dive recorded, and the SVG scales to its container while the card's text // does not. A fixed "flip above the point when it's in the top third" rule -// put the card 11 px past the top edge of a scroll container that clips -// (`overflow-x: auto` computes `overflow-y` to `auto` too), so it was cut -// off. Anchoring to an edge is correct for *any* card height and any scale. +// put the card 11 px past the top edge of the chart, over whatever sits +// above it. Anchoring to an edge is correct for *any* card height and any +// scale. // // The card moves to the bottom when the topmost dot is high in the plot, so it // doesn't cover the readings it is describing. diff --git a/src/test/chart-layout.ts b/src/test/chart-layout.ts new file mode 100644 index 00000000..5dcf8ae1 --- /dev/null +++ b/src/test/chart-layout.ts @@ -0,0 +1,45 @@ +// Layout checks for the hand-rolled SVG charts, for the browser lane only. jsdom +// lays nothing out, so both come back empty there whatever the markup is. + +// Every pair of `` elements in `svg` whose rendered boxes intersect, +// named by what they say. +export function overlappingLabels(svg: SVGSVGElement): string[] { + const labels = [...svg.querySelectorAll("text")].map((text) => ({ + text: text.textContent ?? "", + box: text.getBoundingClientRect(), + })); + + return labels.flatMap((a, index) => + labels + .slice(index + 1) + .filter( + (b) => + a.box.left < b.box.right && + b.box.left < a.box.right && + a.box.top < b.box.bottom && + b.box.top < a.box.bottom, + ) + .map((b) => `${a.text} / ${b.text}`), + ); +} + +// Every element from `element`'s parent up to and including `root` that is +// wider inside than out - that is, that scrolls sideways. +export function sidewaysScrollers(element: Element, root: Element): string[] { + const found: string[] = []; + for (let node = element.parentElement; node; node = node.parentElement) { + if (node.scrollWidth > node.clientWidth) { + found.push(`${node.className} ${node.scrollWidth} > ${node.clientWidth}`); + } + if (node === root) break; + } + return found; +} + +// Whether `inner`'s box lies within `outer`'s left and right edges, to within +// half a pixel of rounding. +export function withinSides(inner: Element, outer: Element): boolean { + const a = inner.getBoundingClientRect(); + const b = outer.getBoundingClientRect(); + return a.left >= b.left - 0.5 && a.right <= b.right + 0.5; +} From ec538b2274e4ed5ad24ee1ad9a6843e05322a065 Mon Sep 17 00:00:00 2001 From: Aleksei Vesnin Date: Fri, 25 Sep 2026 13:10:43 +0300 Subject: [PATCH 2/3] fix(dashboard): the gas and activity charts fit a phone too Co-Authored-By: Claude Opus 5.5 --- DECISIONS.md | 22 +- .../dashboard/dashboard-page-frame.tsx | 9 +- src/components/dives/chart-skeleton.tsx | 5 +- .../dive-activity-chart.browser.test.tsx | 102 +++++ src/components/dives/dive-activity-chart.tsx | 299 ++++++------- .../dives/dive-profile-chart.browser.test.tsx | 9 +- .../dives/gas-use-chart.browser.test.tsx | 105 +++++ src/components/dives/gas-use-chart.tsx | 396 +++++++++--------- src/lib/chart-scale.ts | 5 +- src/lib/dive-profile.test.ts | 7 + src/lib/dive-profile.ts | 5 +- 11 files changed, 603 insertions(+), 361 deletions(-) create mode 100644 src/components/dives/dive-activity-chart.browser.test.tsx create mode 100644 src/components/dives/gas-use-chart.browser.test.tsx diff --git a/DECISIONS.md b/DECISIONS.md index ee857d27..a691ba73 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -974,9 +974,9 @@ in both themes. The y axis is not zero-based: `niceDomain()` rounds outward from the data, with 2.5 in its progression so a 5-to-26 spread does not step by 10. -Each dot is a plain SVG ``, not `next/link`. Below ~560px the chart scrolls inside -`overflow-x-auto`. The card renders when empty, unlike `ServiceDueCard`: missing pressures or an -average depth are something the diver can fix. +Each dot is a plain SVG ``, not `next/link`. Below 560px the chart narrows its viewBox rather +than scrolling (`fittedChartWidth`). The card renders when empty, unlike `ServiceDueCard`: missing +pressures or an average depth are something the diver can fix. ## The chart windows to All/Year/Month, but scales itself from the whole series @@ -1004,8 +1004,8 @@ point: the same state drives the dot's enlarge-and-brighten, so dot and card can It is positioned in percentages of the chart box (the SVG scales uniformly in a wrapper of its own size) through the `style` prop, an inline attribute the CSP allows (`style-src-attr 'unsafe-inline'`). It flips to stay inside the box — below the dot in the top -third, edge-aligned within 18% of either side — because `overflow-x: auto` on the scroll container -computes `overflow-y` to `auto` and clips. +third, edge-aligned within 18% of either side — and `useKeepInside` pulls in whatever a phone-width +chart still leaves past an edge, where it would scroll the page sideways. Each dot has an invisible `r=7` hit circle with `fill="transparent"`, not `fill="none"`, which takes no pointer events. The card is `pointer-events-none` so it cannot steal the hover, has no accessible @@ -1050,8 +1050,7 @@ maps the cursor to an instant on the profile's millisecond axis once and each ch own sample with `nearestSampleIndex`. Readouts are real readings, never interpolations. `tooltipVerticalAnchor` pins the card to the plot's top or bottom edge, whichever keeps it off the -readings: card height depends on channel count, so offsetting from a point overflows the clipping -scroll container. +readings: card height depends on channel count, so offsetting from a point overflows the chart. Keyboard scrubbing is out of scope. The `aria-label` uses `formatDurationHoursMinutes`, not `MM:SS`. `--pressure` is a third theme-stable token in `globals.css`, violet. @@ -1974,11 +1973,10 @@ label size reads as a rendering fault. ## The two chart cards stack, and gas leads - both measured, not assumed -Each plot carries `min-w-[560px]`, which keeps twelve month labels and a y axis legible. -`lg:grid-cols-2` on the dashboard's `max-w-6xl` gives 482px (546px at `max-w-7xl`), and three things -break: both charts clip and grow a horizontal scrollbar, the axis text halves (16.6px to 8.6px, -since the svg scales uniformly), and the gas header goes from 50px to 114px as its toggle and -stepper drop below the description. Clearing all three needs about 1220px. +Below 560px each plot narrows its viewBox instead of shrinking, so its axis text stops at 8.6px +(`fittedChartWidth`). `lg:grid-cols-2` on the dashboard's `max-w-6xl` gives 482px (546px at +`max-w-7xl`), and two things break: the axis text halves (16.6px to 8.6px), and the gas header goes +from 50px to 114px as its toggle and stepper drop below the description. `RecentDivesCard`/`RecentTripsCard` pair up fine below: their content reflows instead of scaling. Gas consumption leads because it can change how you dive tomorrow; activity records what already diff --git a/src/components/dashboard/dashboard-page-frame.tsx b/src/components/dashboard/dashboard-page-frame.tsx index 7cb5155b..1e6c5976 100644 --- a/src/components/dashboard/dashboard-page-frame.tsx +++ b/src/components/dashboard/dashboard-page-frame.tsx @@ -247,11 +247,10 @@ export function DashboardPageFrame({ already happened - and it's the harder-won number, since it needs dives that recorded pressures and an average depth. - Stacked, not side by side, and that was measured rather than assumed: each - plot needs 560px to keep twelve month labels legible, and a two-column grid - gives it 482px even on a widened page. Both charts clip, their axis text - halves, and the gas card's header doubles in height when its controls can no - longer share a line with its description. See DECISIONS.md. */} + Stacked, not side by side, and that was measured rather than assumed: a + two-column grid gives each plot 482px even on a widened page, where their + axis text halves and the gas card's header doubles in height when its + controls can no longer share a line with its description. See DECISIONS.md. */} {hasDives && } {hasDives && } diff --git a/src/components/dives/chart-skeleton.tsx b/src/components/dives/chart-skeleton.tsx index 3e9e0cea..7a42e9ce 100644 --- a/src/components/dives/chart-skeleton.tsx +++ b/src/components/dives/chart-skeleton.tsx @@ -5,7 +5,8 @@ import { Skeleton } from "@/components/ui/skeleton"; * summary figures over a `720 x 240` SVG drawn at `w-full h-auto`, so the * placeholder reserves the same `3:1` box - a card that collapsed to a spinner * and then grew back to chart height was most of what made arriving at the - * dashboard feel jumpy. + * dashboard feel jumpy. Below 560px the charts keep the height they have there + * (see `fittedChartWidth`), and so does this. * * `legend` covers the one difference between them: the gas chart carries a * `text-xs` legend under its plot (it doubles as the control for which series @@ -29,7 +30,7 @@ export function ChartSkeleton({
))}
- + {legend && }
); diff --git a/src/components/dives/dive-activity-chart.browser.test.tsx b/src/components/dives/dive-activity-chart.browser.test.tsx new file mode 100644 index 00000000..5354b3c6 --- /dev/null +++ b/src/components/dives/dive-activity-chart.browser.test.tsx @@ -0,0 +1,102 @@ +import { describe, expect, it } from "vitest"; +import { fireEvent, render } from "@testing-library/react"; + +import { DiveActivityChart } from "./dive-activity-chart"; +import type { DiveActivityPoint } from "@/lib/api/dive-stats"; +import type { ChartScope } from "@/lib/chart-period"; +import { activityBars, barCeiling } from "@/lib/dive-activity"; +import { + overlappingLabels, + sidewaysScrollers, + withinSides, +} from "@/test/chart-layout"; + +// Load-bearing, as in `dive-profile-chart.browser.test.tsx`, whose first test +// fails without it: without the app's Tailwind a `min-w-*` class computes to +// nothing, so a chart wider than its container would pass everything below. +import "@/app/globals.css"; + +// Twenty-five years of diving, and a 2025 with a dive in every month and on +// every day of August: the widest each scope's axis gets. +const POINTS: DiveActivityPoint[] = [ + ...Array.from({ length: 25 }, (_, index) => ({ + year: 2002 + index, + month: 6, + day: 15, + dives: 1 + (index % 4), + })), + ...Array.from({ length: 12 }, (_, index) => ({ + year: 2025, + month: index + 1, + day: 3, + dives: 2, + })), + ...Array.from({ length: 31 }, (_, index) => ({ + year: 2025, + month: 8, + day: index + 1, + dives: 1 + (index % 3), + })), +]; + +const ANCHOR = Date.UTC(2025, 7, 10); + +// The chart's width on the dashboard at a 375 px and a 320 px viewport, and at +// 1024 px. +const WIDTHS = { phone: 293, smallPhone: 238, desktop: 910 }; +const SCOPES: ChartScope[] = ["all", "year", "month"]; + +const CASES = Object.entries(WIDTHS).flatMap(([name, width]) => + SCOPES.map((scope) => [name, scope, width] as const), +); + +function renderAt(width: number, scope: ChartScope) { + const { container } = render( +
+ +
, + ); + const frame = container.firstElementChild as HTMLElement; + const svg = frame.querySelector("svg") as SVGSVGElement; + return { frame, svg }; +} + +describe("the dive activity chart's layout", () => { + it.each(CASES)( + "draws no label over another at %s width, scope %s", + (_, scope, width) => { + const { svg } = renderAt(width, scope); + + expect(overlappingLabels(svg)).toEqual([]); + }, + ); + + it.each(CASES)( + "fits %s width without scrolling sideways, scope %s", + (_, scope, width) => { + const { frame, svg } = renderAt(width, scope); + + expect(sidewaysScrollers(svg, frame)).toEqual([]); + }, + ); + + it.each(SCOPES)( + "keeps the hover card inside a phone-width chart, scope %s", + (scope) => { + const { frame, svg } = renderAt(WIDTHS.smallPhone, scope); + const columns = [...svg.querySelectorAll('rect[fill="transparent"]')]; + + for (const column of [columns[0], columns[columns.length - 1]]) { + fireEvent.mouseEnter(column); + const card = frame.querySelector('[role="presentation"]') as Element; + + expect(withinSides(card, frame)).toBe(true); + fireEvent.mouseLeave(column); + } + }, + ); +}); diff --git a/src/components/dives/dive-activity-chart.tsx b/src/components/dives/dive-activity-chart.tsx index 3c168bc5..eaac7148 100644 --- a/src/components/dives/dive-activity-chart.tsx +++ b/src/components/dives/dive-activity-chart.tsx @@ -1,11 +1,13 @@ "use client"; -import { useState } from "react"; +import { useRef, useState } from "react"; import { barPath } from "@/lib/chart-path"; -import { axisTicks, countDomain } from "@/lib/chart-scale"; +import { axisTicks, countDomain, labelCapacity } from "@/lib/chart-scale"; import type { ChartScope } from "@/lib/chart-period"; import type { ActivityBar } from "@/lib/dive-activity"; import { cn } from "@/lib/utils"; +import { useChartWidth } from "@/hooks/useChartWidth"; +import { useKeepInside } from "@/hooks/useKeepInside"; // Hand-rolled SVG, for the reasons `gas-use-chart.tsx` sets out at length: the // app ships a strict nonce-based CSP that a `