From 594368dbd5220e13ba9eb9478c35330bce4f35e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 30 Sep 2026 04:05:45 -0400 Subject: [PATCH 1/2] fix(studio): a click after an empty-canvas press selects on the first try --- .../src/components/editor/DomEditOverlay.tsx | 28 +----- .../editor/DomEditSelectionChrome.test.tsx | 5 - .../editor/DomEditSelectionChrome.tsx | 6 -- .../editor/clickAfterPreventedPress.test.tsx | 97 +++++++++++++++++++ .../editor/shiftDragAxisLock.test.tsx | 2 - 5 files changed, 98 insertions(+), 40 deletions(-) create mode 100644 packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 5940364d81..c4f13f83a5 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -161,8 +161,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const groupGestureRef = useRef(null); const blockedMoveRef = useRef(null); const suppressNextBoxClickRef = useRef(false); - const suppressNextBoxMouseDownRef = useRef(false); - const suppressNextOverlayMouseDownRef = useRef(false); const snapGuidesRef = useRef(null); const rafPausedRef = useRef(false); @@ -329,15 +327,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const handleOverlayMouseDown = (event: React.MouseEvent) => { if (!allowCanvasMovement) return; - if (suppressNextOverlayMouseDownRef.current) { - logSelect("mousedown-suppressed", { shift: event.shiftKey }); - suppressNextOverlayMouseDownRef.current = false; - suppressNextBoxMouseDownRef.current = false; - suppressNextBoxClickRef.current = false; - event.preventDefault(); - event.stopPropagation(); - return; - } const target = event.target as HTMLElement | null; const onBox = Boolean(target?.closest('[data-dom-edit-selection-box="true"]')); logSelect("mousedown", { shift: event.shiftKey, onBox }); @@ -346,10 +335,7 @@ export const DomEditOverlay = memo(function DomEditOverlay({ // extend beyond the composition rect into the gray zone, and users need // to select/deselect them by clicking there. onCanvasMouseDown(event, { hoverSelection: hoverSelectionRef.current }); - if (event.shiftKey) { - suppressNextBoxMouseDownRef.current = true; - suppressNextBoxClickRef.current = true; - } + if (event.shiftKey) suppressNextBoxClickRef.current = true; }; // fallow-ignore-next-line complexity @@ -373,8 +359,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ if (!candidate) return; event.preventDefault(); event.stopPropagation(); - suppressNextOverlayMouseDownRef.current = true; - suppressNextBoxMouseDownRef.current = true; suppressNextBoxClickRef.current = true; onSelectionChangeRef.current(candidate, { additive: true }); return; @@ -419,7 +403,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ // so those elements were always selectable once the band could begin. event.preventDefault(); event.stopPropagation(); - suppressNextOverlayMouseDownRef.current = true; marquee.begin(event); return; } @@ -437,13 +420,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ onCanvasMouseDown(event, { hoverSelection: hoverSelectionRef.current }); }; - const suppressBoxMouseDown = (e: React.MouseEvent) => { - if (!suppressNextBoxMouseDownRef.current) return; - suppressNextBoxMouseDownRef.current = false; - e.preventDefault(); - e.stopPropagation(); - }; - // Right-click state + handler: select the element under the pointer (if // needed), then open the menu; closes when the selection moves off-target. const { contextMenu, closeContextMenu, handleContextMenu } = useCanvasContextMenuState({ @@ -509,7 +485,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ allowBodyDrag={bodyDrag} groupCanMove={groupCanMove} gestures={gestures} - onBoxMouseDown={suppressBoxMouseDown} onBoxClick={handleBoxClick} /> )} @@ -528,7 +503,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ groupSelectionCount={groupSelections.length} gestures={gestures} onStyleCommit={onStyleCommitRef.current} - onBoxMouseDown={suppressBoxMouseDown} onBoxClick={handleBoxClick} /> )} diff --git a/packages/studio/src/components/editor/DomEditSelectionChrome.test.tsx b/packages/studio/src/components/editor/DomEditSelectionChrome.test.tsx index 5047b3508f..1cb88bffbd 100644 --- a/packages/studio/src/components/editor/DomEditSelectionChrome.test.tsx +++ b/packages/studio/src/components/editor/DomEditSelectionChrome.test.tsx @@ -56,7 +56,6 @@ describe("DomEditSelectionChrome crop composition", () => { groupSelectionCount={0} gestures={{ startGesture: vi.fn() } as never} onStyleCommit={vi.fn()} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} />, ); @@ -112,7 +111,6 @@ describe("DomEditSelectionChrome crop composition", () => { groupSelectionCount={0} gestures={{ startGesture: vi.fn() } as never} onStyleCommit={vi.fn()} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} />, ); @@ -173,7 +171,6 @@ describe("DomEditSelectionChrome while editing text", () => { groupSelectionCount={0} gestures={{ startGesture: vi.fn() } as never} onStyleCommit={vi.fn()} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} inlineText={{ editing, startFromPress: vi.fn() }} />, @@ -258,7 +255,6 @@ describe("DomEditSelectionChrome with body drag off", () => { selectionKey="box" groupSelectionCount={0} gestures={gestures as never} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} />, ); @@ -313,7 +309,6 @@ describe("DomEditSelectionChrome with body drag off", () => { allowBodyDrag={false} groupCanMove gestures={gestures as never} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} />, ); diff --git a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx index 3733b8d42b..e07c010755 100644 --- a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx +++ b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx @@ -62,7 +62,6 @@ interface DomEditGroupChromeProps { allowBodyDrag: boolean; groupCanMove: boolean; gestures: GestureHandlers; - onBoxMouseDown: (e: React.MouseEvent) => void; onBoxClick: (event: React.MouseEvent) => void; } @@ -75,7 +74,6 @@ export function DomEditGroupChrome({ allowBodyDrag, groupCanMove, gestures, - onBoxMouseDown, onBoxClick, }: DomEditGroupChromeProps) { const canManipulate = allowCanvasMovement && !usePreviewReadOnly(); @@ -108,7 +106,6 @@ export function DomEditGroupChrome({ if (!canManipulate || !allowBodyDrag || (e.shiftKey && !groupCanMove)) return; gestures.startGroupDrag(e); }} - onMouseDown={onBoxMouseDown} onClick={onBoxClick} /> @@ -128,7 +125,6 @@ interface DomEditSelectionChromeProps { groupSelectionCount: number; gestures: GestureHandlers; onStyleCommit?: (property: string, value: string) => Promise | void; - onBoxMouseDown: (e: React.MouseEvent) => void; onBoxClick: (event: React.MouseEvent) => void; /** The canvas' text-editing session: what opens one, and whether one is open. */ inlineText?: { @@ -157,7 +153,6 @@ export function DomEditSelectionChrome({ groupSelectionCount, gestures, onStyleCommit, - onBoxMouseDown, onBoxClick, inlineText, }: DomEditSelectionChromeProps) { @@ -225,7 +220,6 @@ export function DomEditSelectionChrome({ } if (!e.shiftKey) gestures.startBlockedMove(e, selection); }} - onMouseDown={onBoxMouseDown} onClick={onBoxClick} > {cropOutlineInsetPx && ( diff --git a/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx b/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx new file mode 100644 index 0000000000..3f3a7b0147 --- /dev/null +++ b/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx @@ -0,0 +1,97 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, expect, it, vi } from "vitest"; +import { makeSelection } from "../../hooks/domSelectionTestHarness"; +import type { DomEditSelection } from "./domEditing"; +import "./domEditOverlayTestMocks"; +import { DomEditOverlay } from "./DomEditOverlay"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +HTMLElement.prototype.setPointerCapture ??= () => {}; +HTMLElement.prototype.releasePointerCapture ??= () => {}; + +const pointTarget = vi.hoisted(() => ({ current: null as HTMLElement | null })); +vi.mock("../../utils/studioPreviewHelpers", async (importOriginal) => ({ + ...(await importOriginal()), + getPreviewTargetFromPointer: () => pointTarget.current, +})); +vi.mock("./useDomEditOverlayRects", () => ({ + useDomEditOverlayRects: () => ({ + overlayRect: null, + overlayRectRef: { current: null }, + setOverlayRect: () => undefined, + hoverRect: null, + groupOverlayItems: [], + groupOverlayItemsRef: { current: [] }, + setGroupOverlayItems: () => undefined, + childRects: [], + }), +})); + +let root: Root; +const onCanvasMouseDown = vi.fn(); + +function render(hoverSelection: DomEditSelection | null) { + act(() => + root.render( + Promise.resolve(null)} + onCanvasPointerLeave={() => undefined} + onSelectionChange={() => undefined} + onBlockedMove={() => undefined} + onPathOffsetCommit={() => undefined} + onGroupPathOffsetCommit={() => undefined} + onBoxSizeCommit={() => undefined} + onRotationCommit={() => undefined} + onMarqueeSelect={() => undefined} + />, + ), + ); +} + +// Like Chrome: a default-prevented pointerdown sends no compatibility mousedown. +function press(target: Element, shiftKey = false) { + const init = { bubbles: true, cancelable: true, button: 0, pointerId: 1, shiftKey }; + const down = new PointerEvent("pointerdown", init); + act(() => void target.dispatchEvent(down)); + if (!down.defaultPrevented) + act(() => void target.dispatchEvent(new MouseEvent("mousedown", init))); + act(() => void target.dispatchEvent(new PointerEvent("pointerup", init))); +} + +afterEach(() => { + act(() => root.unmount()); + onCanvasMouseDown.mockClear(); + pointTarget.current = null; + document.body.innerHTML = ""; +}); + +it.each([ + ["a press on empty canvas", false], + ["a shift+click add", true], +])("after %s, the next click on an element selects it", (_, shiftFirst) => { + const host = document.createElement("div"); + document.body.append(host); + root = createRoot(host); + const element = document.createElement("h1"); + document.body.append(element); + const hover = makeSelection("Title", element); + const overlay = () => host.firstElementChild!; + + pointTarget.current = shiftFirst ? element : null; + render(shiftFirst ? hover : null); + press(overlay(), shiftFirst); + expect(onCanvasMouseDown).not.toHaveBeenCalled(); + + pointTarget.current = element; + render(hover); + press(overlay()); + expect(onCanvasMouseDown).toHaveBeenCalledTimes(1); +}); diff --git a/packages/studio/src/components/editor/shiftDragAxisLock.test.tsx b/packages/studio/src/components/editor/shiftDragAxisLock.test.tsx index f1411c7674..55588abe28 100644 --- a/packages/studio/src/components/editor/shiftDragAxisLock.test.tsx +++ b/packages/studio/src/components/editor/shiftDragAxisLock.test.tsx @@ -193,7 +193,6 @@ describe("a shift press on a selected box starts the drag", () => { selectionKey="box" groupSelectionCount={0} gestures={spies as never} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} /> )); @@ -207,7 +206,6 @@ describe("a shift press on a selected box starts the drag", () => { allowBodyDrag groupCanMove={groupCanMove} gestures={spies as never} - onBoxMouseDown={vi.fn()} onBoxClick={vi.fn()} /> )); From 374fc7c56589ac8fe574663c2d221ad7c0a5b75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 30 Sep 2026 08:54:50 -0400 Subject: [PATCH 2/2] fix(studio): a shift press outside the box no longer sets a click guard that never fires --- .../src/components/editor/DomEditOverlay.tsx | 1 - .../editor/clickAfterPreventedPress.test.tsx | 47 ++++++++++++++----- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index c4f13f83a5..4715783fa5 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -335,7 +335,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({ // extend beyond the composition rect into the gray zone, and users need // to select/deselect them by clicking there. onCanvasMouseDown(event, { hoverSelection: hoverSelectionRef.current }); - if (event.shiftKey) suppressNextBoxClickRef.current = true; }; // fallow-ignore-next-line complexity diff --git a/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx b/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx index 3f3a7b0147..aec0f1bd32 100644 --- a/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx +++ b/packages/studio/src/components/editor/clickAfterPreventedPress.test.tsx @@ -13,14 +13,15 @@ HTMLElement.prototype.setPointerCapture ??= () => {}; HTMLElement.prototype.releasePointerCapture ??= () => {}; const pointTarget = vi.hoisted(() => ({ current: null as HTMLElement | null })); +const rect = vi.hoisted(() => ({ current: null as Record | null })); vi.mock("../../utils/studioPreviewHelpers", async (importOriginal) => ({ ...(await importOriginal()), getPreviewTargetFromPointer: () => pointTarget.current, })); vi.mock("./useDomEditOverlayRects", () => ({ useDomEditOverlayRects: () => ({ - overlayRect: null, - overlayRectRef: { current: null }, + overlayRect: rect.current, + overlayRectRef: rect, setOverlayRect: () => undefined, hoverRect: null, groupOverlayItems: [], @@ -32,19 +33,32 @@ vi.mock("./useDomEditOverlayRects", () => ({ let root: Root; const onCanvasMouseDown = vi.fn(); +const onSelectionChange = vi.fn(); -function render(hoverSelection: DomEditSelection | null) { +function mount() { + const host = document.createElement("div"); + document.body.append(host); + root = createRoot(host); + const element = document.createElement("h1"); + document.body.append(element); + return { host, element, hover: makeSelection("Title", element) }; +} + +function render( + hoverSelection: DomEditSelection | null, + selection: DomEditSelection | null = null, +) { act(() => root.render( Promise.resolve(null)} onCanvasPointerLeave={() => undefined} - onSelectionChange={() => undefined} + onSelectionChange={onSelectionChange} onBlockedMove={() => undefined} onPathOffsetCommit={() => undefined} onGroupPathOffsetCommit={() => undefined} @@ -64,12 +78,15 @@ function press(target: Element, shiftKey = false) { if (!down.defaultPrevented) act(() => void target.dispatchEvent(new MouseEvent("mousedown", init))); act(() => void target.dispatchEvent(new PointerEvent("pointerup", init))); + act(() => void target.dispatchEvent(new MouseEvent("click", init))); } afterEach(() => { act(() => root.unmount()); onCanvasMouseDown.mockClear(); + onSelectionChange.mockClear(); pointTarget.current = null; + rect.current = null; document.body.innerHTML = ""; }); @@ -77,12 +94,7 @@ it.each([ ["a press on empty canvas", false], ["a shift+click add", true], ])("after %s, the next click on an element selects it", (_, shiftFirst) => { - const host = document.createElement("div"); - document.body.append(host); - root = createRoot(host); - const element = document.createElement("h1"); - document.body.append(element); - const hover = makeSelection("Title", element); + const { host, element, hover } = mount(); const overlay = () => host.firstElementChild!; pointTarget.current = shiftFirst ? element : null; @@ -95,3 +107,16 @@ it.each([ press(overlay()); expect(onCanvasMouseDown).toHaveBeenCalledTimes(1); }); + +it("a shift+click on a selected box that cannot move toggles it once", () => { + const { host, element, hover } = mount(); + hover.capabilities.canApplyManualOffset = false; + rect.current = { left: 20, top: 30, width: 100, height: 40, editScaleX: 1, editScaleY: 1 }; + pointTarget.current = element; + render(hover, hover); + + press(host.querySelector('[data-dom-edit-selection-box="true"]')!, true); + + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(onCanvasMouseDown).not.toHaveBeenCalled(); +});