diff --git a/frontend/src/components/Common/UplotChart.tsx b/frontend/src/components/Common/UplotChart.tsx index 362d0f4..c3f2850 100644 --- a/frontend/src/components/Common/UplotChart.tsx +++ b/frontend/src/components/Common/UplotChart.tsx @@ -30,8 +30,8 @@ export const CHART_SLOTS = Array.from({ length: MAX_SERIES }, (_, index) => String(index + 1), ) -/** What a chart draws with when nothing chose for it: the ramp, in order. */ -export const DEFAULT_PALETTE = CHART_SLOTS +/** No palette named; a chart left with this spreads itself. */ +export const NO_PALETTE: string[] = [] /** * A stored palette, made safe to draw with: a distinct subset, in draw order. @@ -42,21 +42,47 @@ export const DEFAULT_PALETTE = CHART_SLOTS * close, and two on the *same* slot could not be told apart at all. Free * assignment with repeats is not an improvement on this; it is the bug. * - * Anything unrecognised, repeated or empty falls back to the whole ramp, which - * is what every chart drew before a dashboard could name a palette. + * Empty is the answer for a dashboard that names none, and for anything + * unrecognised: no palette, so the chart spreads itself — see `slotsFor`. */ export function paletteOf(value: unknown): string[] { - if (!Array.isArray(value)) return DEFAULT_PALETTE + if (!Array.isArray(value)) return NO_PALETTE const slots = [...new Set(value.map(String))].filter((slot) => CHART_SLOTS.includes(slot), ) - return slots.length > 0 ? slots : DEFAULT_PALETTE + // One shared array for the common answer, so a dashboard that names no + // palette does not hand its readers a new one on every render. + return slots.length > 0 ? slots : NO_PALETTE } -/** Which slot of the ramp the *index*th series takes from a palette. */ -export function slotFor(index: number, palette: string[] = DEFAULT_PALETTE) { - const slots = palette.length > 0 ? palette : DEFAULT_PALETTE - return slots[index % slots.length] +/** + * What a chart of *n* lines draws with when no palette was named: the ramp + * spread, rather than its first *n* steps. + * + * Three lines used to take slots 1, 2 and 3, which are adjacent steps of a + * ramp that carries identity by lightness alone — at a 2px stroke they are + * close to one picture. The ends and the middle are as far apart as five slots + * allow, and the same reasoning gives every other count. + * + * This moved after the palette shipped, and moving it was safe *because* of + * how the palette is stored: a named palette is honoured exactly as written, + * so no stored document means anything different than it did. Only the + * unwritten default moved, and any dashboard that dislikes where it moved to + * can now name the old one. That inertness is the property worth keeping — + * not this table, which is a design decision and may move again. + */ +const SPREADS: Record = { + 1: ["1"], + 2: ["1", "5"], + 3: ["1", "3", "5"], + 4: ["1", "2", "4", "5"], + 5: ["1", "2", "3", "4", "5"], +} + +/** The slots a chart of `count` lines draws with, named palette or not. */ +export function slotsFor(count: number, palette?: string[]): string[] { + if (palette?.length) return palette + return SPREADS[Math.min(Math.max(Math.trunc(count) || 1, 1), MAX_SERIES)] } /** @@ -101,8 +127,8 @@ function token(name: string): string { .trim() } -const seriesColor = (index: number, palette?: string[]) => - token(`--chart-${slotFor(index, palette)}`) +const seriesColor = (index: number, slots: string[]) => + token(`--chart-${slots[index % slots.length]}`) /** * Tick labels that stay distinct. @@ -173,8 +199,9 @@ export function UplotChart({ /** A named y axis; the unit stays on the ticks. */ yLabel?: string /** The ramp slots this chart's lines take, in order — the dashboard's own - * palette. Unset is the whole ramp, which is what a chart outside a - * dashboard (Health, Home) draws with. */ + * palette. Unset, the chart spreads itself across the ramp by how many + * lines it draws, which is what one outside a dashboard (Health, Home) + * always does. */ palette?: string[] /** Draw the lines as a monotone cubic spline rather than straight segments. * Monotone rather than plain cubic on purpose: a spline that overshoots @@ -223,6 +250,9 @@ export function UplotChart({ * does not re-render it. */ let told: number | null = null const refine = cursorRefiner() + // Resolved once for the whole chart: how many lines there are is part of + // which slots they take, when nothing named them. + const slots = slotsFor(labels.length, palette) // One builder for the whole chart: it is a factory, and the series only // need the function it returns. const spline = smooth ? { paths: uPlot.paths.spline?.() } : {} @@ -295,7 +325,7 @@ export function UplotChart({ width: 2, // Read at draw time, so a theme toggle is a redraw rather than a // rebuilt chart. - stroke: () => seriesColor(index, palette), + stroke: () => seriesColor(index, slots), // The cursor readout is what decides how wide the legend gets, so // it is shortened here; a named unit is short enough to keep. Four // figures rather than three: this is the number someone is pointing diff --git a/frontend/src/components/Dashboard/palette.check.ts b/frontend/src/components/Dashboard/palette.check.ts index 0d54a77..1202f47 100644 --- a/frontend/src/components/Dashboard/palette.check.ts +++ b/frontend/src/components/Dashboard/palette.check.ts @@ -7,63 +7,76 @@ * * cd frontend && bun run src/components/Dashboard/palette.check.ts * - * The first block is the one that matters. A palette changes how *every* - * existing dashboard is drawn, and the whole safety argument is that a - * dashboard which names none is drawn exactly as it was before palettes - * existed — `--chart-${(index % 5) + 1}`, the literal expression `seriesColor` - * used. That equivalence is asserted here rather than trusted. + * Two properties, and it is worth being clear which is which. + * + * The one that must never move is **inertness**: a palette a dashboard wrote + * down is drawn exactly as written, so shipping this mechanism could not + * change the meaning of a single stored document. That is what made the + * default safe to move afterwards. + * + * The one that is free to move is the **default** itself — where a chart that + * named no palette lands. It started as the ramp in order, which gave three + * lines the three adjacent steps 1, 2 and 3; it is now spread. That is a + * design decision, pinned here so it changes on purpose rather than by + * accident, and a five-line chart is unchanged either way. */ import assert from "node:assert/strict" import { CHART_SLOTS, - DEFAULT_PALETTE, + NO_PALETTE, paletteOf, - slotFor, + slotsFor, } from "@/components/Common/UplotChart" -/** What a chart drew before a dashboard could name a palette. */ -const before = (index: number) => String((index % 5) + 1) +// --- the default: spread, not the first n steps --------------------------- -// A document with no palette, and every shape a broken one can arrive in: -// all of them fall back to the ramp, and the ramp is the old expression. -for (const stored of [undefined, null, "", [], ["9", "nonsense"], { 0: "1" }]) { - const palette = paletteOf(stored) +const SPREAD: Record = { + 1: ["1"], + 2: ["1", "5"], + 3: ["1", "3", "5"], + 4: ["1", "2", "4", "5"], + 5: ["1", "2", "3", "4", "5"], +} + +for (const [lines, want] of Object.entries(SPREAD)) { assert.deepEqual( - palette, - DEFAULT_PALETTE, - `${JSON.stringify(stored)} is no palette`, - ) - for (let index = 0; index < 23; index++) { - assert.equal( - slotFor(index, palette), - before(index), - `series ${index} of an unset palette draws as it always did`, - ) - } -} - -// The same, for a chart that was handed nothing at all — Health and Home draw -// through the very same component. -for (let index = 0; index < 23; index++) { - assert.equal( - slotFor(index), - before(index), - `series ${index} outside a dashboard`, + slotsFor(Number(lines)), + want, + `${lines} lines with no palette named`, ) } -// A distinct subset in draw order: kept as picked, and cycled through. +// Five lines take the whole ramp in order, which is what they always took. +assert.deepEqual( + slotsFor(CHART_SLOTS.length), + [...CHART_SLOTS], + "a five-line chart is drawn exactly as it was before palettes existed", +) + +// A chart is never left without a colour, whatever it is handed. +for (const lines of [0, -3, 1.5, 9, Number.NaN]) { + const slots = slotsFor(lines) + assert.ok(slots?.length > 0, `${lines} lines still gets a colour`) +} + +// --- inertness: a named palette is drawn as written ----------------------- + assert.deepEqual( paletteOf(["3", "1"]), ["3", "1"], "the order picked is the order kept", ) assert.deepEqual( - [0, 1, 2, 3, 4].map((index) => slotFor(index, ["3", "1"])), - ["3", "1", "3", "1", "3"], - "a chart with more lines than colours starts over", + slotsFor(2, ["3", "1"]), + ["3", "1"], + "a named palette is never re-spread", +) +assert.deepEqual( + slotsFor(5, ["3", "1"]), + ["3", "1"], + "and it is not widened to fit more lines either — the chart cycles", ) // Repeats are dropped rather than drawn: two lines on one slot could not be @@ -78,4 +91,21 @@ assert.deepEqual(paletteOf([...CHART_SLOTS].reverse()), [ "1", ]) +// Naming none, and every shape a broken one arrives in, all mean the same +// thing: no palette, so the chart spreads itself. +for (const stored of [undefined, null, "", [], ["9", "nonsense"], { 0: "1" }]) { + assert.equal( + paletteOf(stored), + NO_PALETTE, + `${JSON.stringify(stored)} names no palette`, + ) + for (let lines = 1; lines <= CHART_SLOTS.length; lines++) { + assert.deepEqual( + slotsFor(lines, paletteOf(stored)), + SPREAD[lines], + `${lines} lines still spread`, + ) + } +} + console.log("palette: ok") diff --git a/frontend/src/components/Dashboard/panels.tsx b/frontend/src/components/Dashboard/panels.tsx index 7e97279..0343182 100644 --- a/frontend/src/components/Dashboard/panels.tsx +++ b/frontend/src/components/Dashboard/panels.tsx @@ -1242,10 +1242,6 @@ export function DashboardPanel({ type="button" aria-pressed={picked} aria-label={`Colour ${slot}`} - // The last one cannot go. An empty palette falls back to - // the whole ramp, so the row would then contradict what - // the charts beside it are drawing. - disabled={picked && palette.length === 1} onClick={() => setSetting("palette", { ...paletteSetting, @@ -1273,9 +1269,10 @@ export function DashboardPanel({

The colours this dashboard's charts draw with, taken in the order you pick them — a chart with more lines than colours starts over - at the first. Each is offered once: two lines sharing a colour - could not be told apart, and neighbouring ones are close enough - already. + at the first. Pick none and each chart spreads itself across the + range by how many lines it has, which is usually what you want. + Each colour is offered once: two lines sharing one could not be + told apart, and neighbouring ones are close enough already.

diff --git a/frontend/src/components/Dashboard/settings.tsx b/frontend/src/components/Dashboard/settings.tsx index e13b12f..0500b1f 100644 --- a/frontend/src/components/Dashboard/settings.tsx +++ b/frontend/src/components/Dashboard/settings.tsx @@ -1,7 +1,7 @@ import { createContext, useContext } from "react" import type { DashboardDef_Output, SettingDef } from "@/client" -import { DEFAULT_PALETTE, paletteOf } from "@/components/Common/UplotChart" +import { NO_PALETTE, paletteOf } from "@/components/Common/UplotChart" import { useLiveValue } from "@/components/Flow/liveStore" /** @@ -100,14 +100,18 @@ export function useDashboardLocked( return useSetting(dashboard, "locked") === true } -const PaletteContext = createContext(DEFAULT_PALETTE) +const PaletteContext = createContext(NO_PALETTE) /** * The data colours everything drawn under it uses. * - * Mounted by the view *and* the editor: an editor showing the default ramp - * while the panel beside it showed the dashboard's own palette would be a - * preview that lies. + * Empty when the dashboard names none, which is not the same as "the ramp": + * a chart with no palette spreads itself across the ramp by how many lines it + * draws (`slotsFor`), and only a named palette overrides that. + * + * Mounted by the view *and* the editor: an editor drawing the automatic + * spread while the panel beside it showed the dashboard's own palette would + * be a preview that lies. * * Data colour only. A fault stays `--destructive` and a condition stays its * `ICON_COLORS` entry, because those name a state rather than tell one series @@ -130,7 +134,7 @@ export function PaletteProvider({ ) } -/** The ramp slots the dashboard around this widget draws its data with. */ +/** The ramp slots the dashboard around this widget named, or none. */ export const usePalette = () => useContext(PaletteContext) const LockedContext = createContext(false)