From aa8d3bb05dab0ecd133b62b52338ac1b0b098f8d Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 3 Aug 2026 23:30:17 +0200 Subject: [PATCH 1/3] fix(timeline): size pills and clips in px, not in fractions of the timeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported on a 30-minute recording: agent-placed pills render far wider than the effect they stand for, and the mismatch changes with the zoom. Three screen-space rules were written as a fraction of `total`, which is a DURATION in disguise — it scales with the recording, so what looked right on a one-minute clip was nonsense on a podcast: - `Math.max(1.5, pctOf(dur))` floored every pill at 1.5% of the timeline: 27 s on a 30-minute project, 58 s on the 65-minute one this was measured against, at every zoom level. Touching pills merged into one block and no pill could be read as a duration. - `total * 0.012` as the drag snap radius was a 21-second magnet, so an edge jumped to a clip boundary it was nowhere near — the more so the longer the recording, and the worse the further you zoomed in to place it precisely. - `.tlClips`'s flex `gap: 6px` was a fixed pixel amount inserted into a proportional layout: each junction pushed what followed 6px right while every clip shrank to pay for it, so a clip's left edge missed its own start time (+2px on clip 2, +6px on clip 3 of a three-clip timeline) while the pills and ruler above it sat at the true position. Constant in px means it was worth 5 s and 15 s of a 30-minute recording zoomed out, and a fraction of a second zoomed in — that changing ratio is what reads as "the pills move when I zoom". Everything on the canvas is now positioned by `pctOf` (clips included, now absolutely positioned instead of flex), and everything that must be a fixed SCREEN size goes through `pxPerSec`: `pillAffordance`, `PILL_SNAP_PX`, `CLIP_GUTTER_PX`. The only floor left on a width is 1px, in CSS. Handles follow from the width rather than fighting it. Above 18px (two 6px handles + a grabbable body) they sit inside the pill as before; below it they mount outside, with the gap on each side belonging to the pill's own hit strip — so a 1px pill still offers ~9px to move and 6px per side to resize, at every zoom. Nothing becomes unreachable at any size. The chrome re-flows mid-drag without disturbing the gesture: deltas come from the pointer and the listeners live on `window`. The flat 0.2 s minimum region is gone too (it refused the last fifth of a second however far you zoomed in); the floor is now the storage grid, 1 ms, since how SHORT a region may be is a data question and how PRECISELY you can aim at one is the zoom's business. Measured in the app on the reported project (65 min, 15 agent-placed trims): zoomed out each trim was drawn at 22.4px (58.5 s) and now sits at its true 0.45–6.94 s; zoomed 30x the old floor would have been 672px. Clip 2 now starts at exactly 50% of the canvas instead of +3px. Every test in V4Timeline.geometry.test.tsx was ablated — reverting each constant turns the matching one red. --- .../ai-edition/v4/EditorShellV4.module.css | 52 +++- .../v4/V4Timeline.geometry.test.tsx | 227 ++++++++++++++++++ src/components/ai-edition/v4/V4Timeline.tsx | 130 ++++++++-- .../architecture/editor-shell.md | 13 + 4 files changed, 395 insertions(+), 27 deletions(-) create mode 100644 src/components/ai-edition/v4/V4Timeline.geometry.test.tsx diff --git a/src/components/ai-edition/v4/EditorShellV4.module.css b/src/components/ai-edition/v4/EditorShellV4.module.css index 79c89f7e0..d1d5e93f6 100644 --- a/src/components/ai-edition/v4/EditorShellV4.module.css +++ b/src/components/ai-edition/v4/EditorShellV4.module.css @@ -1330,6 +1330,11 @@ position: absolute; top: 1px; height: 22px; + /* The ONLY floor on a pill's width, and it is in px so it is the same hairline + at every zoom: the box's width is the effect's duration, full stop. A `%` + minimum is a duration in disguise (the old 1.5% drew everything shorter than + 27 s as 27 s on a 30-minute timeline). */ + min-width: 1px; display: inline-flex; align-items: center; gap: 5px; @@ -1352,6 +1357,27 @@ .lanePillSel { box-shadow: 0 0 0 3px var(--accent-ring); } +/* Narrower than its own chrome (PILL_HANDLES_MIN_PX): the handles mount OUTSIDE + the box instead of inside it, so overflow must not clip them away — there is + no content to clip at this width anyway (see pillAffordance/roomForLabel). + ::after widens the move target to reach the handles (PILL_MOVE_GAP_PX on each + side), which is what keeps a 1px pill grabbable without inflating the bar the + user is reading a duration off. */ +.lanePillCompact { + overflow: visible; + padding: 0; +} +.lanePillCompact::after { + content: ""; + position: absolute; + inset: 0 -4px; +} +/* An outside handle sits on bare lane background, where a transparent grab strip + is undiscoverable — on hover it shows itself as a bar flanking the pill. */ +.lanePillCompact:hover .lanePillHandle { + background: color-mix(in oklch, currentColor 35%, transparent); + border-radius: 2px; +} .lanePillLabel { overflow: hidden; text-overflow: ellipsis; @@ -1378,8 +1404,14 @@ } .tlClips { position: relative; - display: flex; - gap: 6px; + /* NOT a flex row. Clips are absolutely positioned by percentage of the + timeline, like the pills, the ruler and the playhead above them. + A flex row's `gap` is a fixed pixel amount inserted into a proportional + layout: each junction pushed what followed 6px right while every clip + shrank proportionally to pay for the gaps, so a clip's left edge landed off + its true start time — measured at +2px and +6px for clips 2 and 3 of a + three-clip timeline, which is 5 s and 15 s of a 30-minute recording when + zoomed out, and a fraction of a second when zoomed in. See .tlClip. */ height: 66px; width: 100%; padding: 0; @@ -1398,9 +1430,19 @@ pointer-events: none; } .tlClip { - position: relative; - flex: 1 0 0; - min-width: 0; + /* left/width come from V4Timeline, in percent of the timeline. The 6px gutter + that used to be a flex `gap` is now taken off each clip's own width, so it + separates the cards without ever moving the next one: a clip's LEFT edge is + its true start time at every zoom and every clip count. + ponytail: the right edge therefore reads 6px short. Constant, non-cumulative + and below the width of the border it sits next to; draw the separator inside + the box (inset shadow, square-butted cards) if that ever needs to be exact. */ + position: absolute; + top: 0; + bottom: 0; + /* px, like .lanePill's: the gutter is subtracted from the clip's width, so a + clip narrower than 6px would otherwise compute to nothing. */ + min-width: 1px; border-radius: 11px; border: 1.5px solid var(--border); background: var(--surface-1); diff --git a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx new file mode 100644 index 000000000..fa475e3ee --- /dev/null +++ b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx @@ -0,0 +1,227 @@ +import "@testing-library/jest-dom"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { beforeAll, describe, expect, it, vi } from "vitest"; + +// The regression under test is geometric, so the environment has to have a size: +// jsdom reports 0 for every box, which would leave `pxPerSec` at 0 (the +// "unmeasured" case) and hide exactly the thing being checked. +const VIEWPORT_PX = 900; +const TOTAL_SEC = 1800; // a 30-minute recording, as in the report + +vi.mock("@/contexts/I18nContext", () => ({ + useScopedT: () => (key: string) => key, +})); +vi.mock("sonner", () => ({ toast: { error: vi.fn(), info: vi.fn(), success: vi.fn() } })); + +import { V4Timeline } from "./V4Timeline"; + +beforeAll(() => { + globalThis.ResizeObserver = class { + // jsdom has none, and the width it would report is stubbed below anyway. + observe() { + /* noop */ + } + unobserve() { + /* noop */ + } + disconnect() { + /* noop */ + } + } as unknown as typeof ResizeObserver; + Object.defineProperty(HTMLElement.prototype, "clientWidth", { + configurable: true, + get: () => VIEWPORT_PX, + }); + Object.defineProperty(HTMLElement.prototype, "getBoundingClientRect", { + configurable: true, + value: () => ({ + x: 0, + y: 0, + left: 0, + top: 0, + right: VIEWPORT_PX, + bottom: 100, + width: VIEWPORT_PX, + height: 100, + toJSON() { + /* unused by the component */ + }, + }), + }); +}); + +function clip(startSec: number, endSec: number) { + return { + id: `c@${startSec}`, + assetId: "a1", + timelineStartSec: startSec, + timelineEndSec: endSec, + sourceStartSec: 0, + sourceEndSec: endSec - startSec, + }; +} + +/** By default one 30-minute clip carrying a single one-second annotation. */ +function renderTimeline( + clips = [clip(0, TOTAL_SEC)], + annotation = { id: "ann1", startMs: 10_000, endMs: 11_000 }, +) { + const tl = { + clips, + assets: [{ id: "a1", label: "rec", durationSec: TOTAL_SEC }], + annotationRegions: [annotation], + speedRegions: [], + cameraFullscreenRegions: [], + zoomRegions: [], + trimRanges: [], + selection: null, + multiSelection: [], + clipSelection: null, + clearSelection: vi.fn(), + selectRegion: vi.fn(), + selectClip: vi.fn(), + updateAnnotationSpan: vi.fn(async () => { + /* the drag only awaits it */ + }), + }; + render( + , + ); + return { + pill: screen.getByTitle("toolbar.newAnnotation"), + clipEls: Array.from(document.querySelectorAll("[data-clip-id]")), + tl, + }; +} + +/** Drag a handle by `dxPx`. The move/up listeners live on `window`, so the drag + * is driven by pointer deltas alone — the handle may re-mount under it. */ +function dragHandle(handle: Element, dxPx: number) { + fireEvent.pointerDown(handle, { clientX: 0 }); + window.dispatchEvent(new MouseEvent("pointermove", { clientX: dxPx })); + window.dispatchEvent(new MouseEvent("pointerup", { clientX: dxPx })); +} + +/** Ctrl+wheel up = zoom in; the handler is a native listener, so dispatch real events. */ +function zoomIn(notches: number) { + const canvas = document.querySelector("[class*=tlTracks]") as HTMLElement; + for (let i = 0; i < notches; i++) { + fireEvent.wheel(canvas, { ctrlKey: true, deltaY: -100, clientX: 0 }); + } +} + +describe("V4Timeline lane pills", () => { + it("draws a pill exactly as wide as its region, at any zoom", () => { + // 1 s of 1800 s. The old `Math.max(1.5, …)` floor drew this as 1.5% — 27 + // seconds of ruler for a one-second annotation — and did it at every zoom, + // since the floor was a percentage of the timeline rather than of the screen. + const { pill } = renderTimeline(); + const expected = (1 / TOTAL_SEC) * 100; + expect(Number.parseFloat(pill.style.width)).toBeCloseTo(expected, 6); + + // The canvas is what scales with zoom, so the pill's share of it must not + // move at all — only the chrome inside it may react (below). + zoomIn(40); + expect(Number.parseFloat(pill.style.width)).toBeCloseTo(expected, 6); + }); + + it("keeps both resize handles reachable when the pill is thinner than they are", () => { + // 0.5 px wide at this zoom: the handles cannot sit inside the box without + // swallowing it whole, so they mount outside it and the body stays a move + // target. Resizing a hairline stays possible — it is the pointer precision + // that is coarse there, not the affordance that is missing. + const { pill } = renderTimeline(); + const [left, right] = Array.from(pill.querySelectorAll("span")); + expect(left.style.left).toBe("-10px"); + expect(right.style.right).toBe("-10px"); + // Nothing legible fits, so no icon/label is rendered (the title attribute + // still carries the value on hover). + expect(pill.textContent).toBe(""); + + // Zoomed to the 50× ceiling the same second is 25 px wide and hosts its own + // chrome again. + zoomIn(40); + expect(left.style.left).toBe("0px"); + expect(right.style.right).toBe("0px"); + }); + + it("grows and shrinks a hairline pill from its outside handles", () => { + // Growing is unbounded by the pill's own size: 90 px right of a 900 px canvas + // is a tenth of the 1800 s timeline, so the 10–11 s annotation ends at 191 s. + // The chrome re-flows inside the box as it crosses PILL_HANDLES_MIN_PX + // mid-drag, which the gesture never notices — the deltas come from the + // pointer and the listeners live on `window`, not on the handle. + const { pill, tl } = renderTimeline(); + const [left, right] = Array.from(pill.querySelectorAll("span")); + dragHandle(right, 90); + expect(tl.updateAnnotationSpan).toHaveBeenCalledWith("ann1", 10_000, 191_000); + + // Shrinking stops at the storage grid (1 ms), not at the old flat 200 ms + // floor that refused the last fifth of a second however far you zoomed in. + dragHandle(left, 90_000); + expect(tl.updateAnnotationSpan).toHaveBeenLastCalledWith("ann1", 10_999, 11_000); + + // 18 s short of the timeline end: 9 px away on screen, so it stays where it + // was dropped. The snap radius used to be 1.2% of the timeline — a 21-second + // magnet here — which is what made a grown edge jump to a clip boundary it + // was nowhere near, the more so the longer the recording. + dragHandle(right, 885.5); + expect(tl.updateAnnotationSpan).toHaveBeenLastCalledWith("ann1", 10_000, 1_782_000); + }); +}); + +describe("V4Timeline clip row", () => { + // Three clips = two junctions. As a flex row with `gap: 6px`, each junction + // added 6px while every clip shrank proportionally to pay for it, so a clip's + // left edge missed its true start: measured in a browser on this very fixture, + // clip 2 by +2px and clip 3 by +6px, while the pills and ruler above them sat + // at the true position. Being a fixed px error in a proportional layout, it was + // worth 5 s and 15 s of timeline zoomed out but a fraction of a second zoomed + // in — which is what reads as "the pills move when I zoom". + const CLIPS = [clip(0, 600), clip(600, 900), clip(900, TOTAL_SEC)]; + const startsAt = (sec: number) => `${(sec / TOTAL_SEC) * 100}%`; + + it("anchors every clip to its own start time, and keeps it there under zoom", () => { + // The annotation starts exactly where the second clip does, so the pill and + // the clip edge under it must resolve to the very same coordinate. + const { clipEls, pill } = renderTimeline(CLIPS, { + id: "ann1", + startMs: 600_000, + endMs: 601_000, + }); + expect(clipEls.map((el) => el.style.left)).toEqual([startsAt(0), startsAt(600), startsAt(900)]); + expect(pill.style.left).toBe(clipEls[1].style.left); + + // Zoom scales the canvas these coordinates live in, so the coordinates + // themselves must not move: same values, same agreement with the pill. + zoomIn(40); + expect(clipEls.map((el) => el.style.left)).toEqual([startsAt(0), startsAt(600), startsAt(900)]); + expect(pill.style.left).toBe(clipEls[1].style.left); + }); + + it("takes the card gutter out of each clip's own width", () => { + // The 6px is what separates two cards. Taken off the clip's width it stays + // local to that clip; inserted between them (a flex gap) it displaced every + // clip that followed. The 1px floor keeps a clip shorter than the gutter + // from collapsing to nothing on a long timeline. + const { clipEls } = renderTimeline(CLIPS); + const widths = clipEls.map((el) => el.style.width); + // (jsdom re-serialises the percentage to 4 decimals, hence the numeric read) + expect(widths.map((w) => w.endsWith("- 6px)"))).toEqual([true, true, true]); + for (const [i, durSec] of [600, 300, 900].entries()) { + expect(Number.parseFloat(widths[i].slice("calc(".length))).toBeCloseTo( + (durSec / TOTAL_SEC) * 100, + 3, + ); + } + }); +}); diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index ca173b155..f48b7286b 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -90,6 +90,74 @@ const MIN_LABEL_GAP_PX = 76; /** Unlabelled ticks drawn between two labelled ones. */ const MINOR_PER_MAJOR = 5; +// ── lane-pill screen geometry ─────────────────────────────────────── +// A pill's width IS its duration — there is no minimum beyond the 1px the CSS +// keeps so a very short region doesn't vanish entirely. The floor used to be +// `max(1.5%, …)` of the whole timeline, a percentage and therefore a DURATION: +// on a 30-minute recording every region shorter than 27 s was drawn as if it +// lasted 27 s, at every zoom level, so agent-placed zooms and trims lied about +// what they covered and touching ones merged into one visual block. +// +// What a pill needs room FOR (two resize handles, a label) is a question about +// its width in PIXELS at the current zoom, which is what pillAffordance answers. +/** Grab-strip width of one resize handle — mirrors .lanePillHandle in the CSS. */ +const PILL_HANDLE_PX = 6; +/** Clear body left between two inside-mounted handles. Under this the handles + * would meet (or overlap) and a "move" drag would silently become a resize — + * the point at which the pill flips to the compact geometry below. */ +const PILL_MOVE_PX = 6; +/** Two handles + a grabbable body: the narrowest pill that can host its own + * chrome inside its box. */ +const PILL_HANDLES_MIN_PX = PILL_HANDLE_PX * 2 + PILL_MOVE_PX; +/** + * Compact pills keep BOTH affordances by moving the chrome outside the box: + * handle | gap | «the pill» | gap | handle. The gaps belong to the move target + * (the pill's ::after strip widens by exactly this much), so even a 1px pill + * offers ~8px to grab for a move and 6px on each side to resize — at every zoom, + * at every duration. Mirrors .lanePillCompact in the CSS. + */ +const PILL_MOVE_GAP_PX = 4; +/** Offset of an outside-mounted handle from the pill's edge. */ +const PILL_HANDLE_OUT_PX = PILL_HANDLE_PX + PILL_MOVE_GAP_PX; +/** Icon (11px) + the pill's own padding (15px): below this, content is pure + * overflow — the lane's colour already says which kind it is, and the title + * attribute still gives the value on hover. */ +const PILL_CONTENT_MIN_PX = 34; +/** Edge-snap radius while dragging a pill, in screen px. */ +const PILL_SNAP_PX = 8; +/** Visual separation between two clip cards. Taken off each clip's own width + * (see .tlClip) rather than inserted between them, so it cannot displace the + * clips that follow — which is what a flex `gap` did, once per junction. */ +const CLIP_GUTTER_PX = 6; +/** + * Shortest region a resize may leave behind — the storage grid itself (regions + * are `Math.round`ed to whole ms, and coalesceRegionsForRuler's epsilon is 1 ms), + * so nothing rounds away to a zero-length row. It replaced a flat 0.2 s floor, + * which quietly refused the last 200 ms of every trim however far you zoomed in. + * How SHORT a region can be is a data question; how PRECISELY you can aim at one + * is the zoom's business, and the two were conflated. + */ +const MIN_REGION_SEC = 0.001; + +/** + * How a pill's chrome is laid out at its current on-screen size. + * + * `compact` — the box is too narrow to hold handles AND a draggable body, so the + * handles mount outside it (see PILL_MOVE_GAP_PX). Nothing is lost: move and + * resize both stay reachable at any width and any zoom, the pill just stops + * containing its own controls. + * + * `pxPerSec <= 0` means the panel hasn't been measured yet (first paint, jsdom). + * Assume roomy rather than reflowing every pill's chrome for one frame. + */ +export function pillAffordance( + durSec: number, + pxPerSec: number, +): { compact: boolean; roomForLabel: boolean } { + const widthPx = pxPerSec > 0 ? durSec * pxPerSec : Number.POSITIVE_INFINITY; + return { compact: widthPx < PILL_HANDLES_MIN_PX, roomForLabel: widthPx >= PILL_CONTENT_MIN_PX }; +} + // Ruler tick label. Precision follows the step: whole seconds read as a clean // M:SS, but once the ruler is zoomed past one tick per second the fraction is // the only thing telling two labels apart. @@ -369,6 +437,13 @@ export function V4Timeline({ const pctOf = useCallback((sec: number) => (sec / total) * 100, [total]); const showLanes = variant === "edit"; + // The visible fraction of the timeline, and what one second is worth on screen + // at that zoom. Every screen-space rule below — ruler step, pill affordances, + // snap radius — goes through this instead of being written as a fraction of + // `total`, which is a duration in disguise and so scales with the recording. + const navSpan = Math.max(0.02, nav.end - nav.start); + const pxPerSec = viewportWidthPx / navSpan / total; + // ── region lanes ──────────────────────────────────────────────── // zoom/speed/annotation: one pill per row, never coalesced — each carries // distinct per-instance content (depth/focus, speed value, text) that two @@ -434,8 +509,6 @@ export function V4Timeline({ // is the first "nice" one whose on-screen gap clears MIN_LABEL_GAP_PX, which // is why the labels never collide however narrow the panel gets. const rulerTicks = useMemo((): { step: number; ticks: RulerTick[] } => { - const span = Math.max(0.02, nav.end - nav.start); - const pxPerSec = viewportWidthPx / span / total; if (!Number.isFinite(pxPerSec) || pxPerSec <= 0) return { step: 1, ticks: [] }; const step = TICK_STEPS_SEC.find((s) => s * pxPerSec >= MIN_LABEL_GAP_PX) ?? @@ -451,7 +524,7 @@ export function V4Timeline({ ticks.push({ sec: i * minor, major: i % MINOR_PER_MAJOR === 0 }); } return { step, ticks }; - }, [total, nav.start, nav.end, viewportWidthPx]); + }, [total, nav.start, nav.end, pxPerSec]); // Live scrub position. The store write behind it is rAF-throttled (see // seekToClientX), so this keeps the playhead and the timecode pinned to the @@ -582,15 +655,18 @@ export function V4Timeline({ // to `setTrimEntries` as `dropIds` so a shrinking span deletes the entries // it no longer needs. const trimOwned: string[] = [...pill.sourceIds]; - // Snap targets: clip boundaries + timeline ends. Within ~1% of total, - // an edge snaps and a vertical guide is shown (Bottombar parity). + // Snap targets: clip boundaries + timeline ends. Within PILL_SNAP_PX of + // one on screen, an edge snaps and a vertical guide is shown. + // The radius is in PIXELS: as a fraction of total (it was 1.2%) it was a + // 21-second magnet on a 30-minute project, so a pill dragged anywhere near + // a junction jumped to it however far you zoomed in to place it precisely. const snapTargets = [ 0, total, ...clips.map((c) => c.timelineStartSec), ...clips.map((c) => c.timelineEndSec), ]; - const snapThresh = total * 0.012; + const snapThresh = pxPerSec > 0 ? PILL_SNAP_PX / pxPerSec : 0; const snap = (v: number): number => { let best = v; let bestD = snapThresh; @@ -605,8 +681,8 @@ export function V4Timeline({ return best; }; const apply = async (start: number, end: number): Promise => { - const s = Math.max(0, Math.min(end - 0.2, start)); - const en = Math.min(total, Math.max(s + 0.2, end)); + const s = Math.max(0, Math.min(end - MIN_REGION_SEC, start)); + const en = Math.min(total, Math.max(s + MIN_REGION_SEC, end)); if (pill.kind === "zoom") await tl.updateZoomSpan(pill.id, s * 1000, en * 1000); else if (pill.kind === "speed") await tl.updateSpeedSpan(pill.id, s * 1000, en * 1000); else if (pill.kind === "annotation") @@ -642,11 +718,11 @@ export function V4Timeline({ ns = Math.max(0, Math.min(total - dur, snap(pill.start + dxSec))); ne = ns + dur; } else if (dragMode === "l") { - ns = Math.max(0, Math.min(pill.end - 0.2, snap(pill.start + dxSec))); + ns = Math.max(0, Math.min(pill.end - MIN_REGION_SEC, snap(pill.start + dxSec))); ne = pill.end; } else { ns = pill.start; - ne = Math.min(total, Math.max(pill.start + 0.2, snap(pill.end + dxSec))); + ne = Math.min(total, Math.max(pill.start + MIN_REGION_SEC, snap(pill.end + dxSec))); } const nextState = { id: pill.id, kind: pill.kind, start: ns, end: ne }; activePillDragRef.current = nextState; @@ -672,7 +748,7 @@ export function V4Timeline({ window.addEventListener("pointermove", move); window.addEventListener("pointerup", up); }, - [tl, total, clips], + [tl, total, clips, pxPerSec], ); const startNavDrag = useCallback( @@ -772,7 +848,6 @@ export function V4Timeline({ // canvas's own (already widened) box — so scrolling to nav.start is a flat // -nav.start of the canvas. Scaling it by 1/navSpan as well double-counted // the zoom and threw the whole timeline off-screen at any nav.start > 0. - const navSpan = Math.max(0.02, nav.end - nav.start); const canvasStyle = { width: `${(100 / navSpan).toFixed(3)}%`, transform: `translateX(${(-nav.start * 100).toFixed(3)}%)`, @@ -827,13 +902,12 @@ export function V4Timeline({ // Width + gap the dragged clip displaces its neighbours by — measured // once at drag start (only its position changes during the drag, not // its size). - const gapPx = 6; - const shiftAmount = clipEl.getBoundingClientRect().width + gapPx; + const shiftAmount = clipEl.getBoundingClientRect().width + CLIP_GUTTER_PX; // Boundaries are captured once, before any transform is applied — // re-querying live rects mid-drag would pick up the dragged clip's own // translated (pointer-following) position and corrupt the math, since - // its rect no longer reflects its untouched flex slot. + // its rect no longer reflects its untouched slot. const originalRects = Array.from( container.querySelectorAll("[data-clip-id]"), ).map((el) => el.getBoundingClientRect()); @@ -1012,17 +1086,21 @@ export function V4Timeline({ suppressRightSeam: boolean; }) => { const { pill: p } = seg; + const durSec = seg.segEnd - seg.segStart; + // The box is exactly as long as the effect is; only what fits INSIDE it + // varies with the zoom. + const { compact, roomForLabel } = pillAffordance(durSec, pxPerSec); return (
startPillDrag(e, p, "l")} /> ) : null} - {seg.showContent ? ( + {seg.showContent && roomForLabel ? ( <> {pillIcon(p.kind)} {p.label} @@ -1055,7 +1133,7 @@ export function V4Timeline({ {seg.interactive ? ( startPillDrag(e, p, "r")} /> ) : null} @@ -1446,7 +1524,15 @@ export function V4Timeline({ className={`${styles.tlClip}${selected ? ` ${styles.tlClipSel}` : ""}${ dragging ? ` ${styles.tlClipDragging}` : "" }`} - style={{ flex: `${dur} 0 0`, transform: clipTransform }} + style={{ + left: `${pctOf(c.timelineStartSec)}%`, + // Minus the gutter that separates two cards (it used to be the + // flex row's `gap`). A clip shorter than the gutter lands on + // .tlClip's 1px min-width instead of collapsing — same rule as + // the lane pills above. + width: `calc(${pctOf(dur)}% - ${CLIP_GUTTER_PX}px)`, + transform: clipTransform, + }} onPointerDown={(e) => startClipDrag(e, c)} onClick={(e) => { e.stopPropagation(); diff --git a/technical-documentation/architecture/editor-shell.md b/technical-documentation/architecture/editor-shell.md index 947c3db51..cfcd6005e 100644 --- a/technical-documentation/architecture/editor-shell.md +++ b/technical-documentation/architecture/editor-shell.md @@ -133,6 +133,19 @@ switches on it is checked below. Each path has been verified on this branch. them through `renderPills` inside a `
` block (`:1244-1252`), and extend the `kind` union at `:203` so drag, resize, and delete handler switches route correctly. + + **Coordinates on the timeline canvas obey one rule**: position and size are + `pctOf(sec)` percentages of the whole timeline — pills, ruler ticks, playhead, + snap guide and clips all use it, which is what keeps them aligned when the + canvas is scaled by `1/navSpan` for zoom. Anything that must be a fixed + *screen* size (a minimum width, a grab handle, a snap radius, the gutter + between two clip cards) is expressed in **px**, converted through `pxPerSec` + where it needs to reach time. A percentage used as a screen constant is a + duration in disguise: it scales with the recording, so a `1.5%` minimum pill + width was a 27-second pill on a 30-minute project, and a flex `gap` used as a + clip separator displaced every clip after it. See `pillAffordance`, + `PILL_SNAP_PX` and `CLIP_GUTTER_PX`, and the invariants in + `V4Timeline.geometry.test.tsx`. 5. **Inspector selection pane** — `src/components/ai-edition/v4/FloatingInspector.tsx`. The `SelectionPane` (`:444`) is the kind-switch site, not the facets; add an `if (selection.kind === "xxx") { ... }` branch alongside `:513, :612, From f4d8c015c38501b4f2bb5b58c6fec54e27edf36f Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 3 Aug 2026 23:37:53 +0200 Subject: [PATCH 2/3] refactor(timeline): derive roomForLabel from compact, not from a constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on the commit before it. `.lanePillCompact` turns overflow visible — it has to, its handles hang outside the box — so a compact pill that rendered a label would spill it across the lane with nothing to clip it. That can only happen if PILL_CONTENT_MIN_PX drops below PILL_HANDLES_MIN_PX, which nothing enforced: two independent numbers with an invisible dependency between them. `roomForLabel` now derives from `compact` itself, so the guarantee holds whatever those numbers become. Also spells out why the snap radius is 0 while the panel is still unmeasured. --- src/components/ai-edition/v4/V4Timeline.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index f48b7286b..7cee23786 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -155,7 +155,13 @@ export function pillAffordance( pxPerSec: number, ): { compact: boolean; roomForLabel: boolean } { const widthPx = pxPerSec > 0 ? durSec * pxPerSec : Number.POSITIVE_INFINITY; - return { compact: widthPx < PILL_HANDLES_MIN_PX, roomForLabel: widthPx >= PILL_CONTENT_MIN_PX }; + const compact = widthPx < PILL_HANDLES_MIN_PX; + // `!compact &&` is load-bearing, not belt-and-braces: .lanePillCompact turns + // overflow visible (it has to, its handles hang outside the box), so a compact + // pill that rendered a label would spill it across the lane with nothing to + // clip it. Today PILL_CONTENT_MIN_PX > PILL_HANDLES_MIN_PX makes that + // impossible; this makes it impossible whatever those two numbers become. + return { compact, roomForLabel: !compact && widthPx >= PILL_CONTENT_MIN_PX }; } // Ruler tick label. Precision follows the step: whole seconds read as a clean @@ -666,6 +672,9 @@ export function V4Timeline({ ...clips.map((c) => c.timelineStartSec), ...clips.map((c) => c.timelineEndSec), ]; + // 0 = no snapping at all while the panel is unmeasured (first paint): + // better to drop the edge exactly where it was released than to move it + // by a radius computed from a width we do not have. const snapThresh = pxPerSec > 0 ? PILL_SNAP_PX / pxPerSec : 0; const snap = (v: number): number => { let best = v; From 327e06f1065a326ff24ed40acf9ab2db061157cc Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 3 Aug 2026 23:52:58 +0200 Subject: [PATCH 3/3] fix(timeline): drop the test's `any` and refresh the checklist's line refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the automated review on aa8d3bb0: - the mocked `tl` was cast to `any` behind a biome-ignore, which AGENTS.md rules out ("don't add new `any`"). Cast through `unknown` to the real `ReturnType` instead: the prop keeps its type and the suppression goes away. - the "add a region kind" checklist pointed at pre-PR line numbers in V4Timeline.tsx, and this branch moved them by ~130 lines. Recomputed against the current file: :461-509 for the pill call site, :1478-1488 for the lane render block, :332 for the `kind` union. (They were already drifting before this branch — check-docs does not verify line numbers.) The third finding, `currentColor` → `currentcolor` for stylelint's value-keyword-case, does not apply here: the repo has no stylelint at all (CI's Lint job is `biome check`, green on both spellings), and the convention in website/src/css/custom.css is `currentColor`. Changing it would leave the only lowercase spelling in the codebase. --- src/components/ai-edition/v4/V4Timeline.geometry.test.tsx | 6 ++++-- technical-documentation/architecture/editor-shell.md | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx index fa475e3ee..5497244bc 100644 --- a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx +++ b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx @@ -13,6 +13,7 @@ vi.mock("@/contexts/I18nContext", () => ({ })); vi.mock("sonner", () => ({ toast: { error: vi.fn(), info: vi.fn(), success: vi.fn() } })); +import type { useTimeline } from "@/lib/ai-edition/store/useTimeline"; import { V4Timeline } from "./V4Timeline"; beforeAll(() => { @@ -86,8 +87,9 @@ function renderTimeline( }; render( } setCurrentTime={vi.fn()} playing={false} onTogglePlay={vi.fn()} diff --git a/technical-documentation/architecture/editor-shell.md b/technical-documentation/architecture/editor-shell.md index cfcd6005e..b6c79db9f 100644 --- a/technical-documentation/architecture/editor-shell.md +++ b/technical-documentation/architecture/editor-shell.md @@ -129,9 +129,9 @@ switches on it is checked below. Each path has been verified on this branch. (`document/timeline.ts:591`) for batch / single deletes. 4. **Lane in `V4Timeline`** — `src/components/ai-edition/v4/V4Timeline.tsx`. Compute the pills at the same call site as the four existing lanes - (`coalesceRegionsForRuler(tl.xxxRegions).map(...)` near `:321-347`), render + (`coalesceRegionsForRuler(tl.xxxRegions).map(...)` near `:461-509`), render them through `renderPills` inside a `
` - block (`:1244-1252`), and extend the `kind` union at `:203` so drag, + block (`:1478-1488`), and extend the `kind` union at `:332` so drag, resize, and delete handler switches route correctly. **Coordinates on the timeline canvas obey one rule**: position and size are