From 7e20114a1cc4b0eaa8d8d92818b3cb71862bccc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 30 Sep 2026 01:46:00 -0400 Subject: [PATCH 1/2] fix(studio): shift+click on a group member removes it for good Removing a member with shift+click shrank the canvas group, but the timeline kept the old set, and the timeline syncs its set back onto the canvas, so the member came back about 70 ms later. A one-element marquee over a live group had the same fault. The canvas now publishes the new set whenever the caller states the whole selection (a shift toggle or a marquee), while a late single primary inside the live set still keeps it. --- .../src/hooks/domSelectionTimelineMirror.ts | 9 +- .../studio/src/hooks/useDomSelection.test.ts | 85 ++++++++++++------- packages/studio/src/hooks/useDomSelection.ts | 7 +- 3 files changed, 62 insertions(+), 39 deletions(-) diff --git a/packages/studio/src/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts index 27dcdc2b15..b6171986d8 100644 --- a/packages/studio/src/hooks/domSelectionTimelineMirror.ts +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -28,6 +28,7 @@ export function announceTimelineSelection( deps: TimelineMirrorDeps, group: DomEditSelection[], primary: DomEditSelection | null, + replaceSet = false, ): void { const { timelineElements, @@ -72,10 +73,10 @@ export function announceTimelineSelection( } return; } - // A late async primary that already belongs to the live set must preserve the - // group. A fresh single click does not belong to it, so publish the singleton - // first; otherwise `preserveSet` clears the set and sync wipes the canvas. - if (group.length > 1 || !getTimelineSelectionSet().has(timelineAnchor)) { + // A late async primary inside the live set keeps the group. A click outside it, or a + // group the caller states in full (`replaceSet`: shift toggle, marquee), is published + // first; otherwise `preserveSet` clears or keeps a stale set and sync undoes the canvas. + if (replaceSet || group.length > 1 || !getTimelineSelectionSet().has(timelineAnchor)) { setTimelineSelectionSet(publishedMembers); } setSelectedTimelineElementId(timelineAnchor, { preserveSet: true }); diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index dda61c7e97..fd8c440202 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -29,9 +29,13 @@ function renderHarness( cleanup: () => void; timeline: TimelineSpies; } { + // Reads back what was last published, as the timeline store does. + let publishedSet: ReadonlySet = new Set(); const timeline: TimelineSpies = { setSelectedTimelineElementId: vi.fn(), - setTimelineSelectionSet: vi.fn(), + setTimelineSelectionSet: vi.fn((ids: Set) => { + publishedSet = ids; + }), }; const host = document.createElement("div"); document.body.append(host); @@ -47,7 +51,7 @@ function renderHarness( captionEditMode: false, previewIframeRef: { current: null }, timelineElements: options.timelineElements ?? [], - getTimelineSelectionSet: () => new Set(), + getTimelineSelectionSet: () => publishedSet, setSelectedTimelineElementId: timeline.setSelectedTimelineElementId, setTimelineSelectionSet: timeline.setTimelineSelectionSet, setRightCollapsed: vi.fn(), @@ -107,6 +111,17 @@ function timelineElement(domId: string): TimelineElement { } as TimelineElement; } +function renderCardAndChip() { + const card = makeSelection("Card", Object.assign(document.createElement("div"), { id: "card" })); + const chip = makeSelection("Chip", Object.assign(document.createElement("div"), { id: "chip" })); + document.body.append(card.element, chip.element); + const harness = renderHarness( + { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, + { timelineElements: [timelineElement("card"), timelineElement("chip")] }, + ); + return { harness, card, chip }; +} + /** * A marquee builds the group correctly and then used to lose it: it announced only * the primary to the timeline, the timeline is the source of truth for what is @@ -116,24 +131,9 @@ function timelineElement(domId: string): TimelineElement { */ describe("useDomSelection marquee", () => { it("announces every marquee'd element to the timeline, anchored on the primary", () => { - const first = document.createElement("div"); - first.id = "card"; - const second = document.createElement("div"); - second.id = "chip"; - document.body.append(first, second); - const harness = renderHarness( - { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, - { timelineElements: [timelineElement("card"), timelineElement("chip")] }, - ); + const { harness, card, chip } = renderCardAndChip(); - act(() => - harness - .current() - .applyMarqueeSelection( - [makeSelection("Card", first), makeSelection("Chip", second)], - false, - ), - ); + act(() => harness.current().applyMarqueeSelection([card, chip], false)); expect(harness.current().domEditGroupSelections).toHaveLength(2); expect(harness.timeline.setTimelineSelectionSet).toHaveBeenCalledWith( @@ -181,20 +181,10 @@ describe("useDomSelection marquee", () => { */ describe("useDomSelection additive", () => { it("announces both members when a second element joins the selection", () => { - const first = document.createElement("div"); - first.id = "card"; - const second = document.createElement("div"); - second.id = "chip"; - document.body.append(first, second); - const harness = renderHarness( - { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, - { timelineElements: [timelineElement("card"), timelineElement("chip")] }, - ); + const { harness, card, chip } = renderCardAndChip(); - act(() => harness.current().applyDomSelection(makeSelection("Card", first))); - act(() => - harness.current().applyDomSelection(makeSelection("Chip", second), { additive: true }), - ); + act(() => harness.current().applyDomSelection(card)); + act(() => harness.current().applyDomSelection(chip, { additive: true })); expect(harness.current().domEditGroupSelections).toHaveLength(2); expect(harness.timeline.setTimelineSelectionSet).toHaveBeenLastCalledWith( @@ -205,6 +195,37 @@ describe("useDomSelection additive", () => { }); harness.cleanup(); }); + + // The timeline syncs its set back onto the canvas, so a set left at two re-adds the member. + it.each([ + ["shift+click removes a member", "toggle"], + ["a marquee catches one member of a live group", "marquee"], + ] as const)("publishes the smaller set when %s", (_, shrink) => { + const { harness, card, chip } = renderCardAndChip(); + + act(() => harness.current().applyMarqueeSelection([card, chip], false)); + act(() => + shrink === "toggle" + ? harness.current().applyDomSelection(chip, { additive: true }) + : harness.current().applyMarqueeSelection([card], false), + ); + + expect(harness.current().domEditGroupSelections).toHaveLength(1); + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenLastCalledWith(new Set(["card"])); + harness.cleanup(); + }); + + it("keeps the live set when a single member is re-announced", () => { + const { harness, card, chip } = renderCardAndChip(); + + act(() => harness.current().applyMarqueeSelection([card, chip], false)); + act(() => harness.current().applyDomSelection(chip)); + + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenLastCalledWith( + new Set(["card", "chip"]), + ); + harness.cleanup(); + }); }); describe("useDomSelection", () => { diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 76ca4bca94..4802a6937e 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -83,7 +83,7 @@ export function useDomSelection({ // ── Callbacks ── const announceTimelineSelection = useCallback( - (group: DomEditSelection[], primary: DomEditSelection | null) => + (group: DomEditSelection[], primary: DomEditSelection | null, replaceSet?: boolean) => announceSelectionToTimeline( { timelineElements, @@ -93,6 +93,7 @@ export function useDomSelection({ }, group, primary, + replaceSet, ), [ getTimelineSelectionSet, @@ -179,7 +180,7 @@ export function useDomSelection({ setRightPanelTab("design"); } } - announceTimelineSelection(nextGroup, nextSelection); + announceTimelineSelection(nextGroup, nextSelection, isAdditiveSelection); return; } @@ -510,7 +511,7 @@ export function useDomSelection({ domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - announceTimelineSelection(nextGroup, nextSelection); + announceTimelineSelection(nextGroup, nextSelection, true); }, [applyDomSelection, announceTimelineSelection], ); From c75cb1f8f88aeb4d244f2093b44d78a9151a8c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 30 Sep 2026 02:52:42 -0400 Subject: [PATCH 2/2] test(studio): a shrinking selection keeps the remaining member as the anchor --- packages/studio/src/hooks/domSelectionTimelineMirror.ts | 2 +- packages/studio/src/hooks/useDomSelection.test.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/studio/src/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts index b6171986d8..2531f441bb 100644 --- a/packages/studio/src/hooks/domSelectionTimelineMirror.ts +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -74,7 +74,7 @@ export function announceTimelineSelection( return; } // A late async primary inside the live set keeps the group. A click outside it, or a - // group the caller states in full (`replaceSet`: shift toggle, marquee), is published + // group the caller states in full (`replaceSet`: shift toggle, marquee, timeline sync), is published // first; otherwise `preserveSet` clears or keeps a stale set and sync undoes the canvas. if (replaceSet || group.length > 1 || !getTimelineSelectionSet().has(timelineAnchor)) { setTimelineSelectionSet(publishedMembers); diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index fd8c440202..43047c25f6 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -212,6 +212,9 @@ describe("useDomSelection additive", () => { expect(harness.current().domEditGroupSelections).toHaveLength(1); expect(harness.timeline.setTimelineSelectionSet).toHaveBeenLastCalledWith(new Set(["card"])); + expect(harness.timeline.setSelectedTimelineElementId).toHaveBeenLastCalledWith("card", { + preserveSet: true, + }); harness.cleanup(); });