From 1faa0cbdadc71abdea220d8705682c0a18e499bb Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 15:42:07 +0200 Subject: [PATCH] fix(studio): announce real track numbers and point aria-controls at the lanes The timeline's track key is a fractional z-order sort value, and the header built its visibility label straight from it, so screen readers announced "Hide track 0.16666666666666666". A track's 1-based display row is now passed alongside the key: the row number goes in every label, the key keeps routing every callback (visibility toggle, lane context menu). The same fix covers the `Track N` fallback used when a track holds no labelled element. The layer disclosure caret's aria-controls named a div in the sticky label column. That subtree is not empty, it holds the per-lane keyframe controls, but its children are all absolutely positioned so the div computes to 0x0, and the diamonds the caret visibly reveals live on the canvas instead. The caret expands two disjoint subtrees and was naming the less useful one. TimelinePropertyLanes now renders one static wrapper (static, not relative, so it establishes no containing block and the absolutely-positioned lanes keep resolving against the track-content div with identical geometry) and takes the id. TimelineLanes mints that id, since it is the only place that sees both ends of the disclosure, and mounts the wrapper for the track's keyframe clip in both disclosure states so the reference still resolves while collapsed. TimelineLaneBaseProps moves to its own module: it is the contract shared by TimelineCanvas and TimelineLanes, and lifting it out keeps TimelineLanes.tsx well under the 600-line cap instead of pushing past it. --- .../player/components/LayerDisclosureRow.tsx | 5 +- .../src/player/components/Timeline.test.ts | 4 +- .../src/player/components/TimelineCanvas.tsx | 3 +- .../player/components/TimelineLanes.test.tsx | 257 ++++++++++++++++++ .../src/player/components/TimelineLanes.tsx | 120 ++------ .../components/TimelinePropertyLanes.test.tsx | 63 +++++ .../components/TimelinePropertyLanes.tsx | 17 +- .../components/TimelineTrackHeader.test.tsx | 30 +- .../player/components/TimelineTrackHeader.tsx | 66 +++-- .../player/components/timelineLaneProps.ts | 86 ++++++ 10 files changed, 524 insertions(+), 127 deletions(-) create mode 100644 packages/studio/src/player/components/TimelineLanes.test.tsx create mode 100644 packages/studio/src/player/components/timelineLaneProps.ts diff --git a/packages/studio/src/player/components/LayerDisclosureRow.tsx b/packages/studio/src/player/components/LayerDisclosureRow.tsx index 2f56f0aa8..415b72c09 100644 --- a/packages/studio/src/player/components/LayerDisclosureRow.tsx +++ b/packages/studio/src/player/components/LayerDisclosureRow.tsx @@ -22,7 +22,10 @@ export function LayerDisclosureRow({ /** Same adaptive width the lane rows use: a narrowed header column must not * leave this row hanging over the clips it labels. */ columnWidth: number; - /** Id of the element holding the lanes this row's caret expands. */ + /** Id of the CANVAS-side element holding the diamond lanes this row's caret + * expands (see TimelinePropertyLanes). The caret also reveals the per-lane + * control rows in this column, but the diamonds are what following the + * reference should land on. */ lanesId: string; onToggleClipExpanded: () => void; /** Trailing controls that act on the LAYER (the visibility eye), not on a lane. */ diff --git a/packages/studio/src/player/components/Timeline.test.ts b/packages/studio/src/player/components/Timeline.test.ts index 81b095961..7335a0fad 100644 --- a/packages/studio/src/player/components/Timeline.test.ts +++ b/packages/studio/src/player/components/Timeline.test.ts @@ -293,7 +293,9 @@ describe("Timeline provider boundary", () => { // mounted before we query it. act(() => {}); - const button = host.querySelector('button[aria-label="Show track 0"]'); + // "1", not "0": the label carries the 1-based display row, while the + // callback below still routes by the track's own key. + const button = host.querySelector('button[aria-label="Show track 1"]'); expect(button).not.toBeNull(); if (!button) throw new Error("Expected a track visibility toggle"); diff --git a/packages/studio/src/player/components/TimelineCanvas.tsx b/packages/studio/src/player/components/TimelineCanvas.tsx index bdf63bbc5..3cb0d52e6 100644 --- a/packages/studio/src/player/components/TimelineCanvas.tsx +++ b/packages/studio/src/player/components/TimelineCanvas.tsx @@ -20,7 +20,8 @@ import { type MultiDragPreviewInput } from "./timelineMultiDragPreview"; import { useTimelineEditContextOptional } from "../../contexts/TimelineEditContext"; import type { Rect } from "../../utils/marqueeGeometry"; import { TimelineClip } from "./TimelineClip"; -import { TimelineLanes, type TimelineLaneBaseProps } from "./TimelineLanes"; +import { TimelineLanes } from "./TimelineLanes"; +import type { TimelineLaneBaseProps } from "./timelineLaneProps"; import { renderClipChildren } from "./timelineClipChildren"; import { useTimelineRevealClip } from "./useTimelineRevealClip"; import type { TimelineLaneGapStrips } from "./useTimelineGapHighlights"; diff --git a/packages/studio/src/player/components/TimelineLanes.test.tsx b/packages/studio/src/player/components/TimelineLanes.test.tsx new file mode 100644 index 000000000..eecc98f89 --- /dev/null +++ b/packages/studio/src/player/components/TimelineLanes.test.tsx @@ -0,0 +1,257 @@ +// @vitest-environment happy-dom + +import React, { act, createRef } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TimelineLanes } from "./TimelineLanes"; +import { getTrackStyle } from "./timelineIcons"; +import { defaultTimelineTheme } from "./timelineTheme"; +import { TRACK_H } from "./timelineLayout"; +import { usePlayerStore, type TimelineElement } from "../store/playerStore"; +import type { MultiDragPreviewInput } from "./timelineMultiDragPreview"; +import type { TimelineEditCallbacks } from "./timelineCallbacks"; +import type { DraggedClipState, BlockedClipState } from "./useTimelineClipDrag"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; + usePlayerStore.getState().reset(); +}); + +/** The z-order sort keys really are fractional: a clip nudged between two lanes + * lands on the midpoint. These are the values that used to reach aria-label. */ +const TRACK_A = 1 / 6; +const TRACK_B = 0.5; + +function element(id: string, track: number): TimelineElement { + return { id, label: id, tag: "div", start: 0, duration: 2, track }; +} + +function positionTween(id: string): GsapAnimation { + return { + id: `${id}-tween`, + targetSelector: `#${id}`, + method: "to", + position: 0, + duration: 2, + properties: {}, + propertyGroup: "position", + keyframes: { + format: "percentage", + keyframes: [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 100, properties: { x: 100 } }, + ], + }, + }; +} + +interface RenderLanesOptions { + elements?: TimelineElement[]; + animations?: Map; + expandedClipIds?: string[]; + selectedElementIds?: Set; + multiDragPreview?: MultiDragPreviewInput | null; + draggedClip?: DraggedClipState | null; + onToggleTrackHidden?: TimelineEditCallbacks["onToggleTrackHidden"]; + onContextMenuLane?: (e: React.MouseEvent, track: number, time: number) => void; +} + +function renderLanes(options: RenderLanesOptions = {}): { + host: HTMLDivElement; + root: Root; + rerender: (next: RenderLanesOptions) => void; +} { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const render = (next: RenderLanesOptions) => { + const elements = next.elements ?? [element("clip-a", TRACK_A)]; + const gsapAnimations = next.animations ?? new Map(); + const displayTrackOrder = [...new Set(elements.map((el) => el.track))].sort((a, b) => a - b); + const tracks: [number, TimelineElement[]][] = displayTrackOrder.map((track) => [ + track, + elements.filter((el) => el.track === track), + ]); + const laneCounts = new Map( + elements.map((el) => [el.id, (gsapAnimations.get(el.id) ?? []).length]), + ); + act(() => { + usePlayerStore.setState({ expandedClipIds: new Set(next.expandedClipIds ?? []) }); + root.render( + TRACK_H)} + trackOrder={displayTrackOrder} + tracks={tracks} + trackStyles={new Map()} + laneCounts={laneCounts} + selectedElementId={null} + selectedElementIds={next.selectedElementIds ?? new Set()} + hoveredClip={null} + draggedClip={next.draggedClip ?? null} + draggedElement={null} + multiDragPreview={next.multiDragPreview ?? null} + blockedClipRef={createRef()} + suppressClickRef={{ current: false }} + scrollRef={createRef()} + setHoveredClip={vi.fn()} + setShowPopover={vi.fn()} + setRangeSelection={vi.fn()} + setResizingClip={vi.fn()} + setDraggedClip={vi.fn()} + setSelectedElementId={vi.fn()} + syncClipDragAutoScroll={vi.fn()} + shiftClickClipRef={createRef()} + getPreviewElement={(el) => el} + getTrackStyle={getTrackStyle} + gsapAnimations={gsapAnimations} + selectedKeyframes={new Set()} + currentTime={0} + onContextMenuLane={next.onContextMenuLane} + onToggleTrackHidden={next.onToggleTrackHidden} + onTogglePropertyGroupKeyframe={vi.fn()} + onResizeElement={vi.fn()} + onMoveElement={vi.fn()} + onRazorSplit={vi.fn()} + onRazorSplitAll={vi.fn()} + />, + ); + }); + }; + render(options); + return { host, root, rerender: render }; +} + +function visibilityLabels(host: HTMLElement): (string | null)[] { + return Array.from(host.querySelectorAll("button[aria-label^='Hide track ']")).map((button) => + button.getAttribute("aria-label"), + ); +} + +describe("TimelineLanes track numbering", () => { + // Screen readers literally announced "Hide track 0.16666666666666666". + it("numbers tracks contiguously from 1 regardless of the fractional sort keys", () => { + const view = renderLanes({ + elements: [element("clip-a", TRACK_A), element("clip-b", TRACK_B)], + }); + + expect(visibilityLabels(view.host)).toEqual(["Hide track 1", "Hide track 2"]); + expect(view.host.innerHTML).not.toContain("0.16666666666666666"); + act(() => view.root.unmount()); + }); + + it("hands the visibility toggle the real track key, not the display index", () => { + const onToggleTrackHidden = vi.fn(); + const view = renderLanes({ + elements: [element("clip-a", TRACK_A), element("clip-b", TRACK_B)], + onToggleTrackHidden, + }); + + const second = view.host.querySelector('button[aria-label="Hide track 2"]'); + act(() => second?.click()); + + expect(onToggleTrackHidden).toHaveBeenCalledWith(TRACK_B, true); + act(() => view.root.unmount()); + }); + + // The gap menu inserts at the track it is given, so a display index here would + // drop the new clip on the wrong lane. + it("hands the lane context menu the real track key, not the display index", () => { + const onContextMenuLane = vi.fn(); + const view = renderLanes({ + elements: [element("clip-a", TRACK_A), element("clip-b", TRACK_B)], + onContextMenuLane, + }); + + // Row children: [sticky header column, time-mapped track content]. + const rows = Array.from(view.host.children); + const secondTrackContent = rows[1]?.children.item(1); + act(() => { + secondTrackContent?.dispatchEvent( + new MouseEvent("contextmenu", { bubbles: true, cancelable: true, clientX: 100 }), + ); + }); + + expect(onContextMenuLane).toHaveBeenCalledOnce(); + expect(onContextMenuLane.mock.calls[0]?.[1]).toBe(TRACK_B); + act(() => view.root.unmount()); + }); +}); + +describe("TimelineLanes disclosure target", () => { + const ANIMATIONS = new Map([["clip-a", [positionTween("clip-a")]]]); + + function ariaControlsTarget(host: HTMLElement): HTMLElement | null { + const caret = host.querySelector("button[aria-controls]"); + const id = caret?.getAttribute("aria-controls"); + return id ? host.querySelector(`#${id}`) : null; + } + + // aria-controls used to name a div in the sticky label column: it computed to + // 0x0 and held no diamonds at all. + it("resolves the caret's aria-controls to an element holding the property lanes", () => { + const view = renderLanes({ animations: ANIMATIONS, expandedClipIds: ["clip-a"] }); + const target = ariaControlsTarget(view.host); + + expect(target).not.toBeNull(); + expect(target?.querySelectorAll("[data-timeline-property-lane]").length).toBeGreaterThan(0); + act(() => view.root.unmount()); + }); + + it("still resolves the caret's aria-controls while the layer is collapsed", () => { + const view = renderLanes({ animations: ANIMATIONS, expandedClipIds: [] }); + const target = ariaControlsTarget(view.host); + + expect(target).not.toBeNull(); + expect(target?.querySelectorAll("[data-timeline-property-lane]")).toHaveLength(0); + act(() => view.root.unmount()); + }); + + // The passenger branch wraps [clip, lanes] in a transformed div that re-renders + // on every pointer move. An unstable key there remounts the lanes and drops the + // in-flight drag. + it("does not remount the lanes while a multi-clip drag slides the formation", () => { + const elements = [element("clip-a", TRACK_A), element("clip-b", TRACK_A)]; + const selectedElementIds = new Set(["clip-a", "clip-b"]); + const preview = (draggedPreviewStart: number): MultiDragPreviewInput => ({ + dragStarted: true, + draggedKey: "clip-b", + draggedOriginStart: 0, + draggedPreviewStart, + selectedKeys: selectedElementIds, + }); + const view = renderLanes({ + elements, + animations: ANIMATIONS, + expandedClipIds: ["clip-a"], + selectedElementIds, + multiDragPreview: preview(0.25), + }); + + const before = ariaControlsTarget(view.host); + const beforeLane = before?.querySelector("[data-timeline-property-lane]"); + expect(before).not.toBeNull(); + expect(beforeLane).not.toBeNull(); + + view.rerender({ + elements, + animations: ANIMATIONS, + expandedClipIds: ["clip-a"], + selectedElementIds, + multiDragPreview: preview(0.75), + }); + + // Node identity, not just presence: a remount replaces these nodes. + expect(ariaControlsTarget(view.host)).toBe(before); + expect(before?.querySelector("[data-timeline-property-lane]")).toBe(beforeLane); + act(() => view.root.unmount()); + }); +}); diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index e6e0b8a60..c69402557 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -1,5 +1,3 @@ -import { type ReactNode } from "react"; -import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { BeatStrip, BeatBackgroundLines } from "./BeatStrip"; import { TimelineClip } from "./TimelineClip"; import { TimelineClipDiamonds } from "./TimelineClipDiamonds"; @@ -7,23 +5,15 @@ import { TimelinePropertyLanes } from "./TimelinePropertyLanes"; import { TimelineTrackHeader } from "./TimelineTrackHeader"; import { resolveTrackKeyframeClip } from "./useTimelineTrackLayout"; import { clipTimingStart } from "../../hooks/gsapShared"; -import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity"; -import type { MusicBeatAnalysis } from "@hyperframes/core/beats"; import { getTimelineEditCapabilities, resolveBlockedTimelineEditIntent } from "./timelineEditing"; -import type { TimelineTheme } from "./timelineTheme"; import { CLIP_Y, CLIP_HANDLE_W, TRACK_H, getTimelineRowHeight } from "./timelineLayout"; -import { - usePlayerStore, - type TimelineElement, - type KeyframeCacheEntry, -} from "../store/playerStore"; -import type { DraggedClipState, ResizingClipState, BlockedClipState } from "./useTimelineClipDrag"; +import { usePlayerStore, type TimelineElement } from "../store/playerStore"; import { isMultiDragPassenger, multiDragPassengerOffsetPx, type MultiDragPreviewInput, } from "./timelineMultiDragPreview"; -import type { TrackVisualStyle } from "./timelineIcons"; +import type { TimelineLaneBaseProps } from "./timelineLaneProps"; import type { TimelineEditCallbacks } from "./timelineCallbacks"; import { STUDIO_KEYFRAMES_ENABLED } from "../../components/editor/manualEditingAvailability"; import { trackStudioKeyframeLaneExpand } from "../../telemetry/events"; @@ -31,83 +21,6 @@ import { SPLIT_BOUNDARY_EPSILON_S } from "../../utils/timelineElementSplit"; import { isAudioTimelineElement, isMusicTrack } from "../../utils/timelineInspector"; import { renderClipChildren } from "./timelineClipChildren"; -/** - * Props shared by the scroll container ({@link TimelineCanvas}) and the lane - * renderer below. TimelineCanvas passes these straight through via spread, so - * they are declared once here and both prop types compose from this base — no - * duplicated prop list. - */ -export interface TimelineLaneBaseProps { - pps: number; - contentOrigin: number; - contentGutter: number; - trackContentWidth: number; - theme: TimelineTheme; - displayTrackOrder: number[]; - rowHeights: readonly number[]; - trackOrder: number[]; - tracks: [number, TimelineElement[]][]; - trackStyles: Map; - laneCounts: ReadonlyMap; - selectedElementId: string | null; - selectedElementIds: Set; - hoveredClip: string | null; - draggedClip: DraggedClipState | null; - blockedClipRef: React.RefObject; - suppressClickRef: React.RefObject; - scrollRef: React.RefObject; - renderClipContent?: ( - element: TimelineElement, - style: { clip: string; label: string }, - ) => ReactNode; - renderClipOverlay?: (element: TimelineElement) => ReactNode; - onDrillDown?: (element: TimelineElement) => void; - onSelectElement?: (element: TimelineElement | null) => void; - setHoveredClip: (key: string | null) => void; - setShowPopover: (v: boolean) => void; - setRangeSelection: (v: null) => void; - setResizingClip: (v: ResizingClipState | null) => void; - setDraggedClip: (v: DraggedClipState | null) => void; - setSelectedElementId: (id: string | null) => void; - syncClipDragAutoScroll: (x: number, y: number) => void; - shiftClickClipRef: React.RefObject<{ - element: TimelineElement; - anchorX: number; - anchorY: number; - } | null>; - getPreviewElement: (element: TimelineElement) => TimelineElement; - getTrackStyle: (tag: string) => TrackVisualStyle; - keyframeCache?: Map; - gsapAnimations: Map; - selectedKeyframes: Set; - currentTime: number; - onSeek?: (time: number) => void; - onSelectSegment?: (elementId: string, target: TimelineKeyframeTarget) => void; - onClickKeyframe?: (element: TimelineElement, target: TimelineKeyframeTarget) => void; - onShiftClickKeyframe?: (elementId: string, target: TimelineKeyframeTarget) => void; - onContextMenuKeyframe?: ( - e: React.MouseEvent, - elementId: string, - target: TimelineKeyframeTarget, - ) => void; - onMoveKeyframe?: ( - elementId: string, - keyframe: TimelineKeyframeTarget, - toClipPercentage: number, - propertyGroup?: string, - tweenPercentage?: number, - animationId?: string, - ) => Promise; - onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void; - /** - * Right-click on EMPTY lane space (not on a clip — those preventDefault - * before this fires — not the gutter/ruler, not below the lanes). `time` is - * the timeline time (seconds) under the pointer on that lane. - */ - onContextMenuLane?: (e: React.MouseEvent, track: number, time: number) => void; - beatAnalysis?: MusicBeatAnalysis | null; -} - interface TimelineLanesProps extends TimelineLaneBaseProps { /** Live-derived by TimelineCanvas from {@link TimelineLaneBaseProps.draggedClip}. */ draggedElement: TimelineElement | null; @@ -218,6 +131,11 @@ export function TimelineLanes({ const keyframeClipKey = keyframeClip?.key ?? keyframeClip?.id; const keyframeClipExpanded = keyframeClipKey != null && expandedClipIds.has(keyframeClipKey); + // Minted here because this is the only place that sees BOTH ends of + // the disclosure: the caret in the sticky header and the diamond lanes + // on the canvas. Keyed by display row, not by `trackNum`, which is a + // fractional sort key and would mint ids like `...-0.16666666666666666`. + const lanesId = `timeline-lanes-track-${row}`; return (
); - const propertyLanes = showsLanes && ( + // Mounted for the track's keyframe clip in BOTH disclosure + // states, so the header caret's aria-controls resolves while + // collapsed too; collapsed just feeds it no animations, so + // the wrapper renders empty. The key is stable across a + // multi-drag: without it the passenger branch below remounts + // this subtree and interrupts the gesture. + const propertyLanes = isTrackKeyframeClip && ( { act(() => root.unmount()); }); + // The disclosure caret's aria-controls used to name a div in the STICKY LABEL + // COLUMN whose children are all absolutely positioned: it computed to 0x0 and + // held no diamonds. The real lanes had no wrapper at all to point at. + it("wraps the lanes in the identified element so aria-controls resolves to the diamonds", () => { + const animations = [ + animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 100, properties: { x: 100 } }, + ]), + animation("visual-tween", "visual", [ + { percentage: 0, properties: { opacity: 0 } }, + { percentage: 100, properties: { opacity: 1 } }, + ]), + ]; + const { host, root } = renderPropertyLanes({ id: "timeline-lanes-track-0", animations }); + const wrapper = host.querySelector("#timeline-lanes-track-0"); + + expect(wrapper).not.toBeNull(); + expect(wrapper?.querySelectorAll("[data-timeline-property-lane]")).toHaveLength(2); + expect(wrapper?.querySelectorAll("button[data-keyframe-percentage]").length).toBeGreaterThan(0); + // Load-bearing: a `position: relative` wrapper would become the containing + // block for the absolute lanes below and shift every one of them. + expect((wrapper as HTMLElement).style.position).toBe(""); + act(() => root.unmount()); + }); + + // happy-dom has no CSS engine, so measured geometry is always 0x0. The lanes' + // own inline offsets are what the component actually computes, so pin those. + it("leaves every lane's inline offsets untouched by the wrapper", () => { + const animations = [ + animation("position-tween", "position", [{ percentage: 0, properties: { x: 0 } }]), + animation("visual-tween", "visual", [{ percentage: 0, properties: { opacity: 0 } }]), + ]; + const { host, root } = renderPropertyLanes({ + id: "timeline-lanes-track-0", + animations, + clipLeftPx: 120, + clipWidthPx: 200, + }); + + const lanes = Array.from(host.querySelectorAll("[data-timeline-property-lane]")); + expect(lanes.map((lane) => lane.style.top)).toEqual([ + `${getTimelineLaneTop(0)}px`, + `${getTimelineLaneTop(1)}px`, + ]); + expect(lanes.map((lane) => lane.style.left)).toEqual(["120px", "120px"]); + expect(lanes.map((lane) => lane.style.width)).toEqual(["200px", "200px"]); + expect(lanes.map((lane) => lane.style.height)).toEqual([`${LANE_H}px`, `${LANE_H}px`]); + act(() => root.unmount()); + }); + + // The wrapper is the aria-controls target in BOTH disclosure states, so a + // collapsed layer (no animations reach it) must still resolve the id. + it("still renders the identified wrapper when there are no lanes to show", () => { + const { host, root } = renderPropertyLanes({ id: "timeline-lanes-track-0", animations: [] }); + + const wrapper = host.querySelector("#timeline-lanes-track-0"); + expect(wrapper).not.toBeNull(); + expect(wrapper?.querySelectorAll("[data-timeline-property-lane]")).toHaveLength(0); + act(() => root.unmount()); + }); + it("keeps the collapsed TimelineClipDiamonds positions and callback contract unchanged", () => { const onClickKeyframe = vi.fn(); const COLLAPSED_IDENTITY = { animationId: "position-tween", propertyGroup: "position" }; diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.tsx index 8091b3df5..641d5eba5 100644 --- a/packages/studio/src/player/components/TimelinePropertyLanes.tsx +++ b/packages/studio/src/player/components/TimelinePropertyLanes.tsx @@ -11,6 +11,12 @@ import { LANE_H, getTimelineLaneTop } from "./timelineLayout"; import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity"; export interface TimelinePropertyLanesProps { + /** + * Id of the wrapper below, so the layer's disclosure caret can point + * `aria-controls` at the lanes a sighted user sees it reveal. Minted by + * TimelineLanes, which owns both this subtree and the caret's. + */ + id?: string; animations: readonly GsapAnimation[]; clipStart: number; clipDuration: number; @@ -170,6 +176,7 @@ export function getTimelinePropertyLanes( } export function TimelinePropertyLanes({ + id, animations, clipStart, clipDuration, @@ -206,9 +213,13 @@ export function TimelinePropertyLanes({ [lanes], ); - if (laneData.length === 0) return null; + // One STATIC wrapper, never `relative`: a static box establishes no containing + // block, so every absolutely-positioned lane below still resolves against the + // track-content div and the rendered geometry is byte-identical to the bare + // fragment this replaced. It is also rendered when there are no lanes at all + // (collapsed layer), so `id` stays resolvable in both disclosure states. return ( - <> +
{laneData.map(({ group, keyframesData }, laneIndex) => (
))} - +
); } diff --git a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx index f8fb06f2c..4a0b11cd0 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx @@ -67,6 +67,7 @@ interface RenderHeaderOptions { expanded?: boolean; onSeek?: (time: number) => void; onTogglePropertyGroupKeyframe?: TimelineEditCallbacks["onTogglePropertyGroupKeyframe"]; + onToggleTrackHidden?: TimelineEditCallbacks["onToggleTrackHidden"]; } function renderHeader(options: RenderHeaderOptions = {}): { @@ -81,8 +82,12 @@ function renderHeader(options: RenderHeaderOptions = {}): { act(() => { root.render( , @@ -173,10 +178,27 @@ describe("TimelineTrackHeader", () => { // in every disclosure state — a hover-gated eye is unusable by keyboard. it("keeps the visibility eye mounted whether the layer is expanded or collapsed", () => { const view = renderHeader({ expanded: true }); - expect(view.host.querySelector('button[aria-label="Hide track 0"]')).not.toBeNull(); + expect(view.host.querySelector('button[aria-label="Hide track 1"]')).not.toBeNull(); view.rerender({ expanded: false }); - expect(view.host.querySelector('button[aria-label="Hide track 0"]')).not.toBeNull(); + expect(view.host.querySelector('button[aria-label="Hide track 1"]')).not.toBeNull(); + act(() => view.root.unmount()); + }); + + // trackNumber is a fractional z-order sort key, so building the label from it + // made screen readers announce "Hide track 0.16666666666666666". The display + // number is label-only; the toggle still routes by the real key. + it("announces the display track number but toggles with the real fractional key", () => { + const onToggleTrackHidden = vi.fn(); + const view = renderHeader({ onToggleTrackHidden }); + const eye = view.host.querySelector('button[aria-label="Hide track 1"]'); + + expect(eye).not.toBeNull(); + expect(eye?.title).toBe("Hide track 1"); + expect(view.host.innerHTML).not.toContain("0.16666666666666666"); + + act(() => eye?.click()); + expect(onToggleTrackHidden).toHaveBeenCalledWith(1 / 6, true); act(() => view.root.unmount()); }); diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index 06a17eed2..d8a00e24f 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -17,8 +17,17 @@ import { import { valueReadout } from "./trackHeaderLaneValues"; interface TimelineTrackHeaderProps { + /** The track's real key: a FRACTIONAL z-order sort value. Routes callbacks; + * never shown or announced. */ trackNumber: number; + /** The track's 1-based position in the rendered order: the only number safe + * to put in a label. Announcing `trackNumber` read out "track + * 0.16666666666666666". */ + trackDisplayNumber: number; trackLabel: string; + /** Id of the canvas-side lanes element the disclosure caret expands. Minted by + * TimelineLanes, which is the one place that sees both subtrees. */ + lanesId: string; contentOrigin: number; /** The track's active keyframe clip (selected, else primary) — the one whose * disclosure + property rows this header shows, whether expanded or not. */ @@ -40,16 +49,20 @@ interface TimelineTrackHeaderProps { function VisibilityButton({ hidden, trackNumber, + trackDisplayNumber, visible, onToggle, }: { hidden: boolean; trackNumber: number; + trackDisplayNumber: number; visible: boolean; onToggle: TimelineEditCallbacks["onToggleTrackHidden"]; }) { if (!visible) return