fix(studio): stop the grouping dialog opening off the bottom of the window (#3421)

Reported as "the grouping button did nothing — I clicked it and nothing
happened". The dialog WAS opening. It positioned itself at
`anchorRect.bottom + 4` with no flip and no clamp, and this button lives in a
track header at the bottom of the studio window, so it opened past the viewport
edge. It was the last floating surface in the timeline with no viewport handling
at all.

It now goes through `resolveFloatingPanelPosition`, the helper the other body
portals already position with (`RenderQueue`, `propertyPanelColor`), so it flips
above the anchor when there is no room below and clamps so neither edge leaves
the viewport. `GROUP_DIALOG_SIZE` is a declared estimate in the same style as
`FORMAT_PANEL_SIZE` and `COLOR_PICKER_SIZE`: `w-56` is exact, only the flip
decision reads the height, and the clamp keeps the dialog on screen either way.

Two tests, at a realistic bottom-of-window anchor and hard against the right
edge. Both verified to fail against the raw positioning.

Worth noting why this shipped: the existing `group-pointer` test passes with or
without the fix. happy-dom reports an all-zero rect for an unlaid-out button, so
the dialog landed at top:4 — on screen, and nothing like the real app. A geometry
test that never sets a geometry proves nothing.

Deliberately NOT included: a toast for the grouping write's silent
`elements.length < 2` bail. That path is real in code but I could not reach it
from the UI — the button only renders on a track with 2+ ungrouped clips, and
sub-composition audio arrives as separate single-clip rows, so the offer never
appears there. Adding a message for an unreachable branch, plus the file split it
would force to stay under the 600-line studio cap, is not justified by evidence.
This commit is contained in:
Vance Ingalls
2026-08-22 17:04:45 -07:00
committed by GitHub
parent f11b60854a
commit dd0626a55a
2 changed files with 55 additions and 1 deletions
@@ -68,6 +68,37 @@ describe("TimelineFxButton", () => {
expect(document.querySelector('[role="dialog"]')).toBeTruthy();
});
// The reported symptom was "the grouping button did nothing". The dialog WAS
// opening — it positioned at `anchorRect.bottom + 4` with no flip and no
// clamp, and this button lives in a track header at the bottom of the studio
// window, so it opened past the viewport edge. The test below it passed
// throughout: happy-dom reports an all-zero rect for an unlaid-out button,
// which lands the dialog at top:4 — on screen, and nothing like the app.
it("flips the group dialog above the anchor when there is no room below", () => {
const host = mount(<TimelineFxButton variant="group-pointer" onGroupClips={vi.fn()} />);
const fx = byTextButton(host, "FX");
// A track header near the bottom edge of the (1024x768) window.
fx!.getBoundingClientRect = () =>
({ left: 300, top: 760, right: 320, bottom: 776, width: 20, height: 16 }) as DOMRect;
act(() => fx?.click());
const dialog = document.querySelector('[role="dialog"]') as HTMLElement;
expect(dialog).toBeTruthy();
// Above the anchor, and fully inside the viewport: 644 + 112 === 756.
expect(dialog.style.top).toBe("644px");
});
it("keeps the group dialog inside the right edge of the window", () => {
const host = mount(<TimelineFxButton variant="group-pointer" onGroupClips={vi.fn()} />);
const fx = byTextButton(host, "FX");
// Hard against the right edge: 224px wide + a 12px margin has to fit.
fx!.getBoundingClientRect = () =>
({ left: 1010, top: 100, right: 1024, bottom: 116, width: 14, height: 16 }) as DOMRect;
act(() => fx?.click());
const dialog = document.querySelector('[role="dialog"]') as HTMLElement;
expect(dialog.style.left).toBe("788px");
expect(dialog.style.top).toBe("120px");
});
it("group-pointer variant offers Group instead of a popover", () => {
const onGroupClips = vi.fn();
const host = mount(<TimelineFxButton variant="group-pointer" onGroupClips={onGroupClips} />);
@@ -18,6 +18,24 @@ import {
} from "@hyperframes/core/audio-fx";
import type { HfAudioNameKind } from "@hyperframes/core/audio-carve";
import { TimelineFxPopover } from "../../components/editor/TimelineFxPopover.js";
import { resolveFloatingPanelPosition } from "../../components/editor/floatingPanel.js";
// Estimated, like FORMAT_PANEL_SIZE in RenderQueue: `w-56` is exact, and only
// the flip decision uses the height — the clamp keeps the dialog on screen
// either way.
const GROUP_DIALOG_SIZE = { width: 224, height: 112 };
/** Where the grouping dialog goes: flipped above the anchor when there is no
* room below, and clamped so neither edge leaves the viewport. */
function groupDialogPosition(anchorRect: DOMRect): { left: number; top: number } {
const { left, top } = resolveFloatingPanelPosition(
anchorRect,
{ width: window.innerWidth, height: window.innerHeight },
GROUP_DIALOG_SIZE,
{ offset: 4 },
);
return { left, top };
}
function parseFxChainOrEmpty(raw: string | undefined): HfAudioFxChain {
if (!raw) return { version: 1, nodes: [] };
@@ -80,7 +98,12 @@ export function TimelineFxButton(props: TimelineFxButtonProps) {
role="dialog"
aria-label="Group these clips to add effects"
className="z-[200] w-56 rounded-md border border-white/10 bg-[#1b1b1f] p-2.5 text-[11px] text-white/75 shadow-xl"
style={{ position: "fixed", left: anchorRect.left, top: anchorRect.bottom + 4 }}
// Was `top: anchorRect.bottom + 4` with no flip and no clamp. This
// button lives in a track header at the BOTTOM of the studio
// window, so the dialog opened past the viewport edge and the
// click read as doing nothing at all. Same helper the other body
// portals position with.
style={{ position: "fixed", ...groupDialogPosition(anchorRect) }}
onPointerDown={(event) => event.stopPropagation()}
>
<p>Group these clips to add effects to all of them.</p>