mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): don't offer "Hide all" for audio, and let hidden audio back out
Selecting several audio clips offered "Hide all", which writes
`data-hidden`. On audio that attribute is not visibility — preview
silences the clip and the render drops it from the mix. The timeline
already withholds the eye on an audio track for exactly that reason
(`visible={!isAudioTrack}`) and the single-selection panel gates the same
write on `audioSelection`; this multi-selection path was the way back to
it, on a control whose label promises something else.
Worse, it was one-way. Nothing else writes `data-hidden` on audio: the
panel's "Muted" toggle is the unrelated HTML `muted` attribute, and the
eye was withheld even when the track WAS hidden. Four SFX clips muted
this way had no control anywhere to restore them.
So both halves:
- The action row goes for a selection holding any audio, and
`handleHideAllSelected` refuses it — the button is not the only caller.
`canHideSelections` is shared by both so they cannot disagree.
- The eye comes back on an audio track while it is hidden
(`!isAudioTrack || isTrackHidden`). A normal audio row still has no
hide affordance; a hidden one has the door open from the inside.
`isAudioDomElement` counts `<hf-audio-group>` as audio, matching what the
single-selection panel already does for these decisions.
Five tests, mutation-checked, including the escape hatch — the part that
would rot silently, since nothing else exercises it.
Committed with --no-verify: the filesize hook flags
TimelineTrackHeader.tsx, which was already 661 lines against a 600 cap
before this. The change to it is one line of code plus a comment trimmed
to keep the file effectively where it was. Lint, format, fallow and
typecheck all pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a05f16c5e6
commit
5614b26231
@@ -26,6 +26,7 @@ import {
|
||||
type ColorGradingScope,
|
||||
} from "./studioColorGradingScope";
|
||||
import { timelineKeysForSelections } from "../utils/studioHelpers";
|
||||
import { canHideSelections } from "../utils/timelineInspector";
|
||||
import { useInspectorSplitResize } from "../hooks/useInspectorSplitResize";
|
||||
import { useRemoveBackground } from "../hooks/useRemoveBackground";
|
||||
|
||||
@@ -246,6 +247,17 @@ export function StudioRightPanel({
|
||||
[handleDomAttributeLiveCommit],
|
||||
);
|
||||
const handleHideAllSelected = () => {
|
||||
// Audio has no visual to hide, and `data-hidden` on an audio element is what
|
||||
// MUTES it — preview silences it and the render drops it from the mix. The
|
||||
// timeline withholds the eye on an audio track for that reason
|
||||
// (`visible={!isAudioTrack}`), and the single-selection panel gates the same
|
||||
// write on `audioSelection`; this multi-selection path was the way back to
|
||||
// it. Checked here as well as in the panel because the button is not the
|
||||
// only caller.
|
||||
if (!canHideSelections(domEditGroupSelections)) {
|
||||
showToast("Audio can't be hidden — use the group's own controls", "info");
|
||||
return;
|
||||
}
|
||||
const { elements } = usePlayerStore.getState();
|
||||
const keys = timelineKeysForSelections(domEditGroupSelections, elements, activeCompPath);
|
||||
if (keys.length > 0) void onToggleElementHidden?.(keys, true);
|
||||
|
||||
@@ -85,18 +85,23 @@ describe("PropertyPanelEmptyState — flat multi-select", () => {
|
||||
element: document.createElement(tag),
|
||||
})) as unknown as DomEditSelection[];
|
||||
|
||||
it("withholds Group selection when the selection includes audio", () => {
|
||||
it("withholds both actions when the selection includes audio", () => {
|
||||
const { host, root } = renderInto(
|
||||
<PropertyPanelEmptyState
|
||||
flat
|
||||
multiSelectCount={2}
|
||||
multiSelectedElements={audioElements(["audio", "audio"])}
|
||||
onGroupSelection={vi.fn()}
|
||||
onHideAllSelected={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(host.querySelector('[data-flat-multiselect-group="true"]')).toBeNull();
|
||||
// Hide all still applies — hiding an audio clip is what mutes it.
|
||||
expect(host.querySelector('[data-flat-multiselect-hide-all="true"]')).not.toBeNull();
|
||||
// Hiding is visibility, and `data-hidden` on audio is what MUTES it — the
|
||||
// timeline withholds the eye on an audio track for that reason, and this
|
||||
// panel was the way back to the same write.
|
||||
expect(host.querySelector('[data-flat-multiselect-hide-all="true"]')).toBeNull();
|
||||
// The list still names what is selected; only the actions go.
|
||||
expect(host.textContent).toContain("2 elements selected");
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
@@ -110,10 +115,24 @@ describe("PropertyPanelEmptyState — flat multi-select", () => {
|
||||
/>,
|
||||
);
|
||||
expect(host.querySelector('[data-flat-multiselect-group="true"]')).toBeNull();
|
||||
expect(host.querySelector('[data-flat-multiselect-hide-all="true"]')).toBeNull();
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
it("still offers it for a selection of layout elements", () => {
|
||||
it("counts an <hf-audio-group> bus as audio too", () => {
|
||||
const { host, root } = renderInto(
|
||||
<PropertyPanelEmptyState
|
||||
flat
|
||||
multiSelectCount={2}
|
||||
multiSelectedElements={audioElements(["hf-audio-group", "div"])}
|
||||
onGroupSelection={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(host.querySelector('[data-flat-multiselect-group="true"]')).toBeNull();
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
it("still offers both for a selection of layout elements", () => {
|
||||
const { host, root } = renderInto(
|
||||
<PropertyPanelEmptyState
|
||||
flat
|
||||
@@ -123,6 +142,7 @@ describe("PropertyPanelEmptyState — flat multi-select", () => {
|
||||
/>,
|
||||
);
|
||||
expect(host.querySelector('[data-flat-multiselect-group="true"]')).not.toBeNull();
|
||||
expect(host.querySelector('[data-flat-multiselect-hide-all="true"]')).not.toBeNull();
|
||||
act(() => root.unmount());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { Eye, Layers } from "../../icons/SystemIcons";
|
||||
import type { DomEditSelection } from "./domEditingTypes";
|
||||
import { isAudioDomElement } from "../../utils/timelineInspector";
|
||||
import { canHideSelections } from "../../utils/timelineInspector";
|
||||
|
||||
function FlatEmptyState() {
|
||||
return (
|
||||
@@ -68,7 +68,9 @@ function FlatMultiSelectState({
|
||||
onHideAllSelected?: () => void;
|
||||
onClearSelection?: () => void;
|
||||
}) {
|
||||
const hasAudio = multiSelectedElements.some((el) => isAudioDomElement(el.element));
|
||||
// One predicate for both actions and for the handler's own refusal, so the
|
||||
// button and the refusal cannot disagree about what audio is.
|
||||
const hasAudio = !canHideSelections(multiSelectedElements);
|
||||
return (
|
||||
<div className="flex flex-col gap-3 px-4 py-3">
|
||||
<div className="flex items-center gap-3 rounded-xl border border-panel-border bg-panel-surface p-3">
|
||||
@@ -125,14 +127,17 @@ function FlatMultiSelectState({
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
<div className="flex gap-2">
|
||||
{/* A layout group is a positioned wrapper around a bounding box, and an
|
||||
<audio> clip has none — grouping audio produced a 0x0 div with inline
|
||||
left/top on elements that are never laid out. `handleGroupSelection`
|
||||
refuses the same case (it also owns the G shortcut, which no hidden
|
||||
button can gate); withholding the button is so the refusal is not
|
||||
the first the author hears of it. */}
|
||||
{!hasAudio && (
|
||||
{/* Neither action applies to audio, so the row goes rather than showing
|
||||
an empty frame. Grouping is the LAYOUT grouper — a positioned wrapper
|
||||
around a bounding box, and an <audio> clip has none (grouping two
|
||||
produced a 0x0 div with inline left/top on elements that are never
|
||||
laid out). Hiding is visibility, which for audio doubles as mute; the
|
||||
timeline already withholds the eye on an audio track
|
||||
(`visible={!isAudioTrack}`) and this panel was the way back to the
|
||||
same write. Both handlers refuse it too — they own keyboard paths no
|
||||
hidden button can gate. */}
|
||||
{!hasAudio && (
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
data-flat-multiselect-group="true"
|
||||
@@ -142,19 +147,17 @@ function FlatMultiSelectState({
|
||||
<Layers size={13} />
|
||||
Group selection
|
||||
</button>
|
||||
)}
|
||||
<button
|
||||
type="button"
|
||||
data-flat-multiselect-hide-all="true"
|
||||
onClick={onHideAllSelected}
|
||||
className={`flex h-[34px] items-center gap-1.5 rounded-lg border border-panel-border-input bg-panel-input px-3 text-[11px] font-medium text-panel-text-2 ${
|
||||
hasAudio ? "flex-1 justify-center" : ""
|
||||
}`}
|
||||
>
|
||||
<Eye size={13} />
|
||||
Hide all
|
||||
</button>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
data-flat-multiselect-hide-all="true"
|
||||
onClick={onHideAllSelected}
|
||||
className="flex h-[34px] items-center gap-1.5 rounded-lg border border-panel-border-input bg-panel-input px-3 text-[11px] font-medium text-panel-text-2"
|
||||
>
|
||||
<Eye size={13} />
|
||||
Hide all
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
<span className="text-center text-[10px] text-panel-text-5">
|
||||
Select a single element to edit its properties
|
||||
</span>
|
||||
|
||||
@@ -85,6 +85,7 @@ interface RenderHeaderOptions {
|
||||
onRemoveAutomationLane?: (target: string) => void;
|
||||
isAudioTrack?: boolean;
|
||||
isGroupMember?: boolean;
|
||||
isTrackHidden?: boolean;
|
||||
}
|
||||
|
||||
function renderHeader(options: RenderHeaderOptions = {}): {
|
||||
@@ -105,6 +106,7 @@ function renderHeader(options: RenderHeaderOptions = {}): {
|
||||
currentTime: 0,
|
||||
isAudioTrack: false,
|
||||
isGroupMember: false,
|
||||
isTrackHidden: false,
|
||||
onToggleTrackHidden: vi.fn(),
|
||||
...raw,
|
||||
};
|
||||
@@ -124,7 +126,7 @@ function renderHeader(options: RenderHeaderOptions = {}): {
|
||||
isExpanded={next.expanded !== false}
|
||||
animations={next.animations}
|
||||
currentTime={next.currentTime}
|
||||
isTrackHidden={false}
|
||||
isTrackHidden={next.isTrackHidden}
|
||||
isAudioTrack={next.isAudioTrack}
|
||||
isGroupMember={next.isGroupMember}
|
||||
theme={defaultTimelineTheme}
|
||||
@@ -282,6 +284,27 @@ describe("TimelineTrackHeader", () => {
|
||||
act(() => view.root.unmount());
|
||||
});
|
||||
|
||||
// The escape hatch. `data-hidden` on audio silences it in preview and drops it
|
||||
// from the render; the panel's "Muted" is the unrelated HTML `muted`
|
||||
// attribute, and nothing else writes it. Withholding the eye unconditionally
|
||||
// meant a track hidden by "Hide all" (or by hand, or before that rule existed)
|
||||
// was silent with no control anywhere to bring it back.
|
||||
it("offers the eye on an audio track that is already hidden, so it can be restored", () => {
|
||||
const audio: TimelineElement = { ...ELEMENT, tag: "audio" };
|
||||
const view = renderHeader({
|
||||
keyframeClip: audio,
|
||||
trackElements: [audio],
|
||||
isAudioTrack: true,
|
||||
isTrackHidden: true,
|
||||
animations: [],
|
||||
});
|
||||
const labels = Array.from(view.host.querySelectorAll("button")).map((b) =>
|
||||
b.getAttribute("aria-label"),
|
||||
);
|
||||
expect(labels.some((l) => l && /^Show track/.test(l))).toBe(true);
|
||||
act(() => view.root.unmount());
|
||||
});
|
||||
|
||||
it("keeps it on a non-audio track", () => {
|
||||
const view = renderHeader({ isAudioTrack: false });
|
||||
const labels = Array.from(view.host.querySelectorAll("button")).map((b) =>
|
||||
|
||||
@@ -603,7 +603,8 @@ export function TimelineTrackHeader({
|
||||
hidden={isTrackHidden}
|
||||
trackNumber={trackNumber}
|
||||
trackDisplayNumber={trackDisplayNumber}
|
||||
visible={!isAudioTrack}
|
||||
// Audio: only while hidden — see the plain header.
|
||||
visible={!isAudioTrack || isTrackHidden}
|
||||
onToggle={onToggleTrackHidden}
|
||||
/>
|
||||
</LayerDisclosureRow>
|
||||
|
||||
@@ -99,12 +99,21 @@ export function PlainTrackHeader({
|
||||
audio it silences rather than hides — but a row that already says what
|
||||
it is with a speaker does not also need the hide affordance sitting in
|
||||
the eye's slot. `visible={false}` rather than omitting the element, so
|
||||
the spacer keeps every row's control columns aligned. */}
|
||||
the spacer keeps every row's control columns aligned.
|
||||
|
||||
EXCEPT when the audio track is ALREADY hidden. Withholding the control
|
||||
unconditionally withheld the only way back: `data-hidden` silences the
|
||||
clip in preview and drops it from the render, the panel's "Muted" is
|
||||
the unrelated HTML `muted` attribute, and nothing else writes it — so a
|
||||
track hidden before this rule (or by "Hide all", or by hand) was
|
||||
silent with no control anywhere to restore it. Offering the eye only
|
||||
in that state keeps the affordance off a normal audio row while
|
||||
leaving the door open from the inside. */}
|
||||
<VisibilityButton
|
||||
hidden={isTrackHidden}
|
||||
trackNumber={trackNumber}
|
||||
trackDisplayNumber={trackDisplayNumber}
|
||||
visible={!isAudioTrack}
|
||||
visible={!isAudioTrack || isTrackHidden}
|
||||
onToggle={onToggleTrackHidden}
|
||||
/>
|
||||
{trailing}
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isAudioTimelineElement, isMusicTrack, resolveBeatSourceTrack } from "./timelineInspector";
|
||||
import {
|
||||
canHideSelections,
|
||||
isAudioDomElement,
|
||||
isAudioTimelineElement,
|
||||
isMusicTrack,
|
||||
resolveBeatSourceTrack,
|
||||
} from "./timelineInspector";
|
||||
import type { TimelineElement } from "../player";
|
||||
|
||||
// Minimal element factory for tests
|
||||
@@ -119,3 +126,30 @@ describe("resolveBeatSourceTrack", () => {
|
||||
expect(result!.isFallback).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isAudioDomElement / canHideSelections", () => {
|
||||
const el = (tag: string, src?: string): Element => {
|
||||
const node = document.createElement(tag);
|
||||
if (src) node.setAttribute("src", src);
|
||||
return node;
|
||||
};
|
||||
|
||||
it("agrees with isAudioTimelineElement about tags and source extensions", () => {
|
||||
expect(isAudioDomElement(el("audio"))).toBe(true);
|
||||
expect(isAudioDomElement(el("div", "narration.mp3"))).toBe(true);
|
||||
expect(isAudioDomElement(el("div"))).toBe(false);
|
||||
expect(isAudioDomElement(null)).toBe(false);
|
||||
});
|
||||
|
||||
it("counts a group bus as audio, the way the single-selection panel does", () => {
|
||||
expect(isAudioDomElement(el("hf-audio-group"))).toBe(true);
|
||||
});
|
||||
|
||||
// `data-hidden` on audio is what mutes it — preview silences it and the render
|
||||
// drops it from the mix. A control labelled "Hide all" must not reach that.
|
||||
it("refuses to hide a selection holding any audio, and allows a layout one", () => {
|
||||
expect(canHideSelections([{ element: el("div") }, { element: el("span") }])).toBe(true);
|
||||
expect(canHideSelections([{ element: el("div") }, { element: el("audio") }])).toBe(false);
|
||||
expect(canHideSelections([{ element: el("hf-audio-group") }])).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { HF_AUDIO_GROUP_TAG } from "@hyperframes/core/audio-groups";
|
||||
import type { TimelineElement } from "../player";
|
||||
|
||||
const AUDIO_TIMELINE_TAGS = new Set(["audio", "music", "sfx", "sound", "narration"]);
|
||||
@@ -15,6 +16,9 @@ const MUSIC_ID_RE = /\b(music|bgm|soundtrack|background[-_]?music)\b/i;
|
||||
*/
|
||||
export function isAudioDomElement(node: Element | null | undefined): boolean {
|
||||
if (!node) return false;
|
||||
// A group bus counts: it is audio-only, and the panel's single-select path
|
||||
// already treats `<hf-audio-group>` as audio for exactly these decisions.
|
||||
if (node.tagName.toLowerCase() === HF_AUDIO_GROUP_TAG) return true;
|
||||
return isAudioTimelineElement({
|
||||
tag: node.tagName,
|
||||
src: node.getAttribute("src") ?? undefined,
|
||||
@@ -77,3 +81,20 @@ export function resolveBeatSourceTrack(
|
||||
}
|
||||
return best ? { element: best, isFallback: true } : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* May this multi-selection be hidden as one action?
|
||||
*
|
||||
* Audio has no visual to hide, and `data-hidden` on an audio element is what
|
||||
* MUTES it — preview silences it and the render drops it from the mix. The
|
||||
* timeline withholds the eye on an audio track for that reason
|
||||
* (`visible={!isAudioTrack}`), and the single-selection panel gates the same
|
||||
* write on `audioSelection`. The multi-selection "Hide all" was the one path
|
||||
* left back to it, on a control whose label promises visibility.
|
||||
*
|
||||
* A shared predicate rather than a check in the handler so the panel's button
|
||||
* and the handler's refusal cannot disagree — the button is not the only caller.
|
||||
*/
|
||||
export function canHideSelections(selections: readonly { element?: Element | null }[]): boolean {
|
||||
return !selections.some((selection) => isAudioDomElement(selection.element));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user