From 7223d35c892d015430eb024a284f1239de8ab2e7 Mon Sep 17 00:00:00 2001 From: L-Sun Date: Fri, 23 May 2025 16:31:01 +0000 Subject: [PATCH] fix(editor): shape tool should be the last used shape (#12425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close [BS-3305](https://linear.app/affine-design/issue/BS-3305/白板按s应该使用上次的shape形状) ## Summary by CodeRabbit - **New Features** - Added a method to cycle through shape types when using the shape tool. - **Bug Fixes** - Improved shape tool behavior to ensure the selected shape type does not change unexpectedly after adding a new shape. - **Tests** - Added an end-to-end test to verify that the shape tool retains the selected shape type after adding a new shape. - Enhanced shortcut tests to verify cycling shapes forward and backward using 's' and 'Shift+s' keys. - **Refactor** - Streamlined the logic for cycling through shape types and connector modes for improved maintainability. - Removed external utility for cycling shapes, integrating the functionality directly into the shape tool. - Updated keyboard shortcut handling to use the new cycling method within the shape tool. --- .../root/src/edgeless/edgeless-keyboard.ts | 5 +-- .../root/src/edgeless/utils/hotkey-utils.ts | 15 --------- .../gfx/connector/src/connector-tool.ts | 18 ++++++----- .../shape/src/draggable/shape-draggable.ts | 29 +++++++++-------- blocksuite/affine/gfx/shape/src/shape-tool.ts | 31 +++++++++++++++++-- blocksuite/affine/model/src/consts/shape.ts | 4 +-- tests/blocksuite/e2e/edgeless/shape.spec.ts | 19 ++++++++++++ .../blocksuite/e2e/edgeless/shortcut.spec.ts | 7 ++++- 8 files changed, 83 insertions(+), 45 deletions(-) delete mode 100644 blocksuite/affine/blocks/root/src/edgeless/utils/hotkey-utils.ts diff --git a/blocksuite/affine/blocks/root/src/edgeless/edgeless-keyboard.ts b/blocksuite/affine/blocks/root/src/edgeless/edgeless-keyboard.ts index bb43c4c72a..e03a05a5cf 100644 --- a/blocksuite/affine/blocks/root/src/edgeless/edgeless-keyboard.ts +++ b/blocksuite/affine/blocks/root/src/edgeless/edgeless-keyboard.ts @@ -66,7 +66,6 @@ import { DEFAULT_NOTE_TIP, } from './utils/consts.js'; import { deleteElements } from './utils/crud.js'; -import { getNextShapeType } from './utils/hotkey-utils.js'; import { isCanvasElement } from './utils/query.js'; export class EdgelessPageKeyboardManager extends PageKeyboardManager { @@ -216,10 +215,8 @@ export class EdgelessPageKeyboardManager extends PageKeyboardManager { ) { return; } - const { shapeName } = controller.activatedOption; - const nextShapeName = getNextShapeType(shapeName); this._setEdgelessTool(ShapeTool, { - shapeName: nextShapeName, + shapeName: controller.cycleShapeName('prev'), }); controller.createOverlay(); diff --git a/blocksuite/affine/blocks/root/src/edgeless/utils/hotkey-utils.ts b/blocksuite/affine/blocks/root/src/edgeless/utils/hotkey-utils.ts deleted file mode 100644 index fcd217d72f..0000000000 --- a/blocksuite/affine/blocks/root/src/edgeless/utils/hotkey-utils.ts +++ /dev/null @@ -1,15 +0,0 @@ -import type { ShapeToolOption } from '@blocksuite/affine-gfx-shape'; -import { ShapeType } from '@blocksuite/affine-model'; - -const shapeMap: Record = { - [ShapeType.Rect]: 0, - [ShapeType.Ellipse]: 1, - [ShapeType.Diamond]: 2, - [ShapeType.Triangle]: 3, - roundedRect: 4, -}; -const shapes = Object.keys(shapeMap) as ShapeToolOption['shapeName'][]; - -export function getNextShapeType(cur: ShapeToolOption['shapeName']) { - return shapes[(shapeMap[cur] + 1) % shapes.length]; -} diff --git a/blocksuite/affine/gfx/connector/src/connector-tool.ts b/blocksuite/affine/gfx/connector/src/connector-tool.ts index fe04313fe3..d601936ca2 100644 --- a/blocksuite/affine/gfx/connector/src/connector-tool.ts +++ b/blocksuite/affine/gfx/connector/src/connector-tool.ts @@ -223,13 +223,15 @@ export class ConnectorTool extends BaseTool { } getNextMode() { - switch (this.activatedOption.mode) { - case ConnectorMode.Curve: - return ConnectorMode.Orthogonal; - case ConnectorMode.Orthogonal: - return ConnectorMode.Straight; - case ConnectorMode.Straight: - return ConnectorMode.Curve; - } + // reorder the enum values + const modes = [ + ConnectorMode.Curve, + ConnectorMode.Orthogonal, + ConnectorMode.Straight, + ]; + + const currentIndex = modes.indexOf(this.activatedOption.mode); + const nextIndex = (currentIndex + 1) % modes.length; + return modes[nextIndex]; } } diff --git a/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts b/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts index c994cccc19..ac539eaa61 100644 --- a/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts +++ b/blocksuite/affine/gfx/shape/src/draggable/shape-draggable.ts @@ -235,21 +235,18 @@ export class EdgelessToolbarShapeDraggable extends EdgelessToolbarToolMixin( const locked = this.gfx.viewport.locked; const selection = this.gfx.selection; if (locked || selection.editing) return; - if ( - this.gfx.tool.dragging$.peek() && - this.gfx.tool.currentToolName$.peek() === 'shape' - ) { - return; - } - const activeIndex = shapes.findIndex( - s => s.name === this.draggingShape - ); - const nextIndex = (activeIndex + 1) % shapes.length; - const next = shapes[nextIndex]; - this.draggingShape = next.name; + const currentTool = this.gfx.tool.currentToolName$.peek(); if (this.readyToDrop) { + if (currentTool === ShapeTool.toolName) { + const activeIndex = shapes.findIndex( + s => s.name === this.draggingShape + ); + const nextIndex = (activeIndex + 1) % shapes.length; + const next = shapes[nextIndex]; + this.draggingShape = next.name; + } this.draggableController.cancelWithoutAnimation(); const el = this.shapeContainer.querySelector( `.shape.${this.draggingShape}` @@ -264,8 +261,14 @@ export class EdgelessToolbarShapeDraggable extends EdgelessToolbarToolMixin( const clientPos = { x: x + left, y: y + top }; this.draggableController.dragAndMoveTo(el, clientPos); } else { + if (this.gfx.tool.dragging$.peek()) return; + let shapeName = + this.gfx.tool.get(ShapeTool).activatedOption.shapeName; + if (currentTool === ShapeTool.toolName) { + shapeName = this.gfx.tool.get(ShapeTool).cycleShapeName('next'); + } this.setEdgelessTool(ShapeTool, { - shapeName: this.draggingShape, + shapeName, }); } }, diff --git a/blocksuite/affine/gfx/shape/src/shape-tool.ts b/blocksuite/affine/gfx/shape/src/shape-tool.ts index de04980e9c..8210448aea 100644 --- a/blocksuite/affine/gfx/shape/src/shape-tool.ts +++ b/blocksuite/affine/gfx/shape/src/shape-tool.ts @@ -5,7 +5,11 @@ import { type SurfaceBlockComponent, } from '@blocksuite/affine-block-surface'; import type { ShapeElementModel, ShapeName } from '@blocksuite/affine-model'; -import { DefaultTheme, getShapeType } from '@blocksuite/affine-model'; +import { + DefaultTheme, + getShapeType, + ShapeType, +} from '@blocksuite/affine-model'; import { EditPropsStore, TelemetryProvider, @@ -15,7 +19,7 @@ import { hasClassNameInList } from '@blocksuite/affine-shared/utils'; import type { IBound } from '@blocksuite/global/gfx'; import { Bound } from '@blocksuite/global/gfx'; import type { PointerEventState } from '@blocksuite/std'; -import { BaseTool } from '@blocksuite/std/gfx'; +import { BaseTool, type GfxController } from '@blocksuite/std/gfx'; import { effect } from '@preact/signals-core'; import { @@ -159,6 +163,13 @@ export class ShapeTool extends BaseTool { this._surfaceComponent?.refresh(); } + constructor(gfx: GfxController) { + super(gfx); + this.activatedOption = { + shapeName: ShapeType.Rect, + }; + } + override activate() { this.createOverlay(); } @@ -337,4 +348,20 @@ export class ShapeTool extends BaseTool { setDisableOverlay(disable: boolean) { this._disableOverlay = disable; } + + cycleShapeName(dir: 'prev' | 'next' = 'next'): ShapeName { + const shapeNames: ShapeName[] = [ + ShapeType.Rect, + ShapeType.Ellipse, + ShapeType.Diamond, + ShapeType.Triangle, + 'roundedRect', + ]; + + const currentIndex = shapeNames.indexOf(this.activatedOption.shapeName); + const nextIndex = + (currentIndex + (dir === 'prev' ? -1 : 1) + shapeNames.length) % + shapeNames.length; + return shapeNames[nextIndex]; + } } diff --git a/blocksuite/affine/model/src/consts/shape.ts b/blocksuite/affine/model/src/consts/shape.ts index e81821cdb1..51da58289c 100644 --- a/blocksuite/affine/model/src/consts/shape.ts +++ b/blocksuite/affine/model/src/consts/shape.ts @@ -11,9 +11,9 @@ export enum ShapeTextFontSize { } export enum ShapeType { - Diamond = 'diamond', - Ellipse = 'ellipse', Rect = 'rect', + Ellipse = 'ellipse', + Diamond = 'diamond', Triangle = 'triangle', } diff --git a/tests/blocksuite/e2e/edgeless/shape.spec.ts b/tests/blocksuite/e2e/edgeless/shape.spec.ts index 901bca1f0a..c099dc8bf1 100644 --- a/tests/blocksuite/e2e/edgeless/shape.spec.ts +++ b/tests/blocksuite/e2e/edgeless/shape.spec.ts @@ -2,6 +2,7 @@ import { expect, type Page } from '@playwright/test'; import { lightThemeV2 } from '@toeverything/theme/v2'; import { + assertEdgelessShapeType, assertEdgelessTool, changeShapeFillColor, changeShapeFillColorToTransparent, @@ -788,3 +789,21 @@ test('shape should be editable when re-enter canvas', async ({ page }) => { await dblclickView(page, [50, 50]); await expect(page.locator('edgeless-shape-text-editor')).toBeAttached(); }); + +test('shape tool should not be changed after adding new shape', async ({ + page, +}) => { + await enterPlaygroundRoom(page); + await initEmptyEdgelessState(page); + await switchEditorMode(page); + + await setEdgelessTool(page, 'shape'); + await page.keyboard.press('s'); + await waitNextFrame(page); + await assertEdgelessShapeType(page, 'ellipse'); + await clickView(page, [0, 0]); + + await page.keyboard.press('s'); + await waitNextFrame(page); + await assertEdgelessShapeType(page, 'ellipse'); +}); diff --git a/tests/blocksuite/e2e/edgeless/shortcut.spec.ts b/tests/blocksuite/e2e/edgeless/shortcut.spec.ts index 1bf73d3956..abafcec14e 100644 --- a/tests/blocksuite/e2e/edgeless/shortcut.spec.ts +++ b/tests/blocksuite/e2e/edgeless/shortcut.spec.ts @@ -86,9 +86,14 @@ test('toggle shapes shortcut', async ({ page }) => { 'roundedRect', ] as ShapeName[]; for (const shape of shapesInOrder) { - await page.keyboard.press('Shift+s'); + await page.keyboard.press('s'); await assertEdgelessShapeType(page, shape); } + + for (const shape of shapesInOrder.reverse()) { + await assertEdgelessShapeType(page, shape); + await page.keyboard.press('Shift+s'); + } }); test('should not switch shapes in editing', async ({ page }) => {