mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-04 16:42:27 +00:00
fix(studio): don't offer layout grouping for a selection of audio clips
Selecting two audio clips offered "Group selection", which is the layout grouper: it wraps the members in a positioned <div> at their bounding box, rebases each child's left/top against that origin, and adopts the topmost member's z-index. None of that means anything for audio. An <audio> clip has no box — offsetWidth/Height are 0 — so the wrapper came out `width: 0px; height: 0px` with inline left/top written onto elements that are never laid out, and the composition gained a <div> standing for nothing audible. Confirmed against `wrapElementsInHtml` directly: it matched and wrapped, producing exactly that. Refused in `handleGroupSelection`, which is where the G shortcut also lands — a hidden button cannot gate a keystroke. The panel withholds the button as well, so the refusal is not the first the author hears of it. The message names the alternative rather than only declining: audio's answer to "these clips belong together" is an <hf-audio-group> bus, which the timeline's own FX pointer already creates. A mixed selection is refused too — the wrapper would take the audio in with the rest. `isAudioDomElement` sits beside `isAudioTimelineElement` and delegates to it, so the selection layer and the timeline cannot drift into disagreeing about what counts as audio. Six tests across both entry points, each mutation-checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a387850032
commit
a05f16c5e6
@@ -71,4 +71,58 @@ describe("PropertyPanelEmptyState — flat multi-select", () => {
|
||||
expect(onClearSelection).toHaveBeenCalledTimes(1);
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
// A layout group is a positioned wrapper around a bounding box; an <audio>
|
||||
// clip has none (offsetWidth/Height are 0), so grouping audio produced a 0x0
|
||||
// div with inline left/top on elements that are never laid out. Withheld
|
||||
// rather than offered-then-refused.
|
||||
const audioElements = (tags: string[]) =>
|
||||
tags.map((tag, i) => ({
|
||||
id: `el-${i}`,
|
||||
selector: `#el-${i}`,
|
||||
label: `El ${i}`,
|
||||
tagName: tag,
|
||||
element: document.createElement(tag),
|
||||
})) as unknown as DomEditSelection[];
|
||||
|
||||
it("withholds Group selection when the selection includes audio", () => {
|
||||
const { host, root } = renderInto(
|
||||
<PropertyPanelEmptyState
|
||||
flat
|
||||
multiSelectCount={2}
|
||||
multiSelectedElements={audioElements(["audio", "audio"])}
|
||||
onGroupSelection={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();
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
it("withholds it for a mixed selection too, since the wrapper would still take audio in", () => {
|
||||
const { host, root } = renderInto(
|
||||
<PropertyPanelEmptyState
|
||||
flat
|
||||
multiSelectCount={2}
|
||||
multiSelectedElements={audioElements(["div", "audio"])}
|
||||
onGroupSelection={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(host.querySelector('[data-flat-multiselect-group="true"]')).toBeNull();
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
it("still offers it for a selection of layout elements", () => {
|
||||
const { host, root } = renderInto(
|
||||
<PropertyPanelEmptyState
|
||||
flat
|
||||
multiSelectCount={2}
|
||||
multiSelectedElements={audioElements(["div", "span"])}
|
||||
onGroupSelection={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(host.querySelector('[data-flat-multiselect-group="true"]')).not.toBeNull();
|
||||
act(() => root.unmount());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Eye, Layers } from "../../icons/SystemIcons";
|
||||
import type { DomEditSelection } from "./domEditingTypes";
|
||||
import { isAudioDomElement } from "../../utils/timelineInspector";
|
||||
|
||||
function FlatEmptyState() {
|
||||
return (
|
||||
@@ -67,6 +68,7 @@ function FlatMultiSelectState({
|
||||
onHideAllSelected?: () => void;
|
||||
onClearSelection?: () => void;
|
||||
}) {
|
||||
const hasAudio = multiSelectedElements.some((el) => isAudioDomElement(el.element));
|
||||
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">
|
||||
@@ -124,20 +126,30 @@ function FlatMultiSelectState({
|
||||
})}
|
||||
</div>
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
data-flat-multiselect-group="true"
|
||||
onClick={onGroupSelection}
|
||||
className="flex h-[34px] flex-1 items-center justify-center gap-2 rounded-lg bg-panel-hover text-[11px] font-semibold text-panel-text-0"
|
||||
>
|
||||
<Layers size={13} />
|
||||
Group selection
|
||||
</button>
|
||||
{/* 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 && (
|
||||
<button
|
||||
type="button"
|
||||
data-flat-multiselect-group="true"
|
||||
onClick={onGroupSelection}
|
||||
className="flex h-[34px] flex-1 items-center justify-center gap-2 rounded-lg bg-panel-hover text-[11px] font-semibold text-panel-text-0"
|
||||
>
|
||||
<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"
|
||||
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
|
||||
|
||||
@@ -58,6 +58,8 @@ const capturedOnReorderShadow: { fn: ((targets: string[]) => void) | undefined }
|
||||
fn: undefined,
|
||||
};
|
||||
const domEditSelectionRef: { current: DomEditSelection | null } = { current: null };
|
||||
const domEditGroupSelectionsRef: { current: DomEditSelection[] } = { current: [] };
|
||||
const groupSelectionSpy = vi.fn();
|
||||
const gsapCommitMutation = Object.assign(vi.fn(), { batch: vi.fn() });
|
||||
|
||||
function createSessionParams(
|
||||
@@ -131,7 +133,7 @@ vi.mock("./useDomSelection", () => ({
|
||||
domEditHoverSelection: null,
|
||||
activeGroupElement: null,
|
||||
domEditSelectionRef,
|
||||
domEditGroupSelectionsRef: { current: [] },
|
||||
domEditGroupSelectionsRef,
|
||||
setActiveGroupElement: vi.fn(),
|
||||
applyDomSelection: vi.fn(),
|
||||
clearDomSelection: vi.fn(),
|
||||
@@ -190,7 +192,7 @@ vi.mock("./useGsapScriptCommits", () => ({
|
||||
}));
|
||||
vi.mock("./useGroupCommits", () => ({
|
||||
useGroupCommits: () => ({
|
||||
groupSelection: vi.fn(),
|
||||
groupSelection: (...args: unknown[]) => groupSelectionSpy(...args),
|
||||
ungroupSelection: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
@@ -407,3 +409,56 @@ describe("bulk segment ease commits", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── Grouping refuses audio ───────────────────────────────────────────────────
|
||||
//
|
||||
// A layout group is a positioned wrapper: it takes the members' bounding box,
|
||||
// rebases each child's left/top against it and adopts the topmost z-index. An
|
||||
// <audio> clip has no box — offsetWidth/Height are 0 — so this produced a 0x0
|
||||
// div with inline left/top on elements that are never laid out. Enforced here
|
||||
// rather than only by hiding the button, because the G shortcut routes through
|
||||
// the same handler and no hidden button can gate a keystroke.
|
||||
|
||||
describe("handleGroupSelection with audio in the selection", () => {
|
||||
const sel = (tag: string): DomEditSelection =>
|
||||
({
|
||||
id: tag,
|
||||
element: document.createElement(tag),
|
||||
sourceFile: "index.html",
|
||||
}) as unknown as DomEditSelection;
|
||||
|
||||
async function group(members: DomEditSelection[]) {
|
||||
const { useDomEditSession } = await import("./useDomEditSession");
|
||||
groupSelectionSpy.mockClear();
|
||||
domEditGroupSelectionsRef.current = members;
|
||||
const showToast = vi.fn();
|
||||
const captured: { fn?: () => void } = {};
|
||||
function Probe() {
|
||||
captured.fn = useDomEditSession(createSessionParams({ showToast })).handleGroupSelection;
|
||||
return null;
|
||||
}
|
||||
const root = createRoot(document.createElement("div"));
|
||||
act(() => root.render(<Probe />));
|
||||
act(() => captured.fn?.());
|
||||
act(() => root.unmount());
|
||||
domEditGroupSelectionsRef.current = [];
|
||||
return { showToast };
|
||||
}
|
||||
|
||||
it("refuses a selection of audio clips, and says where grouping audio lives", async () => {
|
||||
const { showToast } = await group([sel("audio"), sel("audio")]);
|
||||
expect(groupSelectionSpy).not.toHaveBeenCalled();
|
||||
expect(String(showToast.mock.calls[0]?.[0])).toContain("bus");
|
||||
});
|
||||
|
||||
it("refuses a mixed selection, since the wrapper would take the audio in too", async () => {
|
||||
const { showToast } = await group([sel("div"), sel("audio")]);
|
||||
expect(groupSelectionSpy).not.toHaveBeenCalled();
|
||||
expect(String(showToast.mock.calls[0]?.[0])).toContain("layout");
|
||||
});
|
||||
|
||||
it("still groups a selection of layout elements", async () => {
|
||||
await group([sel("div"), sel("span")]);
|
||||
expect(groupSelectionSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useCallback } from "react";
|
||||
import { trackStudioEvent } from "../utils/studioTelemetry";
|
||||
import { isAudioDomElement } from "../utils/timelineInspector";
|
||||
import type { SelectElementOptions, TimelineElement } from "../player";
|
||||
import type { ImportedFontAsset } from "../components/editor/fontAssets";
|
||||
import type { EditHistoryKind } from "../utils/editHistory";
|
||||
@@ -374,6 +375,23 @@ export function useDomEditSession({
|
||||
showToast("Select at least 2 elements to group", "info");
|
||||
return;
|
||||
}
|
||||
// A layout group is a positioned wrapper: it takes the members' bounding
|
||||
// box, rebases each child's left/top against it, and adopts the topmost
|
||||
// z-index. An <audio> clip has no box — offsetWidth/Height are 0 — so
|
||||
// grouping audio produced a 0x0 div with inline left/top written onto
|
||||
// elements that have never been laid out, and the timeline gained a
|
||||
// wrapper standing for nothing audible. The audio answer to "these clips
|
||||
// belong together" is an <hf-audio-group> bus, which the timeline's own FX
|
||||
// pointer creates, so the refusal names it rather than just declining.
|
||||
if (members.some((m) => isAudioDomElement(m.element))) {
|
||||
showToast(
|
||||
members.every((m) => isAudioDomElement(m.element))
|
||||
? "Audio clips group into a bus — use FX on the track header"
|
||||
: "Can't group audio clips with layout elements",
|
||||
"info",
|
||||
);
|
||||
return;
|
||||
}
|
||||
trackStudioEvent("group", { action: "create", count: members.length });
|
||||
void groupSelection(members);
|
||||
}, [domEditGroupSelectionsRef, domEditSelectionRef, groupSelection, showToast]);
|
||||
|
||||
@@ -4,6 +4,23 @@ const AUDIO_TIMELINE_TAGS = new Set(["audio", "music", "sfx", "sound", "narratio
|
||||
const AUDIO_SOURCE_EXT_RE = /\.(aac|flac|m4a|mp3|ogg|opus|wav)(?:[?#].*)?$/i;
|
||||
const MUSIC_ID_RE = /\b(music|bgm|soundtrack|background[-_]?music)\b/i;
|
||||
|
||||
/**
|
||||
* Is this DOM node an audio clip, judged the way `isAudioTimelineElement`
|
||||
* judges a timeline element?
|
||||
*
|
||||
* The selection layer holds real elements rather than timeline records, and
|
||||
* layout grouping is decided there — so it needs the same question asked of a
|
||||
* node. Same tag set and same source-extension fallback, so the two cannot
|
||||
* drift into disagreeing about what counts as audio.
|
||||
*/
|
||||
export function isAudioDomElement(node: Element | null | undefined): boolean {
|
||||
if (!node) return false;
|
||||
return isAudioTimelineElement({
|
||||
tag: node.tagName,
|
||||
src: node.getAttribute("src") ?? undefined,
|
||||
});
|
||||
}
|
||||
|
||||
export function isAudioTimelineElement(
|
||||
element: Pick<TimelineElement, "tag" | "src"> | null | undefined,
|
||||
): boolean {
|
||||
|
||||
Reference in New Issue
Block a user