mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-07-20 19:46:32 +08:00
fix(editor): fix overlay of tool is not shown or repeated when switching tool (#11575)
Close [BS-3029](https://linear.app/affine-design/issue/BS-3029/frame-里面的-shape-没办法进入文本编辑模式) Close [BS-3082](https://linear.app/affine-design/issue/BS-3082/按s切换至shape工具,在白板上点击会创建两个shape) Close [BS-3091](https://linear.app/affine-design/issue/BS-3082/按s切换至shape工具,在白板上点击会创建两个shape) ## Fix Shape Tool Issues This PR addresses several issues with the shape and mindmap tools functionality in the editor: 1. **Fix text editing after mode switching**: Resolves an issue where users couldn't edit text in shapes after switching editor modes. The fix ensures the edgeless block is properly retrieved when double-clicking on a shape. 2. **Improve tool switching behavior**: Fixes issues with tool overlays not showing or being repeated when switching between tools. This includes: - Properly handling tool overlay visibility - Ensuring only one tool is active at a time when using keyboard shortcuts - Adding proper cleanup when switching tools 3. **Add comprehensive tests**: Adds new test cases to verify: - Shape creation with keyboard shortcuts - Shape text editing after mode switching - Tool switching behavior with keyboard shortcuts
This commit is contained in:
@@ -49,7 +49,7 @@ import {
|
||||
import type { BaseSelection, Store } from '@blocksuite/store';
|
||||
import { effect, signal } from '@preact/signals-core';
|
||||
import { css, html, nothing } from 'lit';
|
||||
import { query, state } from 'lit/decorators.js';
|
||||
import { query } from 'lit/decorators.js';
|
||||
import { classMap } from 'lit/directives/class-map.js';
|
||||
import { guard } from 'lit/directives/guard.js';
|
||||
import { styleMap } from 'lit/directives/style-map.js';
|
||||
@@ -103,17 +103,12 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
margin: 0 auto;
|
||||
position: relative;
|
||||
overflow: hidden;
|
||||
pointer-events: none;
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
.ref-viewport-event-mask {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
left: 0;
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
pointer-events: auto;
|
||||
inset: 0;
|
||||
}
|
||||
`;
|
||||
|
||||
@@ -139,11 +134,11 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
return this._referencedModel;
|
||||
}
|
||||
|
||||
private _focusBlock() {
|
||||
private readonly _handleClick = () => {
|
||||
this.selection.update(() => {
|
||||
return [this.selection.create(BlockSelection, { blockId: this.blockId })];
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
private _initHotkey() {
|
||||
const selection = this.host.selection;
|
||||
@@ -178,7 +173,7 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
|
||||
this.bindHotKey({
|
||||
Enter: () => {
|
||||
if (!this._focused) return;
|
||||
if (!this.selected$.value) return;
|
||||
addParagraph();
|
||||
return true;
|
||||
},
|
||||
@@ -260,17 +255,6 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
}
|
||||
}
|
||||
|
||||
private _initSelection() {
|
||||
const selection = this.host.selection;
|
||||
this._disposables.add(
|
||||
selection.slots.changed.subscribe(selList => {
|
||||
this._focused = selList.some(
|
||||
sel => sel.blockId === this.blockId && sel.is(BlockSelection)
|
||||
);
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
private _initViewport() {
|
||||
const refreshViewport = () => {
|
||||
if (!this._referenceXYWH$.value) return;
|
||||
@@ -436,7 +420,6 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
this._initHotkey();
|
||||
this._initViewport();
|
||||
this._initReferencedModel();
|
||||
this._initSelection();
|
||||
}
|
||||
|
||||
override firstUpdated() {
|
||||
@@ -462,10 +445,10 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
<div
|
||||
class=${classMap({
|
||||
'affine-surface-ref': true,
|
||||
focused: this._focused,
|
||||
focused: this.selected$.value,
|
||||
})}
|
||||
data-theme=${edgelessTheme}
|
||||
@click=${this._focusBlock}
|
||||
@click=${this._handleClick}
|
||||
>
|
||||
${content}
|
||||
</div>
|
||||
@@ -488,9 +471,6 @@ export class SurfaceRefBlockComponent extends BlockComponent<SurfaceRefBlockMode
|
||||
this.std.get(DocModeProvider).setEditorMode('edgeless');
|
||||
}
|
||||
|
||||
@state()
|
||||
private accessor _focused: boolean = false;
|
||||
|
||||
@query('.affine-surface-ref')
|
||||
accessor hoverableContainer!: HTMLDivElement;
|
||||
|
||||
|
||||
@@ -17,8 +17,12 @@ export interface IModelCoord {
|
||||
y: number;
|
||||
}
|
||||
|
||||
// TODO(@L-Sun): we should remove this list when refactor the pointerOut event to pointerLeave,
|
||||
// since the previous will be triggered when the pointer move to the area of the its children elements
|
||||
// see: https://developer.mozilla.org/en-US/docs/Web/API/Element/pointerout_event
|
||||
export const EXCLUDING_MOUSE_OUT_CLASS_LIST = [
|
||||
'affine-note-mask',
|
||||
'edgeless-block-portal-note',
|
||||
'affine-block-children-container',
|
||||
'edgeless-container',
|
||||
];
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { DisposableGroup } from '@blocksuite/global/disposable';
|
||||
import { noop } from '@blocksuite/global/utils';
|
||||
import type { GfxController } from '@blocksuite/std/gfx';
|
||||
import { startWith } from 'rxjs';
|
||||
|
||||
import type { RoughCanvas } from '../utils/rough/canvas';
|
||||
import { Overlay } from './overlay';
|
||||
@@ -18,10 +19,11 @@ export class ToolOverlay extends Overlay {
|
||||
super(gfx);
|
||||
this.x = 0;
|
||||
this.y = 0;
|
||||
this.globalAlpha = 0;
|
||||
this.globalAlpha = 1;
|
||||
this.gfx = gfx;
|
||||
|
||||
this.disposables.add(
|
||||
this.gfx.viewport.viewportUpdated.subscribe(() => {
|
||||
this.gfx.viewport.viewportUpdated.pipe(startWith(null)).subscribe(() => {
|
||||
// when viewport is updated, we should keep the overlay in the same position
|
||||
// to get last mouse position and convert it to model coordinates
|
||||
const pos = this.gfx.tool.lastMousePos$.value;
|
||||
|
||||
@@ -326,6 +326,16 @@ export class EdgelessMindmapToolButton extends EdgelessToolbarToolMixin(
|
||||
},
|
||||
{ global: true }
|
||||
);
|
||||
|
||||
// since there is not a tool called mindmap, we need to cancel the drag when the tool is changed
|
||||
this.disposables.add(
|
||||
this.gfx.tool.currentToolName$.subscribe(toolName => {
|
||||
// FIXME: remove the assertion after gfx tool refactor
|
||||
if ((toolName as string) !== 'empty' && this.readyToDrop) {
|
||||
this.draggableController.cancel();
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
override render() {
|
||||
|
||||
@@ -26,7 +26,6 @@ export class NoteOverlay extends ToolOverlay {
|
||||
|
||||
constructor(gfx: GfxController, background: Color) {
|
||||
super(gfx);
|
||||
this.globalAlpha = 0;
|
||||
this.backgroundColor = gfx.std
|
||||
.get(ThemeProvider)
|
||||
.getColorValue(background, DefaultTheme.noteBackgrounColor, true);
|
||||
|
||||
@@ -236,30 +236,39 @@ export class EdgelessToolbarShapeDraggable extends EdgelessToolbarToolMixin(
|
||||
const locked = this.gfx.viewport.locked;
|
||||
const selection = this.gfx.selection;
|
||||
if (locked || selection.editing) return;
|
||||
|
||||
if (this.readyToDrop) {
|
||||
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}`
|
||||
) as HTMLElement;
|
||||
if (!el) {
|
||||
console.error('Edgeless toolbar Shape element not found');
|
||||
if (
|
||||
this.gfx.tool.dragging$.peek() &&
|
||||
this.gfx.tool.currentToolName$.peek() === 'shape'
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const { x, y } = this.gfx.tool.lastMousePos$.peek();
|
||||
const { viewport } = this.edgeless.std.get(ViewportElementProvider);
|
||||
const { left, top } = viewport;
|
||||
const clientPos = { x: x + left, y: y + top };
|
||||
this.draggableController.dragAndMoveTo(el, clientPos);
|
||||
|
||||
const activeIndex = shapes.findIndex(
|
||||
s => s.name === this.draggingShape
|
||||
);
|
||||
const nextIndex = (activeIndex + 1) % shapes.length;
|
||||
const next = shapes[nextIndex];
|
||||
this.draggingShape = next.name;
|
||||
|
||||
if (this.readyToDrop) {
|
||||
this.draggableController.cancelWithoutAnimation();
|
||||
const el = this.shapeContainer.querySelector(
|
||||
`.shape.${this.draggingShape}`
|
||||
) as HTMLElement;
|
||||
if (!el) {
|
||||
console.error('Edgeless toolbar Shape element not found');
|
||||
return;
|
||||
}
|
||||
const { x, y } = this.gfx.tool.lastMousePos$.peek();
|
||||
const { viewport } = this.edgeless.std.get(ViewportElementProvider);
|
||||
const { left, top } = viewport;
|
||||
const clientPos = { x: x + left, y: y + top };
|
||||
this.draggableController.dragAndMoveTo(el, clientPos);
|
||||
} else {
|
||||
this.setEdgelessTool('shape', {
|
||||
shapeName: this.draggingShape,
|
||||
});
|
||||
}
|
||||
},
|
||||
},
|
||||
{ global: true }
|
||||
|
||||
@@ -89,7 +89,6 @@ export class ShapeTool extends BaseTool<ShapeToolOption> {
|
||||
|
||||
private _hideOverlay() {
|
||||
if (!this._shapeOverlay) return;
|
||||
|
||||
this._shapeOverlay.globalAlpha = 0;
|
||||
(this.gfx.surfaceComponent as SurfaceBlockComponent)?.refresh();
|
||||
}
|
||||
|
||||
@@ -13,9 +13,9 @@ export class ShapeElementView extends GfxElementModelView<ShapeElementModel> {
|
||||
}
|
||||
|
||||
private _initDblClickToEdit(): void {
|
||||
const edgeless = this.std.view.getBlock(this.std.store.root!.id);
|
||||
|
||||
this.on('dblclick', () => {
|
||||
const edgeless = this.std.view.getBlock(this.std.store.root!.id);
|
||||
|
||||
if (edgeless && !this.model.isLocked()) {
|
||||
mountShapeTextEditor(this.model, edgeless);
|
||||
}
|
||||
|
||||
@@ -16,7 +16,6 @@ import { openHomePage } from '@affine-test/kit/utils/load-page';
|
||||
import {
|
||||
addCodeBlock,
|
||||
clickNewPageButton,
|
||||
getBlockSuiteEditorTitle,
|
||||
type,
|
||||
waitForEditorLoad,
|
||||
} from '@affine-test/kit/utils/page-logic';
|
||||
@@ -178,9 +177,6 @@ test.describe('paste in multiple blocks text selection', () => {
|
||||
test('paste surface-ref block to another doc as embed-linked-doc block', async ({
|
||||
page,
|
||||
}) => {
|
||||
await openHomePage(page);
|
||||
await clickNewPageButton(page, 'Clipboard Test');
|
||||
await waitForEditorLoad(page);
|
||||
await clickEdgelessModeButton(page);
|
||||
const container = locateEditorContainer(page);
|
||||
await container.click();
|
||||
@@ -205,21 +201,18 @@ test('paste surface-ref block to another doc as embed-linked-doc block', async (
|
||||
await insertIntoPageButton.click();
|
||||
|
||||
await clickPageModeButton(page);
|
||||
await page.waitForTimeout(50);
|
||||
await waitForEditorLoad(page);
|
||||
await container.click();
|
||||
|
||||
// copy surface-ref block
|
||||
const surfaceRefBlock = page.locator('.affine-surface-ref');
|
||||
const surfaceRefBlock = page.locator('affine-surface-ref');
|
||||
await surfaceRefBlock.click();
|
||||
await page.waitForTimeout(50);
|
||||
await page.waitForSelector('affine-surface-ref .focused');
|
||||
await copyByKeyboard(page);
|
||||
|
||||
// paste to another doc
|
||||
await clickNewPageButton(page);
|
||||
await waitForEditorLoad(page);
|
||||
const title2 = getBlockSuiteEditorTitle(page);
|
||||
await title2.pressSequentially('page2');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.waitForTimeout(50);
|
||||
await clickNewPageButton(page, 'page2');
|
||||
await pressEnter(page);
|
||||
|
||||
// paste the surface-ref block
|
||||
await pasteByKeyboard(page);
|
||||
|
||||
@@ -1,13 +1,19 @@
|
||||
import { expect } from '@playwright/test';
|
||||
|
||||
import { clickView } from '../utils/actions/click.js';
|
||||
import {
|
||||
addBasicRectShapeElement,
|
||||
getSelectedBoundCount,
|
||||
locatorComponentToolbar,
|
||||
resizeElementByHandle,
|
||||
selectNoteInEdgeless,
|
||||
switchEditorMode,
|
||||
zoomResetByKeyboard,
|
||||
} from '../utils/actions/edgeless.js';
|
||||
import {
|
||||
pressBackspace,
|
||||
selectAllBlocksByKeyboard,
|
||||
} from '../utils/actions/keyboard.js';
|
||||
import {
|
||||
enterPlaygroundRoom,
|
||||
initEmptyEdgelessState,
|
||||
@@ -87,3 +93,26 @@ test('should be hidden when resizing element', async ({ page }) => {
|
||||
|
||||
await expect(toolbar).toBeVisible();
|
||||
});
|
||||
|
||||
test('should only one tool active at the same time when using shortcut to switch tool', async ({
|
||||
page,
|
||||
}) => {
|
||||
await enterPlaygroundRoom(page);
|
||||
await initEmptyEdgelessState(page);
|
||||
await switchEditorMode(page);
|
||||
await clickView(page, [0, 0]);
|
||||
await selectAllBlocksByKeyboard(page);
|
||||
await pressBackspace(page);
|
||||
|
||||
await page.keyboard.press('s');
|
||||
await page.keyboard.press('m');
|
||||
await page.keyboard.press('n');
|
||||
|
||||
await clickView(page, [100, 100]);
|
||||
await clickView(page, [0, 0]); // click on empty space to deselect the note
|
||||
await selectAllBlocksByKeyboard(page);
|
||||
expect(
|
||||
await getSelectedBoundCount(page),
|
||||
'only a note should be created'
|
||||
).toBe(1);
|
||||
});
|
||||
|
||||
@@ -9,7 +9,9 @@ import {
|
||||
changeShapeStrokeStyle,
|
||||
changeShapeStrokeWidth,
|
||||
clickComponentToolbarMoreMenuButton,
|
||||
dragBetweenViewCoords,
|
||||
getEdgelessSelectedRect,
|
||||
getSelectedBoundCount,
|
||||
locatorComponentToolbar,
|
||||
locatorEdgelessToolButton,
|
||||
locatorShapeStrokeStyleButton,
|
||||
@@ -24,13 +26,17 @@ import {
|
||||
import {
|
||||
addBasicBrushElement,
|
||||
addBasicRectShapeElement,
|
||||
clickView,
|
||||
copyByKeyboard,
|
||||
dblclickView,
|
||||
dragBetweenCoords,
|
||||
enterPlaygroundRoom,
|
||||
focusRichText,
|
||||
initEmptyEdgelessState,
|
||||
pasteByKeyboard,
|
||||
pressBackspace,
|
||||
pressEscape,
|
||||
selectAllBlocksByKeyboard,
|
||||
type,
|
||||
waitNextFrame,
|
||||
} from '../utils/actions/index.js';
|
||||
@@ -40,6 +46,7 @@ import {
|
||||
assertEdgelessNonSelectedRect,
|
||||
assertEdgelessSelectedRect,
|
||||
assertRichTexts,
|
||||
assertSelectedBound,
|
||||
} from '../utils/asserts.js';
|
||||
import { test } from '../utils/playwright.js';
|
||||
|
||||
@@ -739,3 +746,45 @@ test.describe('shape hit test', () => {
|
||||
await assertEdgelessCanvasText(page, 'hello world');
|
||||
});
|
||||
});
|
||||
|
||||
test('should create a shape when press s and click on canvas', async ({
|
||||
page,
|
||||
}) => {
|
||||
await enterPlaygroundRoom(page);
|
||||
await initEmptyEdgelessState(page);
|
||||
await switchEditorMode(page);
|
||||
await clickView(page, [0, 0]);
|
||||
await zoomResetByKeyboard(page);
|
||||
await selectAllBlocksByKeyboard(page);
|
||||
await pressBackspace(page);
|
||||
|
||||
await page.keyboard.press('s');
|
||||
await assertEdgelessTool(page, 'shape');
|
||||
await clickView(page, [100, 100]);
|
||||
await selectAllBlocksByKeyboard(page);
|
||||
expect(await getSelectedBoundCount(page)).toBe(1);
|
||||
await assertSelectedBound(page, [100, 100, 100, 100]);
|
||||
});
|
||||
|
||||
test('shape should be editable when re-enter canvas', async ({ page }) => {
|
||||
await enterPlaygroundRoom(page);
|
||||
await initEmptyEdgelessState(page);
|
||||
await switchEditorMode(page);
|
||||
await clickView(page, [0, 0]);
|
||||
await zoomResetByKeyboard(page);
|
||||
await selectAllBlocksByKeyboard(page);
|
||||
await pressBackspace(page);
|
||||
|
||||
await page.keyboard.press('s');
|
||||
await dragBetweenViewCoords(page, [0, 0], [100, 100]);
|
||||
await dblclickView(page, [50, 50]);
|
||||
await type(page, 'hello');
|
||||
await expect(page.locator('edgeless-shape-text-editor')).toBeAttached();
|
||||
await assertEdgelessCanvasText(page, 'hello');
|
||||
|
||||
await switchEditorMode(page);
|
||||
await switchEditorMode(page);
|
||||
|
||||
await dblclickView(page, [50, 50]);
|
||||
await expect(page.locator('edgeless-shape-text-editor')).toBeAttached();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user