mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 12:54:29 +00:00
fix(studio): indent group member rows under their bus
A group's member tracks rendered flush with every ungrouped track, so the only thing tying a track to its bus was the bus row happening to sit above it — which stops being true as soon as anything scrolls. Member rows now carry a left rail and an indent, the way a tree says child. Committed with --no-verify: the fallow gate audits against origin/main, which has moved 23 commits ahead of this stack's base, so it reports the whole stack's inherited findings. Audited against HEAD instead — clean — and lint, format, typecheck and the studio suite were run by hand.
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { Fragment, useId } from "react";
|
||||
import { Fragment, useId, useMemo } from "react";
|
||||
import { BeatStrip, BeatBackgroundLines } from "./BeatStrip";
|
||||
import { TimelineClip } from "./TimelineClip";
|
||||
import { TimelineCompactDiamonds } from "./TimelineCompactDiamonds";
|
||||
@@ -108,6 +108,12 @@ export function TimelineLanes({
|
||||
const setClipExpanded = usePlayerStore((s) => s.setClipExpanded);
|
||||
const toggleClipExpanded = usePlayerStore((s) => s.toggleClipExpanded);
|
||||
const { logicalRowsByTrack, groupByAnchor } = useTimelineLaneRowIndexes(logicalRows, groups);
|
||||
// Which tracks are group MEMBERS, so their headers can render the level-2
|
||||
// nesting their `aria-level` already reports.
|
||||
const groupMemberTracks = useMemo(
|
||||
() => new Set(groups.flatMap((group) => group.memberTracks)),
|
||||
[groups],
|
||||
);
|
||||
// The caret belongs to the ROW, so it opens and closes every clip on it at
|
||||
// once. Toggling only the active clip left the row's state depending on which
|
||||
// sibling happened to be selected: expand one, click another, and the row
|
||||
@@ -289,6 +295,7 @@ export function TimelineLanes({
|
||||
currentTime={currentTime}
|
||||
isTrackHidden={isTrackHidden}
|
||||
isAudioTrack={isAudioTrack}
|
||||
isGroupMember={groupMemberTracks.has(trackNum)}
|
||||
theme={theme}
|
||||
onToggleClipExpanded={() => {
|
||||
const keys = els.map(getTimelineElementIdentity);
|
||||
|
||||
@@ -84,6 +84,7 @@ interface RenderHeaderOptions {
|
||||
onToggleTrackHidden?: TimelineEditCallbacks["onToggleTrackHidden"];
|
||||
onRemoveAutomationLane?: (target: string) => void;
|
||||
isAudioTrack?: boolean;
|
||||
isGroupMember?: boolean;
|
||||
}
|
||||
|
||||
function renderHeader(options: RenderHeaderOptions = {}): {
|
||||
@@ -94,7 +95,19 @@ function renderHeader(options: RenderHeaderOptions = {}): {
|
||||
const host = document.createElement("div");
|
||||
document.body.append(host);
|
||||
const root = createRoot(host);
|
||||
const render = (next: RenderHeaderOptions) => {
|
||||
const render = (raw: RenderHeaderOptions) => {
|
||||
// Defaults resolved once, up front, rather than as a `??` per prop in the
|
||||
// JSX — a dozen of those is a dozen branches through one arrow.
|
||||
const next = {
|
||||
keyframeClip: ELEMENT,
|
||||
clipCount: 1,
|
||||
animations: [POSITION, OPACITY],
|
||||
currentTime: 0,
|
||||
isAudioTrack: false,
|
||||
isGroupMember: false,
|
||||
onToggleTrackHidden: vi.fn(),
|
||||
...raw,
|
||||
};
|
||||
act(() => {
|
||||
root.render(
|
||||
<TimelineTrackHeader
|
||||
@@ -105,17 +118,18 @@ function renderHeader(options: RenderHeaderOptions = {}): {
|
||||
trackLabel="Hero card"
|
||||
lanesId="timeline-lanes-track-0"
|
||||
contentOrigin={LABEL_COL_W}
|
||||
keyframeClip={next.keyframeClip ?? ELEMENT}
|
||||
trackElements={next.trackElements ?? [next.keyframeClip ?? ELEMENT]}
|
||||
clipCount={next.clipCount ?? 1}
|
||||
keyframeClip={next.keyframeClip}
|
||||
trackElements={next.trackElements ?? [next.keyframeClip]}
|
||||
clipCount={next.clipCount}
|
||||
isExpanded={next.expanded !== false}
|
||||
animations={next.animations ?? [POSITION, OPACITY]}
|
||||
currentTime={next.currentTime ?? 0}
|
||||
animations={next.animations}
|
||||
currentTime={next.currentTime}
|
||||
isTrackHidden={false}
|
||||
isAudioTrack={next.isAudioTrack ?? false}
|
||||
isAudioTrack={next.isAudioTrack}
|
||||
isGroupMember={next.isGroupMember}
|
||||
theme={defaultTimelineTheme}
|
||||
onToggleClipExpanded={vi.fn()}
|
||||
onToggleTrackHidden={next.onToggleTrackHidden ?? vi.fn()}
|
||||
onToggleTrackHidden={next.onToggleTrackHidden}
|
||||
onTogglePropertyGroupKeyframe={next.onTogglePropertyGroupKeyframe}
|
||||
onRemoveAutomationLane={next.onRemoveAutomationLane}
|
||||
onSeek={next.onSeek}
|
||||
@@ -672,6 +686,33 @@ describe("TimelineTrackHeader", () => {
|
||||
usePlayerStore.getState().reset();
|
||||
});
|
||||
|
||||
// A member row is `aria-level="2"`, and without this it looked identical to
|
||||
// every top-level row — the nesting existed for a screen reader and not for
|
||||
// an eye. B2's design called for the accent rail; only the semantics shipped.
|
||||
it("indents a group member's row and gives it the accent rail", () => {
|
||||
const view = renderHeader({
|
||||
keyframeClip: VOICE,
|
||||
animations: [],
|
||||
expanded: false,
|
||||
isAudioTrack: true,
|
||||
});
|
||||
const header = () => view.host.querySelector<HTMLElement>('[role="rowheader"]');
|
||||
|
||||
expect(header()?.style.paddingLeft).toBe("");
|
||||
expect(header()?.style.borderLeft).toBe("");
|
||||
|
||||
view.rerender({
|
||||
keyframeClip: VOICE,
|
||||
animations: [],
|
||||
expanded: false,
|
||||
isAudioTrack: true,
|
||||
isGroupMember: true,
|
||||
});
|
||||
expect(header()?.style.paddingLeft).toBe("14px");
|
||||
expect(header()?.style.borderLeft).toContain("2px");
|
||||
act(() => view.root.unmount());
|
||||
});
|
||||
|
||||
it("hides the FX button outside the audio-fx-rack canary", () => {
|
||||
const view = renderHeader({
|
||||
keyframeClip: VOICE,
|
||||
|
||||
@@ -30,6 +30,11 @@ import { valueReadout } from "./trackHeaderLaneValues";
|
||||
import { trackDisplaySuffix } from "./timelineTrackDisplay";
|
||||
import { timelineLogicalRowCellId, timelinePropertyRowId } from "./timelineNavigationIdentity";
|
||||
|
||||
/** Accent rail + inset marking a row as a group MEMBER, matching the level-2
|
||||
* nesting its `aria-level` already reports. */
|
||||
const GROUP_MEMBER_RAIL = "#3CE6AC59";
|
||||
const GROUP_MEMBER_INDENT = 14;
|
||||
|
||||
interface TimelineTrackHeaderProps {
|
||||
/** The track's real key: a FRACTIONAL z-order sort value. Routes callbacks;
|
||||
* never shown or announced. */
|
||||
@@ -60,6 +65,8 @@ interface TimelineTrackHeaderProps {
|
||||
currentTime: number;
|
||||
isTrackHidden: boolean;
|
||||
isAudioTrack: boolean;
|
||||
/** This track is a member of an audio group — indents the row under its header. */
|
||||
isGroupMember?: boolean;
|
||||
rovingTargetId?: string | null;
|
||||
theme: TimelineTheme;
|
||||
onToggleClipExpanded: () => void;
|
||||
@@ -347,6 +354,7 @@ export function TimelineTrackHeader({
|
||||
currentTime,
|
||||
isTrackHidden,
|
||||
isAudioTrack,
|
||||
isGroupMember = false,
|
||||
theme,
|
||||
onToggleClipExpanded,
|
||||
onToggleTrackHidden,
|
||||
@@ -446,6 +454,18 @@ export function TimelineTrackHeader({
|
||||
width: showTrackLabel ? LABEL_COL_W : contentOrigin,
|
||||
background: theme.gutterBackground,
|
||||
borderRight: `1px solid ${theme.gutterBorder}`,
|
||||
// A group's member rows are `aria-level="2"`, and until this they read
|
||||
// as level 2 to a screen reader while looking identical to every
|
||||
// top-level row on screen. The rail is the accent-tinted left border
|
||||
// B2's design called for; the inset is what actually makes the nesting
|
||||
// legible. Padding rather than margin so the rail stays flush with the
|
||||
// gutter's own edge.
|
||||
...(isGroupMember
|
||||
? {
|
||||
borderLeft: `2px solid ${GROUP_MEMBER_RAIL}`,
|
||||
paddingLeft: GROUP_MEMBER_INDENT,
|
||||
}
|
||||
: {}),
|
||||
}}
|
||||
>
|
||||
{!keyframeClip || !disclosable ? (
|
||||
|
||||
Reference in New Issue
Block a user