From ebac14bb803e539885d8dcca1a67cb044aca89e5 Mon Sep 17 00:00:00 2001 From: Bri Stanback <233396+Stanback@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:50:27 -0700 Subject: [PATCH] fix(web): keep close method-spectrum labels from colliding Each spectrum pin centred its name and time under the dot, so methods whose converted times sit close together printed over each other (women's 1650 SCY 17:36.20 -> 1500 SCM rendered "NCAAEquivalence" / "17:33.04 17:45.52"). Dots stay on a zero-width anchor at their true position. Labels are measured and laid out separately: centred on the pin when it fits, clamped inside the track, and moved to the first lower row where they touch no other label, with a leader tick back to the dot. The label is now the button, so its focus ring and hit area match what is drawn. The Playwright case reproduces the 1650 example at desktop and 390px in light and dark, asserting no label overlap, containment in the card and true dot positions. Co-Authored-By: Claude Opus 5.5 --- src/lib/MethodComparison.svelte | 127 ++++++++++++++++++++++++++++---- tests/app.spec.ts | 78 ++++++++++++++++++++ 2 files changed, 190 insertions(+), 15 deletions(-) diff --git a/src/lib/MethodComparison.svelte b/src/lib/MethodComparison.svelte index 65e6b66..af0feac 100644 --- a/src/lib/MethodComparison.svelte +++ b/src/lib/MethodComparison.svelte @@ -157,6 +157,62 @@ }; }); + // Pins sit at their true positions; labels are laid out separately so close + // methods never print over each other. Each label centres on its pin when it + // fits, stays inside the track, and drops to the first lower row (with a + // leader tick back to its pin) where it touches no other label. Among those + // rows it prefers one where no leader runs through a label. + let trackWidth = $state(0); + let labelWidths = $state([]); + let labelHeights = $state([]); + const rowPitch = $derived( + labelHeights.some(Boolean) ? Math.max(...labelHeights) + 4 : null, + ); + const labelPlaces = $derived.by(() => { + const nodes = spectrum?.nodes ?? []; + const places = nodes.map(() => ({ shift: null as number | null, row: 0 })); + if (!trackWidth || nodes.some((_, i) => !labelWidths[i])) return places; + const gap = 8; + type Box = { left: number; right: number; row: number; x: number }; + const placed: Box[] = []; + const overlaps = (a: Box, b: Box) => + a.row === b.row && a.left < b.right + gap && b.left < a.right + gap; + // A leader runs from the rail down through every row above its label. + const crosses = (leader: Box, label: Box) => + label.row < leader.row && + leader.x > label.left - gap && + leader.x < label.right + gap; + const order = nodes + .map((_, i) => i) + .sort((a, b) => nodes[a].pct - nodes[b].pct); + for (const i of order) { + const width = labelWidths[i]; + const x = (nodes[i].pct / 100) * trackWidth; + const left = Math.max(0, Math.min(x - width / 2, trackWidth - width)); + // Row placed.length is always empty, so a free row always exists. + const candidates = Array.from( + { length: placed.length + 1 }, + (_, row) => ({ + left, + right: left + width, + row, + x, + }), + ).filter((box) => !placed.some((other) => overlaps(box, other))); + const box = + candidates.find( + (box) => + !placed.some((other) => crosses(box, other) || crosses(other, box)), + ) ?? candidates[0]; + placed.push(box); + places[i] = { shift: box.left - x, row: box.row }; + } + return places; + }); + const labelRows = $derived( + Math.max(1, ...labelPlaces.map((place) => place.row + 1)), + ); + const paceExplorer = $derived.by(() => { if ( chosen?.result.status !== "available" || @@ -312,22 +368,36 @@ Method spread: {spectrum.spreadSeconds}s Slower ({spectrum.maxTime}) -
+
- {#each spectrum.nodes as node} - + {#if place.row > 0} + + {/if} + +
{/each}
@@ -624,8 +694,9 @@ font-weight: 600; } .spectrum-track { + --pin-row: 2.6rem; position: relative; - height: 46px; + height: calc(18px + var(--label-rows, 1) * var(--pin-row)); } .spectrum-rail { position: absolute; @@ -636,25 +707,51 @@ border-radius: 3px; background: var(--tm-color-border-subtle); } + /* Zero-width anchor at the method's true position. The dot centres on it; + the label is shifted by the measured layout (centred before measuring). */ .spectrum-pin { position: absolute; top: 0; - transform: translateX(-50%); + width: 0; + } + .pin-leader { + position: absolute; + top: 16px; + left: -0.5px; + width: 1px; + height: calc(2px + var(--label-row) * var(--pin-row)); + background: var(--tm-color-border-control); + } + .pin-label { + position: absolute; + top: calc(18px + var(--label-row) * var(--pin-row)); + left: 0; + z-index: 1; + transform: translateX(var(--label-shift, -50%)); display: flex; flex-direction: column; align-items: center; gap: 2px; padding: 0 4px; border: 0; - background: transparent; + border-radius: var(--tm-radius-control); + /* Opaque so a leader that must pass a label runs behind its text. */ + background: var(--tm-color-surface); color: var(--tm-color-text-muted); font: inherit; cursor: pointer; } - .spectrum-pin:disabled { + .pin-label:disabled { cursor: default; } + .pin-label:focus-visible { + outline: 2px solid var(--tm-color-accent); + outline-offset: 2px; + } .pin-dot { + position: absolute; + top: 3px; + left: -6px; width: 12px; height: 12px; border-radius: 50%; diff --git a/tests/app.spec.ts b/tests/app.spec.ts index 4b4ce07..857f1f4 100644 --- a/tests/app.spec.ts +++ b/tests/app.spec.ts @@ -1070,3 +1070,81 @@ test("benchmark swim selection leads the unified benchmark selector and marks on await expect(estimate).not.toHaveAttribute("aria-describedby"); await expect(estimate).toContainText("1:06.72"); }); + +test("method spectrum keeps close method labels apart and inside the card", async ({ + page, +}, testInfo) => { + if (testInfo.project.name === "mobile") + await page.setViewportSize({ width: 390, height: 844 }); + // NCAA and Equivalence land about 12s apart while Colorado is two minutes + // slower, so the two faster labels would collide at their pin positions. + await page.goto("./#t=17%3A36.20&s=FR&d=1500&c=SCY&g=women&h=0"); + await page.getByTestId("result-SCM").click(); + await page.locator(".result-method summary").click(); + const methods = page.getByRole("region", { name: "Compare methods in SCM" }); + const spectrum = methods.locator(".method-spectrum"); + await expect(spectrum).toBeVisible(); + const pins = spectrum.locator(".spectrum-pin"); + await expect(pins).toHaveCount(3); + for (const text of ["17:33.04", "17:45.52", "19:32.38"]) + await expect(spectrum).toContainText(text); + + for (const scheme of ["light", "dark"] as const) { + await page.emulateMedia({ colorScheme: scheme }); + const card = (await spectrum.boundingBox())!; + const labels = await spectrum + .locator(".pin-name, .pin-time") + .evaluateAll((nodes) => + nodes.map((node) => { + const r = node.getBoundingClientRect(); + return { + text: node.textContent ?? "", + left: r.left, + right: r.right, + top: r.top, + bottom: r.bottom, + }; + }), + ); + expect(labels).toHaveLength(6); + for (const label of labels) { + expect(label.left, label.text).toBeGreaterThanOrEqual(card.x); + expect(label.right, label.text).toBeLessThanOrEqual(card.x + card.width); + expect(label.top, label.text).toBeGreaterThanOrEqual(card.y); + expect(label.bottom, label.text).toBeLessThanOrEqual( + card.y + card.height, + ); + } + for (const [i, a] of labels.entries()) + for (const b of labels.slice(i + 1)) + expect( + a.right <= b.left || + b.right <= a.left || + a.bottom <= b.top || + b.bottom <= a.top, + `${a.text} overlaps ${b.text}`, + ).toBe(true); + + // Dots stay at their true relative positions on the time axis. + const dots = await pins.evaluateAll((nodes) => + nodes.map((node) => { + const r = node.querySelector(".pin-dot")!.getBoundingClientRect(); + return { + time: node.querySelector(".pin-time")!.textContent!, + x: r.left + r.width / 2, + }; + }), + ); + const at = (time: string) => dots.find((dot) => dot.time === time)!.x; + const ncaa = at("17:33.04"); + const equivalence = at("17:45.52"); + const colorado = at("19:32.38"); + expect(ncaa).toBeLessThan(equivalence); + expect(equivalence).toBeLessThan(colorado); + // 12.48s of a 119.34s spread: the gap is about a tenth of the span. + expect((equivalence - ncaa) / (colorado - ncaa)).toBeCloseTo( + 12.48 / 119.34, + 2, + ); + } +});