From 6c36fc594149324bf524d4493311f2aa33deb5da Mon Sep 17 00:00:00 2001 From: ents1008 <73383417+ents1008@users.noreply.github.com> Date: Tue, 18 Nov 2025 14:26:27 +0800 Subject: [PATCH] fix(editor): switch to PanTool on same frame for middle mouse; restore selection snapshot (#13911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: In Edgeless mode, pressing and dragging the middle mouse button over any element incorrectly triggers DefaultTool in the same frame, causing unintended selection/drag instead of panning. Dragging on empty area works because no element intercepts left-click logic. Reproduction: - Open an Edgeless canvas - Press and hold middle mouse button over a shape/text/any element and drag - Expected: pan the canvas - Actual: the element gets selected or moved; no panning occurs Root cause: 1. PanTool switched via requestAnimationFrame; the current frame’s pointerDown/pointerMove were handled by DefaultTool first (handing middle mouse to left-click logic). 2. Selection restore used a live reference to `this.gfx.selection.surfaceSelections`, which could be mutated by other selection logic during the temporary pan, leading to incorrect restoration. Fix: - Switch to PanTool immediately on the same frame when middle mouse is pressed; add a guard to avoid switching if PanTool is already active. - Snapshot `surfaceSelections` using `slice()` before the temporary switch; restore it on `pointerup` so external mutations won’t affect restoration. - Only register the temporary `pointerup` listener when actually switching; on release, restore the previous tool (including `frameNavigator` with `restoredAfterPan: true`) and selection. Additionally, disable black background when exiting from frameNavigator. Affected files: - blocksuite/affine/gfx/pointer/src/tools/pan-tool.ts Tests: - packages/frontend/core/src/blocksuite/__tests__/pan-tool-middle-mouse.spec.ts - Verifies immediate PanTool switch, selection snapshot restoration, frameNavigator recovery flag, and no-op when PanTool is already active. Notes: - Aligned with docs/contributing/tutorial.md. Local validation performed. Thanks for reviewing! ## Summary by CodeRabbit * **Bug Fixes** * Prevented accidental re-activation of the middle-click pan tool. * Preserved and restored the user's selection and previous tool options after panning, including correct handling when returning to the frame navigator. * Ensured immediate tool switch to pan and reliable cleanup on middle-button release. * **Tests** * Added tests covering middle-click pan behavior, restoration flows, and no-op when pan is already active. --------- Co-authored-by: DarkSky <25152247+darkskygit@users.noreply.github.com> --- .../affine/gfx/pointer/src/tools/pan-tool.ts | 23 +- .../__tests__/pan-tool-middle-mouse.spec.ts | 223 ++++++++++++++++++ 2 files changed, 237 insertions(+), 9 deletions(-) create mode 100644 packages/frontend/core/src/blocksuite/__tests__/pan-tool-middle-mouse.spec.ts diff --git a/blocksuite/affine/gfx/pointer/src/tools/pan-tool.ts b/blocksuite/affine/gfx/pointer/src/tools/pan-tool.ts index 345011327..c1abe0aea 100644 --- a/blocksuite/affine/gfx/pointer/src/tools/pan-tool.ts +++ b/blocksuite/affine/gfx/pointer/src/tools/pan-tool.ts @@ -60,12 +60,20 @@ export class PanTool extends BaseTool { return; } + const currentTool = this.controller.currentToolOption$.peek(); + const { toolType, options: originalToolOptions } = currentTool; + + if (toolType?.toolName === PanTool.toolName) { + return; + } + evt.raw.preventDefault(); - const currentTool = this.controller.currentToolOption$.peek(); + const selectionToRestore = this.gfx.selection.surfaceSelections.slice(); + const restoreToPrevious = () => { - const { toolType, options: originalToolOptions } = currentTool; - const selectionToRestore = this.gfx.selection.surfaceSelections; + this.gfx.selection.set(selectionToRestore); + if (!toolType) return; // restore to DefaultTool if previous tool is CopilotTool if (toolType.toolName === 'copilot') { @@ -88,21 +96,18 @@ export class PanTool extends BaseTool { } as RestorablePresentToolOptions; } this.controller.setTool(toolType, finalOptions); - this.gfx.selection.set(selectionToRestore); }; // If in presentation mode, disable black background after middle mouse drag - if (currentTool.toolType?.toolName === 'frameNavigator') { + if (toolType?.toolName === 'frameNavigator') { const slots = this.std.get(EdgelessLegacySlotIdentifier); slots.navigatorSettingUpdated.next({ blackBackground: false, }); } - requestAnimationFrame(() => { - this.controller.setTool(PanTool, { - panning: true, - }); + this.controller.setTool(PanTool, { + panning: true, }); const dispose = on(document, 'pointerup', evt => { diff --git a/packages/frontend/core/src/blocksuite/__tests__/pan-tool-middle-mouse.spec.ts b/packages/frontend/core/src/blocksuite/__tests__/pan-tool-middle-mouse.spec.ts new file mode 100644 index 000000000..80a5d17ce --- /dev/null +++ b/packages/frontend/core/src/blocksuite/__tests__/pan-tool-middle-mouse.spec.ts @@ -0,0 +1,223 @@ +// eslint-disable-next-line import-x/no-extraneous-dependencies +import { PanTool } from '@blocksuite/affine-gfx-pointer'; +import { on } from '@blocksuite/affine-shared/utils'; +import type { PointerEventState } from '@blocksuite/std'; +import { + BaseTool, + MouseButton, + type ToolOptionWithType, + type ToolType, +} from '@blocksuite/std/gfx'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; + +type SelectionEntry = { + blockId: string; + elements: string[]; + editing: boolean; + inoperable?: boolean; +}; + +const pointerUpHandlers: unknown[] = []; +const pointerUpDisposers: Array> = []; + +vi.mock('@blocksuite/affine-shared/utils', async () => { + const actual = await vi.importActual< + typeof import('@blocksuite/affine-shared/utils') + >('@blocksuite/affine-shared/utils'); + + return { + ...actual, + on: vi.fn( + ( + _target: Document, + eventName: string, + handler: (event: Pick) => void + ) => { + if (eventName === 'pointerup') { + pointerUpHandlers.push(handler); + const dispose = vi.fn(() => { + const index = pointerUpHandlers.indexOf(handler); + if (index >= 0) { + pointerUpHandlers.splice(index, 1); + } + }); + pointerUpDisposers.push(dispose); + return dispose; + } + return vi.fn(); + } + ), + }; +}); + +const createPointerEventState = ( + button = MouseButton.MIDDLE +): PointerEventState => + ({ + raw: { + button, + preventDefault: vi.fn(), + }, + }) as unknown as PointerEventState; + +const createPanToolHarness = ( + toolName = 'default', + options?: Record +) => { + const selectionEntry: SelectionEntry = { + blockId: 'edgeless', + elements: ['shape-1'], + editing: false, + }; + const surfaceSelections = [selectionEntry]; + const selection = { + surfaceSelections, + set: vi.fn(), + }; + + const originalToolType = { toolName } as unknown as ToolType; + const currentToolOption = { + toolType: originalToolType, + options: options as ToolOptionWithType['options'], + }; + + const setTool = vi.fn(); + const navigatorSettingUpdated = { next: vi.fn() }; + const gfx = { + selection, + std: { + get: vi.fn().mockReturnValue({ + navigatorSettingUpdated, + }), + }, + tool: { + ['currentToolOption$']: { + peek: () => currentToolOption, + get value() { + return currentToolOption; + }, + }, + setTool, + }, + }; + + // Ensure a global document exists for PanTool's middle mouse handler in Node test env + (globalThis as any).document ??= {}; + + const panTool = new PanTool(gfx as unknown as any); + let pointerDownHandler: ((evt: PointerEventState) => void | boolean) | null = + null; + + (panTool as any).eventTarget = { + addHook: (_eventName: string, handler: typeof pointerDownHandler) => { + if (_eventName === 'pointerDown') { + pointerDownHandler = handler; + } + }, + }; + + panTool.mounted(); + + if (!pointerDownHandler) { + throw new Error('pointerDown handler was not registered'); + } + + return { + pointerDownHandler: pointerDownHandler as ( + evt: PointerEventState + ) => void | boolean, + selection, + selectionEntry, + originalToolType, + originalToolOptions: options, + setTool, + navigatorSettingUpdated, + }; +}; + +describe('PanTool middle mouse behavior', () => { + beforeEach(() => { + pointerUpHandlers.length = 0; + pointerUpDisposers.length = 0; + vi.clearAllMocks(); + }); + + test('temporarily switches to pan tool and restores original selection snapshot', () => { + const originalOptions = { foo: 'bar' }; + const { + pointerDownHandler, + selection, + selectionEntry, + originalToolType, + originalToolOptions, + setTool, + } = createPanToolHarness('default', originalOptions); + + const pointerState = createPointerEventState(); + const result = pointerDownHandler(pointerState); + + expect(result).toBe(false); + expect(pointerState.raw.preventDefault).toHaveBeenCalledTimes(1); + expect(setTool).toHaveBeenNthCalledWith(1, PanTool, { panning: true }); + expect(pointerUpHandlers).toHaveLength(1); + + selection.surfaceSelections[0] = { + blockId: 'edgeless', + elements: ['mutated'], + editing: false, + }; + + const middlePointerUpHandler = pointerUpHandlers[0]! as ( + event: Pick + ) => void; + middlePointerUpHandler({ button: MouseButton.MIDDLE }); + + expect(selection.set).toHaveBeenCalledWith([selectionEntry]); + expect(setTool).toHaveBeenNthCalledWith( + 2, + originalToolType, + originalToolOptions + ); + expect(pointerUpDisposers[0]).toHaveBeenCalledTimes(1); + }); + + test('restores frame navigator with restoredAfterPan flag', () => { + const frameOptions = { mode: 'fit' }; + const { + pointerDownHandler, + navigatorSettingUpdated, + setTool, + originalToolType, + } = createPanToolHarness('frameNavigator', frameOptions); + + pointerDownHandler(createPointerEventState()); + expect(navigatorSettingUpdated.next).toHaveBeenCalledWith({ + blackBackground: false, + }); + + const frameNavigatorPointerUpHandler = pointerUpHandlers[0]! as ( + event: Pick + ) => void; + frameNavigatorPointerUpHandler({ button: MouseButton.MIDDLE }); + + expect(setTool).toHaveBeenNthCalledWith(2, originalToolType, { + ...frameOptions, + restoredAfterPan: true, + }); + }); + + test('ignores middle button when PanTool is already active', () => { + const { pointerDownHandler, setTool } = createPanToolHarness( + PanTool.toolName + ); + const pointerState = createPointerEventState(); + + const result = pointerDownHandler(pointerState); + + expect(result).toBeUndefined(); + expect(pointerState.raw.preventDefault).not.toHaveBeenCalled(); + expect(setTool).not.toHaveBeenCalled(); + expect(on).not.toHaveBeenCalled(); + expect(pointerUpHandlers).toHaveLength(0); + }); +});