Skip to content

Commit 4905b4d

Browse files
fix(studio): make clip peak warnings plain and hoverable (#5099)
* fix(studio): make clip peak warnings plain and hoverable * fix(studio): expose peak detail on clip focus above waveforms * fix(studio): preserve shared badge styling and track test identity
1 parent 00342fc commit 4905b4d

7 files changed

Lines changed: 111 additions & 30 deletions

File tree

‎packages/studio/src/components/ui/Tooltip.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,30 @@ interface TooltipProps {
1414
children: ReactElement<{ "aria-describedby"?: string }>;
1515
/** Hover delay in ms. */
1616
delay?: number;
17+
disabled?: boolean;
1718
side?: "top" | "bottom" | "left" | "right";
1819
}
1920

2021
/** Matches the old bubble's gap from its trigger, and its viewport margin. */
2122
const SIDE_OFFSET = 6;
2223
const VIEWPORT_MARGIN = 8;
2324

24-
export function Tooltip({ label, children, delay = 400, side = "top" }: TooltipProps) {
25+
export function Tooltip({
26+
label,
27+
children,
28+
delay = 400,
29+
side = "top",
30+
disabled = false,
31+
}: TooltipProps) {
2532
const [open, setOpen] = useState(false);
2633
const boxRef = useRef<HTMLSpanElement>(null);
2734
const tooltipId = useId();
2835

2936
return (
30-
<BaseTooltip.Root open={open} onOpenChange={setOpen}>
37+
<BaseTooltip.Root open={open} onOpenChange={setOpen} disabled={disabled}>
3138
<BaseTooltip.Trigger delay={delay} render={<span ref={boxRef} className="contents" />}>
3239
{cloneElement(children, {
33-
"aria-describedby": open ? tooltipId : children.props["aria-describedby"],
40+
"aria-describedby": open && !disabled ? tooltipId : children.props["aria-describedby"],
3441
})}
3542
</BaseTooltip.Trigger>
3643
<BaseTooltip.Portal>

‎packages/studio/src/player/components/ClipPeakMarks.test.tsx‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import { act } from "react";
33
import { describe, expect, it, vi } from "vitest";
44
import { createHappyDomRootHarness } from "./testRootHarness";
55
import { ClipPeakMarks } from "./ClipPeakMarks";
6+
import { TimelineClip } from "./TimelineClip";
7+
8+
Object.defineProperty(globalThis, "IS_REACT_ACT_ENVIRONMENT", { configurable: true, value: true });
69

710
const harness = createHappyDomRootHarness();
811

@@ -13,29 +16,70 @@ async function render(url: string, bins: number[], gain: number) {
1316
);
1417
const host = document.createElement("div");
1518
document.body.appendChild(host);
16-
await act(async () => {
17-
harness.mount(host).render(
18-
<ClipPeakMarks peaksUrl={url} sourceWindow={{ mediaStart: 0, sourceSpan: 2 }} gain={gain}>
19+
const root = harness.mount(host);
20+
const view = (volume: number) => (
21+
<TimelineClip
22+
el={{ id: "audio", tag: "audio", start: 0, duration: 2, track: 0 }}
23+
pps={16}
24+
clipY={0}
25+
isSelected={false}
26+
isHovered={false}
27+
hasCustomContent
28+
capabilities={{ canMove: true, canTrimStart: true, canTrimEnd: true }}
29+
isComposition={false}
30+
tabIndex={0}
31+
onHoverStart={() => {}}
32+
onHoverEnd={() => {}}
33+
onClick={() => {}}
34+
onDoubleClick={() => {}}
35+
>
36+
<ClipPeakMarks peaksUrl={url} sourceWindow={{ mediaStart: 0, sourceSpan: 2 }} gain={volume}>
1937
<span>wave</span>
20-
</ClipPeakMarks>,
21-
);
22-
});
38+
</ClipPeakMarks>
39+
</TimelineClip>
40+
);
41+
await act(async () => root.render(view(gain)));
2342
await act(async () => {});
2443
vi.unstubAllGlobals();
25-
return host;
44+
return { host, setGain: async (value: number) => act(async () => root.render(view(value))) };
2645
}
2746

2847
describe("ClipPeakMarks", () => {
29-
it("paints red marks and the peak on a clip that redlines", async () => {
30-
const host = await render("/api/projects/p/peaks/loud.mp4", [0.2, 0.98], 1);
48+
it("uses a plain warning and describes the focused clip with the numeric peak", async () => {
49+
const { host } = await render("/api/projects/p/peaks/loud.mp4", [0.2, 0.98], 1);
3150
expect(host.textContent).toContain("wave");
3251
expect(host.querySelector("[data-testid=clip-peak-marks]")?.textContent).toContain(
33-
"▲ peaks −0.2 dBFS",
52+
"▲ Too loud",
3453
);
54+
expect(host.querySelector("[data-peak-text]")?.textContent).not.toContain("dBFS");
55+
const clip = host.querySelector("button")!;
56+
await act(async () => clip.focus());
57+
await vi.waitFor(() =>
58+
expect(document.querySelector('[role="tooltip"]')?.textContent).toBe(
59+
"Peaks −0.2 dBFS at this volume; export lowers the whole mix",
60+
),
61+
);
62+
expect(clip.getAttribute("aria-describedby")).toBe(
63+
document.querySelector('[role="tooltip"]')?.id,
64+
);
65+
expect(host.querySelectorAll("button")).toHaveLength(1);
66+
});
67+
68+
it("removes the focused warning when volume becomes quiet without replacing the clip", async () => {
69+
const { host, setGain } = await render("/api/projects/p/peaks/changing.mp4", [1, 1], 1);
70+
const clip = host.querySelector("button")!;
71+
await act(async () => clip.focus());
72+
await vi.waitFor(() => expect(document.querySelector('[role="tooltip"]')).not.toBeNull());
73+
await setGain(0.25);
74+
await vi.waitFor(() => expect(document.querySelector('[role="tooltip"]')).toBeNull());
75+
expect(clip.getAttribute("aria-describedby")).toBeNull();
76+
expect(document.activeElement).toBe(clip);
77+
expect(host.querySelector("button")).toBe(clip);
78+
expect(host.querySelector("[data-peak-badge]")).toBeNull();
3579
});
3680

3781
it("paints nothing on a quiet clip", async () => {
38-
const host = await render("/api/projects/p/peaks/quiet.mp4", [0.25, 0.25], 1);
82+
const { host } = await render("/api/projects/p/peaks/quiet.mp4", [0.25, 0.25], 1);
3983
expect(host.querySelector("[data-testid=clip-peak-marks]")).toBeNull();
4084
});
4185
});

‎packages/studio/src/player/components/ClipPeakMarks.tsx‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { useEffect, useState, type ReactNode } from "react";
1+
import { useContext, useEffect, useState, type ReactNode } from "react";
22
import { clipPeakRuns, type ClipSourceWindow, type PeakMap } from "./clipPeakRuns";
33
import { loadPeakMap } from "./clipPeakMap";
4+
import { ClipPeakDescriptionContext } from "./ClipPeakTooltip";
45

56
const dbText = (db: number) => `${db < 0 ? "−" : "+"}${Math.abs(db).toFixed(1)} dBFS`;
67

@@ -30,15 +31,20 @@ export function ClipPeakMarks({
3031
const { runs, peakDbfs } = map
3132
? clipPeakRuns(map, sourceWindow, gain)
3233
: { runs: [], peakDbfs: null };
34+
const detail =
35+
runs.length > 0 && peakDbfs !== null
36+
? `Peaks ${dbText(peakDbfs)} at this volume; export lowers the whole mix`
37+
: null;
38+
const setDetail = useContext(ClipPeakDescriptionContext);
39+
useEffect(() => {
40+
setDetail?.(detail);
41+
return () => setDetail?.(null);
42+
}, [detail, setDetail]);
3343
return (
34-
<div className="relative h-full w-full">
44+
<div className="relative isolate h-full w-full">
3545
{children}
36-
{runs.length > 0 && peakDbfs !== null && (
37-
<div
38-
className="pointer-events-none absolute inset-0"
39-
data-testid="clip-peak-marks"
40-
title={`Peaks ${dbText(peakDbfs)} at this volume; export lowers the whole mix`}
41-
>
46+
{detail !== null && (
47+
<div className="pointer-events-none absolute inset-0" data-testid="clip-peak-marks">
4248
{runs.map((run) => (
4349
<div
4450
key={run.from}
@@ -50,10 +56,10 @@ export function ClipPeakMarks({
5056
/>
5157
))}
5258
<span
53-
className="absolute top-0 right-1 font-mono text-[9px] leading-none text-red-400"
59+
className="pointer-events-auto absolute right-4 bottom-1 font-mono text-[9px] leading-none text-red-400"
5460
data-peak-badge
5561
>
56-
▲<span data-peak-text> peaks {dbText(peakDbfs)}</span>
62+
▲<span data-peak-text> Too loud</span>
5763
</span>
5864
</div>
5965
)}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { cloneElement, createContext, useState, type ReactElement } from "react";
2+
import { Tooltip } from "../../components/ui/Tooltip";
3+
4+
export const ClipPeakDescriptionContext = createContext<((detail: string | null) => void) | null>(
5+
null,
6+
);
7+
8+
export function ClipPeakTooltip({
9+
children,
10+
}: {
11+
children: ReactElement<{ title?: string; "aria-describedby"?: string }>;
12+
}) {
13+
const [detail, setDetail] = useState<string | null>(null);
14+
return (
15+
<ClipPeakDescriptionContext.Provider value={setDetail}>
16+
<Tooltip label={detail ?? ""} disabled={detail === null}>
17+
{cloneElement(children, { title: detail === null ? children.props.title : undefined })}
18+
</Tooltip>
19+
</ClipPeakDescriptionContext.Provider>
20+
);
21+
}

‎packages/studio/src/player/components/Timeline.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,8 @@ describe("timeline viewport geometry", () => {
8787
});
8888
});
8989

90-
/** The track's content row: a clip's parent, past its `display: contents` wrapper. */
9190
function trackContentOf(clip: HTMLElement | null | undefined): HTMLElement | null {
92-
let row = clip?.parentElement ?? null;
93-
while (row?.style.display === "contents") row = row.parentElement;
94-
return row;
91+
return clip?.closest<HTMLElement>('[role="gridcell"]') ?? null;
9592
}
9693

9794
function getHorizontalGeometry(host: HTMLElement, clipId: string, tickLabel: string) {

‎packages/studio/src/player/components/TimelineClip.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { ClipBadges } from "./ClipBadges";
1616
import { linkLabelColor } from "./linkLabelColor";
1717
import { OutOfSyncBadge } from "./OutOfSyncBadge";
1818
import { clipSpeedSuffix } from "./clipToolAttrs";
19+
import { ClipPeakTooltip } from "./ClipPeakTooltip";
1920

2021
interface TimelineClipProps {
2122
el: TimelineElement;
@@ -128,7 +129,7 @@ export const TimelineClip = memo(function TimelineClip({
128129
...passengerStyle,
129130
};
130131

131-
return (
132+
const clip = (
132133
<button
133134
type="button"
134135
data-clip={isGestureActor ? undefined : "true"}
@@ -256,4 +257,5 @@ export const TimelineClip = memo(function TimelineClip({
256257
)}
257258
</button>
258259
);
260+
return isAudioClip || el.hasAudio ? <ClipPeakTooltip>{clip}</ClipPeakTooltip> : clip;
259261
});

‎packages/studio/src/styles/components.css‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,14 +314,18 @@
314314
display: none;
315315
}
316316

317+
[data-peak-badge] {
318+
z-index: 20;
319+
}
320+
317321
.audible-video-wave [data-peak-badge] {
318322
top: 50%;
323+
bottom: auto;
319324
right: var(--timeline-clip-radius);
320325
translate: 0 -50%;
321326
padding: 1px 3px;
322327
border-radius: 3px;
323328
background: var(--timeline-clip-chip-audio-bg);
324-
z-index: 20;
325329
}
326330

327331
.timeline-clip.is-audio.is-hovered {

0 commit comments

Comments
 (0)