From c6cbd24aeb36c0932e8a290620b694e8b58c43ad Mon Sep 17 00:00:00 2001 From: L-Sun Date: Tue, 15 Oct 2024 02:58:02 +0000 Subject: [PATCH] fix(edgeless): cmd a will select element inner frame (#8517) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close [BS-1579](https://linear.app/affine-design/issue/BS-1579/cmda全选会选到frame内部yuan素) ### Before select shape unexpectly https://github.com/user-attachments/assets/412115f8-9e89-43fc-b56f-a959ac25b40b ### After Only select frame because it is the container of the shape https://github.com/user-attachments/assets/136b073a-4d38-486a-9493-764bcafacca7 --- .../root-block/edgeless/edgeless-keyboard.ts | 4 +- .../edgeless/edgeless-root-service.ts | 11 +- .../element-toolbar/more-menu/config.ts | 16 +- tests/edgeless/frame/frame.spec.ts | 170 ++++++++++-------- tests/edgeless/frame/selection.spec.ts | 26 ++- tests/utils/actions/edgeless.ts | 29 ++- tests/utils/asserts.ts | 10 ++ 7 files changed, 180 insertions(+), 86 deletions(-) diff --git a/packages/blocks/src/root-block/edgeless/edgeless-keyboard.ts b/packages/blocks/src/root-block/edgeless/edgeless-keyboard.ts index f271cb48f030..13af2e6a3795 100644 --- a/packages/blocks/src/root-block/edgeless/edgeless-keyboard.ts +++ b/packages/blocks/src/root-block/edgeless/edgeless-keyboard.ts @@ -259,7 +259,7 @@ export class EdgelessPageKeyboardManager extends PageKeyboardManager { ...service.blocks .filter( block => - block.group === null && + block.container === null && !( matchFlavours(block, ['affine:note']) && block.displayMode === NoteDisplayMode.DocOnly @@ -267,7 +267,7 @@ export class EdgelessPageKeyboardManager extends PageKeyboardManager { ) .map(block => block.id), ...service.elements - .filter(el => el.group === null) + .filter(el => el.container === null) .map(el => el.id), ], editing: false, diff --git a/packages/blocks/src/root-block/edgeless/edgeless-root-service.ts b/packages/blocks/src/root-block/edgeless/edgeless-root-service.ts index 30de5bc3789b..64ab6c20c4bd 100644 --- a/packages/blocks/src/root-block/edgeless/edgeless-root-service.ts +++ b/packages/blocks/src/root-block/edgeless/edgeless-root-service.ts @@ -28,7 +28,10 @@ import { } from '@blocksuite/affine-model'; import { EditPropsStore } from '@blocksuite/affine-shared/services'; import { clamp } from '@blocksuite/affine-shared/utils'; -import { GfxControllerIdentifier } from '@blocksuite/block-std/gfx'; +import { + GfxControllerIdentifier, + isGfxContainerElm, +} from '@blocksuite/block-std/gfx'; import { BlockSuiteError, ErrorCode } from '@blocksuite/global/exceptions'; import { Bound, getCommonBound, last } from '@blocksuite/global/utils'; import { type BlockModel, Slot } from '@blocksuite/store'; @@ -474,6 +477,12 @@ export class EdgelessRootService extends RootService implements SurfaceContext { id = typeof id === 'string' ? id : id.id; const el = this.getElementById(id); + if (isGfxContainerElm(el)) { + el.childIds.forEach(childId => { + this.removeElement(childId); + }); + } + if (el instanceof GfxBlockModel) { this.doc.deleteBlock(el); return; diff --git a/packages/blocks/src/root-block/widgets/element-toolbar/more-menu/config.ts b/packages/blocks/src/root-block/widgets/element-toolbar/more-menu/config.ts index 36e4c7d04447..8525d22ad86a 100644 --- a/packages/blocks/src/root-block/widgets/element-toolbar/more-menu/config.ts +++ b/packages/blocks/src/root-block/widgets/element-toolbar/more-menu/config.ts @@ -322,6 +322,10 @@ export const conversionsGroup: MenuItemGroup = { elements, title ); + // delete selected elements + doc.transact(() => { + deleteElements(edgeless, elements); + }); // insert linked doc card const width = 364; const height = 390; @@ -335,6 +339,10 @@ export const conversionsGroup: MenuItemGroup = { }, surface.model.id ); + selection.set({ + elements: [cardId], + editing: false, + }); std.getOptional(TelemetryProvider)?.track('CanvasElementAdded', { control: 'context-menu', page: 'whiteboard editor', @@ -355,14 +363,6 @@ export const conversionsGroup: MenuItemGroup = { type: 'embed-linked-doc', other: 'new doc', }); - // delete selected elements - doc.transact(() => { - deleteElements(edgeless, elements); - }); - selection.set({ - elements: [cardId], - editing: false, - }); notifyDocCreated(host, doc); }, diff --git a/tests/edgeless/frame/frame.spec.ts b/tests/edgeless/frame/frame.spec.ts index 97721ff56224..0d95147b6965 100644 --- a/tests/edgeless/frame/frame.spec.ts +++ b/tests/edgeless/frame/frame.spec.ts @@ -9,6 +9,7 @@ import { dragBetweenViewCoords, edgelessCommonSetup, getFirstContainerId, + getSelectedIds, setEdgelessTool, Shape, shiftClickView, @@ -19,11 +20,12 @@ import { import { pressBackspace, pressEscape, - selectAllByKeyboard, SHORT_KEY, } from '../../utils/actions/keyboard.js'; import { + assertCanvasElementsCount, assertContainerChildCount, + assertEdgelessElementBound, assertSelectedBound, } from '../../utils/asserts.js'; import { test } from '../../utils/playwright.js'; @@ -33,8 +35,10 @@ const createFrame = async ( coord1: [number, number], coord2: [number, number] ) => { - await _createFrame(page, coord1, coord2); + const frameId = await _createFrame(page, coord1, coord2); await autoFit(page); + await pressEscape(page); + return frameId; }; test.beforeEach(async ({ page }) => { @@ -122,34 +126,44 @@ test.describe('add element to frame and then move frame', () => { test('element should be moved since it is created in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await createShapeElement(page, [100, 100], [200, 200], Shape.Square); + const frameId = await createFrame(page, [50, 50], [550, 550]); + const shapeId = await createShapeElement( + page, + [100, 100], + [200, 200], + Shape.Square + ); + const noteCoord = await toViewCoord(page, [200, 200]); - await addNote(page, '', noteCoord[0], noteCoord[1]); + const noteId = await addNote(page, '', noteCoord[0], noteCoord[1]); + await pressEscape(page); await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [150, 150, 100, 100], 0); // shape - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame - await assertSelectedBound(page, [220, 210, 448, 92], 2); // note + await assertEdgelessElementBound(page, shapeId, [150, 150, 100, 100]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); + await assertEdgelessElementBound(page, noteId, [220, 210, 448, 92]); }); test('element should be not moved since it is created not in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await createShapeElement(page, [600, 600], [500, 500], Shape.Square); + const frameId = await createFrame(page, [50, 50], [550, 550]); + const shapeId = await createShapeElement( + page, + [600, 600], + [500, 500], + Shape.Square + ); await pressEscape(page); await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [500, 500, 100, 100], 0); // shape - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame + await assertEdgelessElementBound(page, shapeId, [500, 500, 100, 100]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); }); @@ -164,64 +178,77 @@ test.describe('add element to frame and then move frame', () => { test('group should be moved since it is fully contained in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await createShapeElement(page, [100, 100], [200, 200], Shape.Square); - await createShapeElement(page, [150, 150], [250, 250], Shape.Square); + const [frameId, ...shapeIds] = [ + await createFrame(page, [50, 50], [550, 550]), + await createShapeElement(page, [100, 100], [200, 200], Shape.Square), + await createShapeElement(page, [150, 150], [250, 250], Shape.Square), + ]; await pressEscape(page); await shiftClickView(page, [110, 110]); await shiftClickView(page, [160, 160]); await page.keyboard.press(`${SHORT_KEY}+g`); + const groupId = (await getSelectedIds(page))[0]; await pressEscape(page); await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [150, 150, 150, 150], 0); // group - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame + await assertEdgelessElementBound(page, shapeIds[0], [150, 150, 100, 100]); + await assertEdgelessElementBound(page, shapeIds[1], [200, 200, 100, 100]); + await assertEdgelessElementBound(page, groupId, [150, 150, 150, 150]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); + test('group should be moved since its center is in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await createShapeElement(page, [450, 450], [550, 550], Shape.Square); - await createShapeElement(page, [500, 500], [600, 600], Shape.Square); + const [frameId, ...shapeIds] = [ + await createFrame(page, [50, 50], [550, 550]), + await createShapeElement(page, [450, 450], [550, 550], Shape.Square), + await createShapeElement(page, [500, 500], [600, 600], Shape.Square), + ]; await pressEscape(page); await shiftClickView(page, [460, 460]); await shiftClickView(page, [510, 510]); await page.keyboard.press(`${SHORT_KEY}+g`); + const groupId = (await getSelectedIds(page))[0]; await pressEscape(page); await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - // since the new group center is in the frame, so the group is not child of frame - await assertSelectedBound(page, [500, 500, 150, 150], 0); // group - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame + await assertEdgelessElementBound(page, shapeIds[0], [500, 500, 100, 100]); + await assertEdgelessElementBound(page, shapeIds[1], [550, 550, 100, 100]); + await assertEdgelessElementBound(page, groupId, [500, 500, 150, 150]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); test('group should not be moved since its center is not in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await createShapeElement(page, [500, 500], [600, 600], Shape.Square); - await createShapeElement(page, [550, 550], [650, 650], Shape.Square); + const [frameId, ...shapeIds] = [ + await createFrame(page, [50, 50], [550, 550]), + await createShapeElement(page, [500, 500], [600, 600], Shape.Square), + await createShapeElement(page, [550, 550], [650, 650], Shape.Square), + ]; + await pressEscape(page); await shiftClickView(page, [510, 510]); await shiftClickView(page, [560, 560]); await page.keyboard.press(`${SHORT_KEY}+g`); + const groupId = (await getSelectedIds(page))[0]; await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); // since the new group center is not in the frame, so the group is not child of frame - await assertSelectedBound(page, [500, 500, 150, 150], 0); // group - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame + await assertEdgelessElementBound(page, shapeIds[0], [500, 500, 100, 100]); + await assertEdgelessElementBound(page, shapeIds[1], [550, 550, 100, 100]); + await assertEdgelessElementBound(page, groupId, [500, 500, 150, 150]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); }); @@ -229,66 +256,64 @@ test.describe('add element to frame and then move frame', () => { test('the inner frame and its children should be moved since it is fully contained in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await pressEscape(page); - await createFrame(page, [100, 100], [300, 300]); - await pressEscape(page); - await createShapeElement(page, [150, 150], [250, 250], Shape.Square); + const [frameId, innerId, shapeId] = [ + await createFrame(page, [50, 50], [550, 550]), + await createFrame(page, [100, 100], [300, 300]), + await createShapeElement(page, [150, 150], [250, 250], Shape.Square), + ]; await pressEscape(page); await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [200, 200, 100, 100], 0); // shape - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame - await assertSelectedBound(page, [150, 150, 200, 200], 2); // inner frame + await assertEdgelessElementBound(page, shapeId, [200, 200, 100, 100]); + await assertEdgelessElementBound(page, innerId, [150, 150, 200, 200]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); test('the inner frame and its children should be moved since its center is in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await pressEscape(page); - await createFrame(page, [400, 400], [600, 600]); - await pressEscape(page); - await createShapeElement(page, [550, 550], [600, 600], Shape.Square); + const [frameId, innerId, shapeId] = [ + await createFrame(page, [50, 50], [550, 550]), + await createFrame(page, [400, 400], [600, 600]), + await createShapeElement(page, [550, 550], [600, 600], Shape.Square), + ]; await pressEscape(page); await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [600, 600, 50, 50], 0); // shape - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame - await assertSelectedBound(page, [450, 450, 200, 200], 2); // inner frame + await assertEdgelessElementBound(page, shapeId, [600, 600, 50, 50]); + await assertEdgelessElementBound(page, innerId, [450, 450, 200, 200]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); test('the inner frame and its children should also be moved even though its center is not in frame', async ({ page, }) => { - await createFrame(page, [50, 50], [550, 550]); - await pressEscape(page); - await createFrame(page, [500, 500], [600, 600]); - await pressEscape(page); - await createShapeElement(page, [550, 550], [600, 600], Shape.Square); - await pressEscape(page); + const [frameId, innerId, shapeId] = [ + await createFrame(page, [50, 50], [550, 550]), + await createFrame(page, [500, 500], [600, 600]), + await createShapeElement(page, [550, 550], [600, 600], Shape.Square), + ]; await clickView(page, [60, 60]); await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [600, 600, 50, 50], 0); // shape - await assertSelectedBound(page, [100, 100, 500, 500], 1); // frame - await assertSelectedBound(page, [550, 550, 100, 100], 2); // inner frame + await assertEdgelessElementBound(page, shapeId, [600, 600, 50, 50]); + await assertEdgelessElementBound(page, innerId, [550, 550, 100, 100]); + await assertEdgelessElementBound(page, frameId, [100, 100, 500, 500]); }); }); }); test.describe('resize frame then move ', () => { test('resize frame to warp shape', async ({ page }) => { - await createFrame(page, [50, 50], [150, 150]); - await createShapeElement(page, [200, 200], [300, 300], Shape.Square); + const [frameId, shapeId] = [ + await createFrame(page, [50, 50], [150, 150]), + await createShapeElement(page, [200, 200], [300, 300], Shape.Square), + ]; await pressEscape(page); await clickView(page, [60, 60]); @@ -296,14 +321,15 @@ test.describe('resize frame then move ', () => { await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [250, 250, 100, 100], 0); // shape - await assertSelectedBound(page, [100, 100, 400, 400], 1); // frame + await assertEdgelessElementBound(page, shapeId, [250, 250, 100, 100]); + await assertEdgelessElementBound(page, frameId, [100, 100, 400, 400]); }); test('resize frame to unwrap shape', async ({ page }) => { - await createFrame(page, [50, 50], [450, 450]); - await createShapeElement(page, [200, 200], [300, 300], Shape.Square); + const [frameId, shapeId] = [ + await createFrame(page, [50, 50], [450, 450]), + await createShapeElement(page, [200, 200], [300, 300], Shape.Square), + ]; await pressEscape(page); await clickView(page, [60, 60]); @@ -311,9 +337,8 @@ test.describe('resize frame then move ', () => { await dragBetweenViewCoords(page, [60, 60], [110, 110]); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [200, 200, 100, 100], 0); // shape - await assertSelectedBound(page, [100, 100, 100, 100], 1); // frame + await assertEdgelessElementBound(page, shapeId, [200, 200, 100, 100]); + await assertEdgelessElementBound(page, frameId, [100, 100, 100, 100]); }); }); @@ -326,6 +351,5 @@ test('delete frame', async ({ page }) => { await pressBackspace(page); await expect(page.locator('affine-frame')).toHaveCount(0); - await selectAllByKeyboard(page); - await assertSelectedBound(page, [200, 200, 100, 100], 0); + await assertCanvasElementsCount(page, 0); }); diff --git a/tests/edgeless/frame/selection.spec.ts b/tests/edgeless/frame/selection.spec.ts index c179893bac3d..0be4e40d7485 100644 --- a/tests/edgeless/frame/selection.spec.ts +++ b/tests/edgeless/frame/selection.spec.ts @@ -5,13 +5,18 @@ import { addNote, autoFit, createShapeElement, + dragBetweenViewCoords, edgelessCommonSetup, getSelectedBoundCount, Shape, toViewCoord, zoomResetByKeyboard, } from 'utils/actions/edgeless.js'; -import { pressEscape, type } from 'utils/actions/keyboard.js'; +import { + pressEscape, + selectAllByKeyboard, + type, +} from 'utils/actions/keyboard.js'; import { assertEdgelessCanvasText, assertRichTexts, @@ -71,4 +76,23 @@ test.describe('frame selection', () => { await type(page, 'hello'); await assertRichTexts(page, ['hello']); }); + + test('element in frame should not be selected when frame is selected by drag or Cmd/Ctrl + A', async ({ + page, + }) => { + await createFrame(page, [50, 50], [200, 200]); + await createShapeElement(page, [100, 100], [150, 150], Shape.Square); + await pressEscape(page); + + await dragBetweenViewCoords(page, [0, 0], [250, 250]); + expect(await getSelectedBoundCount(page)).toBe(1); + await assertSelectedBound(page, [50, 50, 150, 150]); + + await pressEscape(page); + expect(await getSelectedBoundCount(page)).toBe(0); + + await selectAllByKeyboard(page); + expect(await getSelectedBoundCount(page)).toBe(1); + await assertSelectedBound(page, [50, 50, 150, 150]); + }); }); diff --git a/tests/utils/actions/edgeless.ts b/tests/utils/actions/edgeless.ts index d589656b0118..78af4a2dbdd0 100644 --- a/tests/utils/actions/edgeless.ts +++ b/tests/utils/actions/edgeless.ts @@ -498,6 +498,7 @@ export async function addBasicShapeElement( ) { await setEdgelessTool(page, 'shape', shape); await dragBetweenCoords(page, start, end, { steps: 50 }); + return (await getSelectedIds(page))[0]; } export async function addBasicConnectorElement( @@ -1566,6 +1567,30 @@ export async function getConnectorPath(page: Page, index = 0): Promise { ); } +export async function getEdgelessElementBound( + page: Page, + elementId: string +): Promise<[number, number, number, number]> { + return page.evaluate( + ([elementId]) => { + const container = document.querySelector('affine-edgeless-root'); + if (!container) throw new Error('container not found'); + const element = container.service.getElementById(elementId); + if (!element) throw new Error(`element not found: ${elementId}`); + return JSON.parse(element.xywh); + }, + [elementId] + ); +} + +export async function getSelectedIds(page: Page) { + return page.evaluate(() => { + const container = document.querySelector('affine-edgeless-root'); + if (!container) throw new Error('container not found'); + return container.service.selection.selectedElements.map(e => e.id); + }); +} + export async function getSelectedBoundCount(page: Page) { return page.evaluate(() => { const container = document.querySelector('affine-edgeless-root'); @@ -1727,6 +1752,7 @@ export async function createFrame( ) { await page.keyboard.press('f'); await dragBetweenViewCoords(page, coord1, coord2); + return (await getSelectedIds(page))[0]; } export async function createShapeElement( @@ -1737,12 +1763,13 @@ export async function createShapeElement( ) { const start = await toViewCoord(page, coord1); const end = await toViewCoord(page, coord2); - await addBasicShapeElement( + const shapeId = await addBasicShapeElement( page, { x: start[0], y: start[1] }, { x: end[0], y: end[1] }, shape ); + return shapeId; } export async function createConnectorElement( diff --git a/tests/utils/asserts.ts b/tests/utils/asserts.ts index e5837330594d..9c79310ae929 100644 --- a/tests/utils/asserts.ts +++ b/tests/utils/asserts.ts @@ -25,6 +25,7 @@ import { getContainerChildIds, getContainerIds, getContainerOfElements, + getEdgelessElementBound, getEdgelessSelectedRectModel, getNoteRect, getSelectedBound, @@ -1109,6 +1110,15 @@ export function assertRectExist( expect(rect).not.toBe(null); } +export async function assertEdgelessElementBound( + page: Page, + elementId: string, + bound: Bound +) { + const actual = await getEdgelessElementBound(page, elementId); + assertBound(actual, bound); +} + export async function assertSelectedBound( page: Page, expected: Bound,