Skip to content

Commit

Permalink
fix(editor): should delete collapsed siblings when delete heading (#9376
Browse files Browse the repository at this point in the history
)
  • Loading branch information
Flrande committed Dec 30, 2024
1 parent 406460a commit 4fee0e2
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 113 deletions.
1 change: 1 addition & 0 deletions blocksuite/affine/block-list/src/list-block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export class ListBlockComponent extends CaptionedBlockComponent<ListBlockModel>

private readonly _onClickIcon = (e: MouseEvent) => {
e.stopPropagation();
e.preventDefault();

if (this.model.type === 'toggle') {
if (this.doc.readonly) {
Expand Down
101 changes: 37 additions & 64 deletions blocksuite/blocks/src/root-block/keyboard/keyboard-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ import {
notifyDocCreated,
promptDocTitle,
} from '@blocksuite/affine-block-embed';
import { ParagraphBlockComponent } from '@blocksuite/affine-block-paragraph';
import { matchFlavours } from '@blocksuite/affine-shared/utils';
import type {
BlockComponent,
BlockSelection,
UIEventHandler,
} from '@blocksuite/block-std';
import type { BlockComponent, UIEventHandler } from '@blocksuite/block-std';
import { IS_MAC, IS_WINDOWS } from '@blocksuite/global/env';
import { assertExists } from '@blocksuite/global/utils';

export class PageKeyboardManager {
private readonly _handleDelete: UIEventHandler = ctx => {
Expand All @@ -24,26 +20,43 @@ export class PageKeyboardManager {
}

event.preventDefault();
this._doc.transact(() => {
const selection = this._replaceBlocksBySelection(
blockSelections,
'affine:paragraph',
{}
);

if (selection) {
this._selection.setGroup('note', [
this._selection.create('text', {
from: {
index: 0,
length: 0,
blockId: selection.blockId,
},
to: null,
}),
]);

const deletedBlocks: string[] = [];
blockSelections.forEach(sel => {
const id = sel.blockId;
const block = this._doc.getBlock(id);
if (!block) return;
const model = block.model;

if (
matchFlavours(model, ['affine:paragraph']) &&
model.type.startsWith('h') &&
model.collapsed
) {
const component = this.rootComponent.host.view.getBlock(id);
if (!(component instanceof ParagraphBlockComponent)) return;
const collapsedSiblings = component.collapsedSiblings;

deletedBlocks.push(
...[id, ...collapsedSiblings.map(sibling => sibling.id)].filter(
id => !deletedBlocks.includes(id)
)
);
} else {
deletedBlocks.push(id);
}
});

this._doc.transact(() => {
deletedBlocks.forEach(id => {
const block = this._doc.getBlock(id);
if (block) {
this._doc.deleteBlock(block.model);
}
});

this._selection.clear(['block', 'text']);
});
};

private get _currentSelection() {
Expand Down Expand Up @@ -135,44 +148,4 @@ export class PageKeyboardManager {
})
.catch(console.error);
}

private _deleteBlocksBySelection(selections: BlockSelection[]) {
selections.forEach(selection => {
const block = this._doc.getBlockById(selection.blockId);
if (block) {
this._doc.deleteBlock(block);
}
});
}

private _replaceBlocksBySelection(
selections: BlockSelection[],
flavour: string,
props: Record<string, unknown>
) {
const current = selections[0];
const first = this._doc.getBlockById(current.blockId);
const firstElement = this.rootComponent.host.view.getBlock(current.blockId);

assertExists(first, `Cannot find block ${current.blockId}`);
assertExists(firstElement, `Cannot find block view ${current.blockId}`);

const parent = this._doc.getParent(first);
const index = parent?.children.indexOf(first);

this._deleteBlocksBySelection(selections);

try {
this._doc.schema.validate(flavour, parent?.flavour);
} catch {
return null;
}

const blockId = this._doc.addBlock(flavour as never, props, parent, index);

return {
blockId,
path: blockId,
};
}
}
7 changes: 1 addition & 6 deletions blocksuite/tests-legacy/attachment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,12 +481,7 @@ test('should attachment can be deleted', async ({ page }) => {
prop:hidden={false}
prop:index="a0"
prop:lockedBySelf={false}
>
<affine:paragraph
prop:collapsed={false}
prop:type="text"
/>
</affine:note>`,
/>`,
noteId
);
});
Expand Down
2 changes: 1 addition & 1 deletion blocksuite/tests-legacy/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ test('should sync selected-blocks to session-manager when clicking drag handle',
await handle.click();

await page.keyboard.press('Backspace');
await assertRichTexts(page, ['', '456', '789']);
await assertRichTexts(page, ['456', '789']);
});

test.fixme(
Expand Down
46 changes: 21 additions & 25 deletions blocksuite/tests-legacy/selection/block.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ test('block level range delete', async ({ page }) => {

await dragBetweenCoords(page, below789, above123);
await pressBackspace(page);
await assertBlockCount(page, 'paragraph', 1);
await assertRichTexts(page, ['']);
await assertBlockCount(page, 'paragraph', 0);
await assertRichTexts(page, []);

await waitNextFrame(page);
await undoByClick(page);
await assertRichTexts(page, ['123', '456', '789']);

await redoByClick(page);
await assertRichTexts(page, ['']);
await assertRichTexts(page, []);
});

test('block level range delete by forwardDelete', async ({ page }) => {
Expand All @@ -86,15 +86,15 @@ test('block level range delete by forwardDelete', async ({ page }) => {
await dragBetweenCoords(page, below789, above123);
await pressForwardDelete(page);
await waitNextFrame(page);
await assertBlockCount(page, 'paragraph', 1);
await assertRichTexts(page, ['']);
await assertBlockCount(page, 'paragraph', 0);
await assertRichTexts(page, []);

await waitNextFrame(page);
await undoByClick(page);
await assertRichTexts(page, ['123', '456', '789']);

await redoByClick(page);
await assertRichTexts(page, ['']);
await assertRichTexts(page, []);
});

// XXX: Doesn't simulate full user operation due to backspace cursor issue in Playwright.
Expand All @@ -108,9 +108,8 @@ test('select all and delete', async ({ page }) => {
await selectAllByKeyboard(page);
await shamefullyBlurActiveElement(page);
await pressBackspace(page);
await focusRichText(page, 0);
await type(page, 'abc');
await assertRichTexts(page, ['abc']);
await assertBlockCount(page, 'paragraph', 0);
await assertRichTexts(page, []);
});

test('select all and delete by forwardDelete', async ({ page }) => {
Expand All @@ -123,9 +122,8 @@ test('select all and delete by forwardDelete', async ({ page }) => {
await selectAllByKeyboard(page);
await shamefullyBlurActiveElement(page);
await pressForwardDelete(page);
await focusRichText(page, 0);
await type(page, 'abc');
await assertRichTexts(page, ['abc']);
await assertBlockCount(page, 'paragraph', 0);
await assertRichTexts(page, []);
});

test('select all should work for multiple notes in doc mode', async ({
Expand All @@ -148,7 +146,13 @@ test('select all should work for multiple notes in doc mode', async ({

async function clickListIcon(page: Page, i = 0) {
const locator = page.locator('.affine-list-block__prefix').nth(i);
await locator.click({ force: true });
await locator.click({
force: true,
position: {
x: 2,
y: 2,
},
});
}

test('click the list icon can select and copy', async ({ page }) => {
Expand Down Expand Up @@ -179,14 +183,10 @@ test('click the list icon can select and delete', async ({ page }) => {
await initThreeLists(page);
await assertRichTexts(page, ['123', '456', '789']);

await clickListIcon(page, 0);
await waitNextFrame(page);
await pressBackspace(page);
await assertRichTexts(page, ['', '456', '789']);
await clickListIcon(page, 0);
await clickListIcon(page, 1);
await waitNextFrame(page);
await pressBackspace(page);
await assertRichTexts(page, ['', '']);
await assertRichTexts(page, ['123']);
});

test('click the list icon can select and delete by forwardDelete', async ({
Expand All @@ -197,14 +197,10 @@ test('click the list icon can select and delete by forwardDelete', async ({
await initThreeLists(page);
await assertRichTexts(page, ['123', '456', '789']);

await clickListIcon(page, 0);
await waitNextFrame(page);
await pressForwardDelete(page);
await assertRichTexts(page, ['', '456', '789']);
await clickListIcon(page, 0);
await clickListIcon(page, 1);
await waitNextFrame(page);
await pressForwardDelete(page);
await assertRichTexts(page, ['', '']);
await assertRichTexts(page, ['123']);
});

test('selection on heavy page', async ({ page }) => {
Expand Down
27 changes: 10 additions & 17 deletions blocksuite/tests-legacy/selection/native.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ test('native range delete by forwardDelete', async ({ page }) => {
await assertRichTexts(page, ['123', '456', '789']);

const box123 = await getRichTextBoundingBox(page, '2');
const inside123 = { x: box123.left - 1, y: box123.top + 1 };
const inside123 = { x: box123.left + 1, y: box123.top + 1 };

const box789 = await getRichTextBoundingBox(page, '4');
const inside789 = { x: box789.right - 1, y: box789.bottom - 1 };
Expand All @@ -180,7 +180,7 @@ test('native range input', async ({ page }) => {
await assertRichTexts(page, ['123', '456', '789']);

const box123 = await getRichTextBoundingBox(page, '2');
const inside123 = { x: box123.left - 1, y: box123.top + 1 };
const inside123 = { x: box123.left + 1, y: box123.top + 1 };

const box789 = await getRichTextBoundingBox(page, '4');
const inside789 = { x: box789.right - 1, y: box789.bottom - 1 };
Expand All @@ -200,10 +200,10 @@ test('native range selection backwards', async ({ page }) => {
await assertRichTexts(page, ['123', '456', '789']);

const box123 = await getRichTextBoundingBox(page, '2');
const above123 = { x: box123.left, y: box123.top - 2 };
const above123 = { x: box123.left + 1, y: box123.top + 1 };

const box789 = await getRichTextBoundingBox(page, '4');
const bottomRight789 = { x: box789.right, y: box789.bottom };
const bottomRight789 = { x: box789.right - 1, y: box789.bottom - 1 };

// from bottom to top
await dragBetweenCoords(page, bottomRight789, above123, { steps: 10 });
Expand All @@ -230,7 +230,7 @@ test('native range selection backwards by forwardDelete', async ({ page }) => {
const above123 = { x: box123.left, y: box123.top - 2 };

const box789 = await getRichTextBoundingBox(page, '4');
const bottomRight789 = { x: box789.right, y: box789.bottom };
const bottomRight789 = { x: box789.right - 1, y: box789.bottom - 1 };

// from bottom to top
await dragBetweenCoords(page, bottomRight789, above123, { steps: 10 });
Expand Down Expand Up @@ -390,7 +390,7 @@ test('select all text with dragging and delete', async ({ page }) => {
await initThreeParagraphs(page);
await assertRichTexts(page, ['123', '456', '789']);

await dragBetweenIndices(page, [0, 0], [2, 3], undefined, undefined, {
await dragBetweenIndices(page, [0, 0], [2, 3], { x: 1, y: 1 }, undefined, {
steps: 20,
});
await pressBackspace(page);
Expand Down Expand Up @@ -434,14 +434,7 @@ test('select all text with keyboard delete', async ({ page }) => {
await selectAllByKeyboard(page);
await selectAllByKeyboard(page);
await pressBackspace(page);
await assertRichTexts(page, ['', '456', '789']);

await type(page, 'abc');
await selectAllByKeyboard(page);
await selectAllByKeyboard(page);
await selectAllByKeyboard(page);
await pressBackspace(page);
await assertRichTexts(page, ['']);
await assertRichTexts(page, ['456', '789']);
});

test('select text leaving a few words in the last line and delete', async ({
Expand Down Expand Up @@ -801,7 +794,7 @@ test('Delete the second divider between two dividers by forwardDelete', async ({
await pressArrowUp(page);
await pressForwardDelete(page);
await assertDivider(page, 1);
await assertRichTexts(page, ['', '', '']);
await assertRichTexts(page, ['', '']);
});

test('should delete line with content after divider not lose content', async ({
Expand All @@ -818,7 +811,7 @@ test('should delete line with content after divider not lose content', async ({
await waitNextFrame(page);
await pressBackspace(page, 2);
await assertDivider(page, 0);
await assertRichTexts(page, ['', '123']);
await assertRichTexts(page, ['123']);
});

test('should forwardDelete divider works properly', async ({ page }) => {
Expand All @@ -835,7 +828,7 @@ test('should forwardDelete divider works properly', async ({ page }) => {
await page.keyboard.press(`${SHORT_KEY}+ArrowRight`, { delay: 50 });
await pressForwardDelete(page);
await assertDivider(page, 0);
await assertRichTexts(page, ['123', '', '']);
await assertRichTexts(page, ['123', '']);
});

test('the cursor should move to closest editor block when clicking outside container', async ({
Expand Down

0 comments on commit 4fee0e2

Please sign in to comment.