-
Notifications
You must be signed in to change notification settings - Fork 2
Bug/74325 inserting hash inside text removes content after cursor #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,9 +8,7 @@ import type { HashMenuItem } from "./types"; | |
| import type { AnyEditor } from "./editorUtils"; | ||
| import { | ||
| getSizeFromCurrentBlock, | ||
| clearTriggerText, | ||
| insertWpChip, | ||
| insertWpChipIntoBlock, | ||
| } from "./editorUtils"; | ||
|
|
||
| const Menu = styled.div.attrs({ className: "op-bn-hash-menu" })` | ||
|
|
@@ -54,27 +52,31 @@ export function createHashWpMenuComponent( | |
| const searchQuery = items[0]?.title ?? ""; | ||
| const visibleResults = (resultsRef.current ?? []).slice(0, MAX_RESULTS); | ||
|
|
||
| const currentSize = getSizeFromCurrentBlock(editor); | ||
| const currentBlockId = editor.getTextCursorPosition()?.block?.id; | ||
|
|
||
| // Mutate each item's onItemClick so BlockNote's keyboard handler | ||
| // (Enter / PgUp / PgDn) calls the correct insertion for that result. | ||
| visibleResults.forEach((wp, index) => { | ||
| if (!items[index]) return; | ||
|
|
||
| const size = getSizeFromCurrentBlock(editor); | ||
| const blockId = editor.getTextCursorPosition()?.block?.id; | ||
|
|
||
| items[index].onItemClick = () => { | ||
| requestAnimationFrame(() => { | ||
| if (!blockId) return; | ||
| editor.focus(); | ||
| if (!currentBlockId) return; | ||
|
|
||
| // BlockNote splits the block on Enter - remove the new empty block it created. | ||
| const currentBlock = editor.getTextCursorPosition()?.block; | ||
| if (currentBlock && currentBlock.id !== blockId) { | ||
|
|
||
| // BlockNote's default "Enter" behavior splits the block. | ||
| // If the block ID changed, it was a keyboard Enter, so we remove the new empty block. | ||
| const isKeyboardEnter = currentBlock && currentBlock.id !== currentBlockId; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we simplify this by using something like |
||
|
|
||
| if (isKeyboardEnter) { | ||
| editor.removeBlocks([currentBlock.id]); | ||
| } | ||
|
|
||
| clearTriggerText(editor); | ||
| insertWpChipIntoBlock(editor, blockId, wp, size); | ||
| editor.focus(); | ||
|
|
||
| insertWpChip(editor, wp, currentSize); | ||
| }); | ||
| }; | ||
| }); | ||
|
|
@@ -101,17 +103,11 @@ export function createHashWpMenuComponent( | |
| <MenuItem | ||
| key={wp.id} | ||
| $selected={selectedIndex === index} | ||
| // Mouse path: e.preventDefault() stops BlockNote from doing its own | ||
| // cleanup, so we clear the trigger text manually before inserting. | ||
| // Mouse path: e.preventDefault() prevents the editor from losing focus. | ||
| // This allows us to safely insert the chip without needing TipTap to restore the cursor. | ||
| onMouseDown={(e) => { | ||
| e.preventDefault(); | ||
| const size = getSizeFromCurrentBlock(editor); | ||
| const blockId = clearTriggerText(editor); | ||
| if (blockId) { | ||
| editor.focus(); | ||
| editor.setTextCursorPosition(blockId, "end"); | ||
| } | ||
| insertWpChip(editor, wp, size); | ||
| items[index]?.onItemClick(); | ||
| }} | ||
| > | ||
| <BlockCard workPackage={wp} inDropdown /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ export function getSizeFromCurrentBlock(editor: AnyEditor): InlineWpSize { | |
| for (const node of content) { | ||
| if (node.type !== "text") continue; | ||
| const text = node.text as string; | ||
| const match = text.match(/(#+)[^#]/); | ||
| const match = text.match(/(#+)/); | ||
| if (match) { | ||
| const hashCount = match[1].length; | ||
| if (hashCount >= 3) return "s"; | ||
|
|
@@ -34,43 +34,30 @@ export function getSizeFromCurrentBlock(editor: AnyEditor): InlineWpSize { | |
| return "xxs"; | ||
| } | ||
|
|
||
| /** | ||
| * Removes the # trigger text (and any extra # symbols) from the current block. | ||
| * BlockNote removes #query itself on Enter, but may leave extra # characters | ||
| * (e.g. ## from ###query). Returns the block id, or null if nothing was found. | ||
| */ | ||
| export function clearTriggerText(editor: AnyEditor): string | null { | ||
| const block = editor.getTextCursorPosition()?.block; | ||
| if (!block) return null; | ||
|
|
||
| const content = (block.content ?? []) as any[]; | ||
|
|
||
| const triggerNodeIndex = content.findIndex((n) => { | ||
| if (n.type !== "text") return false; | ||
| return /#+/.test(n.text as string); | ||
| }); | ||
| const tiptap = (editor as any)._tiptapEditor; | ||
| if (!tiptap) return null; | ||
|
|
||
| if (triggerNodeIndex === -1) return null; | ||
| const { state, view } = tiptap; | ||
| const { selection } = state; | ||
| const { $from, from } = selection; | ||
|
|
||
| const triggerNode = content[triggerNodeIndex] as { type: string; text: string; styles: any }; | ||
| const text = triggerNode.text; | ||
| const hashIndex = text.search(/#/); | ||
| const textBefore = hashIndex > 0 ? text.slice(0, hashIndex) : null; | ||
| const textBefore = $from.parent.textBetween( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using TipTap here, would it be possible to use BlockNote's If that is not possible I am okay with keeping TipTap, but at the moment I don't see why this approach should cause problems and why we necessarily need TipTap here 😅 |
||
| Math.max(0, $from.parentOffset - 50), | ||
| $from.parentOffset, | ||
| undefined, | ||
| "\n" | ||
| ); | ||
|
|
||
| const cleanedContent = [ | ||
| ...content.slice(0, triggerNodeIndex), | ||
| ...(textBefore ? [{ type: "text", text: textBefore, styles: triggerNode.styles }] : []), | ||
| ]; | ||
| const match = textBefore.match(/(#+\S*)$/); | ||
| if (!match) return null; | ||
|
|
||
| editor.updateBlock(block.id, { content: cleanedContent } as any); | ||
| return block.id; | ||
| } | ||
| const triggerLength = match[1].length; | ||
| const tr = state.tr.delete(from - triggerLength, from); | ||
| view.dispatch(tr); | ||
|
|
||
| function focusAndMoveToEnd(editor: AnyEditor, blockId: string): void { | ||
| requestAnimationFrame(() => { | ||
| editor.focus(); | ||
| editor.setTextCursorPosition(blockId, "end"); | ||
| }); | ||
| const block = (editor as any).getTextCursorPosition()?.block; | ||
| return block?.id ?? null; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -80,43 +67,20 @@ function focusAndMoveToEnd(editor: AnyEditor, blockId: string): void { | |
| export function insertWpChip(editor: AnyEditor, wp: WorkPackage, size: InlineWpSize): void { | ||
| const instanceId = makeInstanceId(); | ||
|
|
||
| clearTriggerText(editor); | ||
|
|
||
| (editor.insertInlineContent as (content: unknown[]) => void)([ | ||
| { type: "openProjectWorkPackageInline", props: { wpid: String(wp.id), instanceId, size } }, | ||
| { type: "text", text: " ", styles: {} }, | ||
| ]); | ||
|
|
||
| requestAnimationFrame(() => { | ||
| editor.focus(); | ||
| const cursor = editor.getTextCursorPosition(); | ||
| if (cursor?.block?.id) { | ||
| editor.setTextCursorPosition(cursor.block.id, "end"); | ||
| } | ||
| }); | ||
| editor.focus(); | ||
| } | ||
| /** | ||
| * Keyboard (Enter) path: inserts chip directly into block content by ID, | ||
| * bypassing cursor position entirely to avoid race conditions with | ||
| * BlockNote's Enter handling which moves the cursor to a new block. | ||
| */ | ||
| export function insertWpChipIntoBlock( | ||
| editor: AnyEditor, | ||
| blockId: string, | ||
| _blockId: string, | ||
| wp: WorkPackage, | ||
| size: InlineWpSize, | ||
| size: InlineWpSize | ||
| ): void { | ||
| const instanceId = makeInstanceId(); | ||
| const block = editor.getBlock(blockId); | ||
| if (!block) return; | ||
|
|
||
| const content = (block.content ?? []) as any[]; | ||
|
|
||
| editor.updateBlock(blockId, { | ||
| content: [ | ||
| ...content, | ||
| { type: "openProjectWorkPackageInline", props: { wpid: String(wp.id), instanceId, size } }, | ||
| { type: "text", text: " ", styles: {} }, | ||
| ], | ||
| } as any); | ||
|
|
||
| focusAndMoveToEnd(editor, blockId); | ||
| insertWpChip(editor, wp, size); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding all those tests! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,114 +1,107 @@ | ||
| // @vitest-environment jsdom | ||
| import { describe, it, expect, vi } from "vitest"; | ||
| import { BlockNoteEditor } from "@blocknote/core"; | ||
| import { | ||
| getSizeFromCurrentBlock, | ||
| insertWpChipIntoBlock, | ||
| clearTriggerText, | ||
| } from "../../../lib/components/HashMenu/editorUtils"; | ||
|
|
||
| type FakeContent = { type: string; text?: string; styles?: any; props?: any }[]; | ||
|
|
||
| function makeFakeEditor(content: FakeContent = []) { | ||
| let block = { id: "block-1", content }; | ||
| const inserted: FakeContent = []; | ||
|
|
||
| return { | ||
| block, | ||
| inserted, | ||
| getTextCursorPosition: () => ({ block }), | ||
| getBlock: (id: string) => (id === block.id ? block : null), | ||
| updateBlock: (id: string, update: { content: FakeContent }) => { | ||
| if (id === block.id) { | ||
| block.content = update.content; | ||
| inserted.push(...update.content); | ||
| } | ||
| }, | ||
| insertInlineContent: (content: FakeContent) => { | ||
| inserted.push(...content); | ||
| }, | ||
| focus: vi.fn(), | ||
| setTextCursorPosition: vi.fn(), | ||
| }; | ||
| function createTestEditor(text: string) { | ||
| const editor = BlockNoteEditor.create({ | ||
| initialContent: [{ type: "paragraph", content: text }], | ||
| }); | ||
|
|
||
| const block = editor.document[0]; | ||
| editor.setTextCursorPosition(block, "end"); | ||
|
|
||
| return editor; | ||
| } | ||
|
|
||
| describe("getSizeFromCurrentBlock", () => { | ||
| it("returns xxs for #", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "#foo" }]); | ||
| const editor = createTestEditor("#foo"); | ||
| expect(getSizeFromCurrentBlock(editor as any)).toBe("xxs"); | ||
| }); | ||
|
|
||
| it("returns xs for ##", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "##foo" }]); | ||
| const editor = createTestEditor("##foo"); | ||
| expect(getSizeFromCurrentBlock(editor as any)).toBe("xs"); | ||
| }); | ||
|
|
||
| it("returns s for ### or more", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "###foo" }]); | ||
| const editor = createTestEditor("###foo"); | ||
| expect(getSizeFromCurrentBlock(editor as any)).toBe("s"); | ||
|
|
||
| const editor2 = makeFakeEditor([{ type: "text", text: "####foo" }]); | ||
| const editor2 = createTestEditor("####foo"); | ||
| expect(getSizeFromCurrentBlock(editor2 as any)).toBe("s"); | ||
| }); | ||
|
|
||
| it("returns xxs if no hashes", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "foo" }]); | ||
| const editor = createTestEditor("foo"); | ||
| expect(getSizeFromCurrentBlock(editor as any)).toBe("xxs"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("clearTriggerText", () => { | ||
| it("removes # text and returns block id", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "#foo" }]); | ||
| const editor = createTestEditor("#foo"); | ||
| const blockId = clearTriggerText(editor as any); | ||
| expect(blockId).toBe("block-1"); | ||
| expect(editor.block.content).toEqual([]); | ||
|
|
||
| expect(blockId).toBe(editor.document[0].id); | ||
| const block = editor.getBlock(editor.document[0].id); | ||
| expect(block?.content).toEqual([]); | ||
| }); | ||
|
|
||
| it("does nothing if no block", () => { | ||
| const editor = { getTextCursorPosition: () => null, updateBlock: vi.fn() }; | ||
| const editor = { | ||
| _tiptapEditor: null, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using TipTap, please try to create an empty BlockNote editor or just remove the contents from the one you create with |
||
| getTextCursorPosition: () => null, | ||
| updateBlock: vi.fn(), | ||
| }; | ||
| expect(clearTriggerText(editor as any)).toBeNull(); | ||
| }); | ||
|
|
||
| it("keeps text before # and removes trigger", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "Hello #foo" }]); | ||
| const editor = createTestEditor("Hello #foo"); | ||
| const blockId = clearTriggerText(editor as any); | ||
| expect(blockId).toBe("block-1"); | ||
| expect(editor.block.content).toEqual([{ type: "text", text: "Hello " }]); | ||
|
|
||
| expect(blockId).toBe(editor.document[0].id); | ||
| const block = editor.getBlock(editor.document[0].id); | ||
| expect((block?.content as any)[0].text).toBe("Hello "); | ||
| }); | ||
|
|
||
| it("works with multiple # in the text", () => { | ||
| const editor = makeFakeEditor([{ type: "text", text: "Pre #one #two #three" }]); | ||
| const editor = createTestEditor("Pre #one #two #three"); | ||
| const blockId = clearTriggerText(editor as any); | ||
| expect(blockId).toBe("block-1"); | ||
| expect(editor.block.content).toEqual([{ type: "text", text: "Pre " }]); | ||
|
|
||
| expect(blockId).toBe(editor.document[0].id); | ||
| const block = editor.getBlock(editor.document[0].id); | ||
|
|
||
| expect((block?.content as any)[0].text).toBe("Pre #one #two "); | ||
| }); | ||
| }); | ||
|
|
||
| describe("insertWpChipIntoBlock", () => { | ||
| it("adds a chip to the block content", () => { | ||
| const editor = makeFakeEditor([]); | ||
| const editor = createTestEditor("test"); | ||
|
|
||
| const insertSpy = vi.spyOn(editor, "insertInlineContent").mockImplementation(() => {}); | ||
| vi.spyOn(editor, "focus").mockImplementation(() => {}); | ||
|
|
||
| insertWpChipIntoBlock( | ||
| editor as any, | ||
| "block-1", | ||
| editor.document[0].id, | ||
| { id: 1, subject: "Fix bug" } as any, | ||
| "xxs" | ||
| ); | ||
|
|
||
| const inserted = editor.inserted; | ||
| expect(inserted.length).toBe(2); | ||
|
|
||
| const chip = inserted.find( | ||
| (c): c is { type: string; props: { size: string } } => | ||
| c.type === "openProjectWorkPackageInline" && c.props?.size | ||
| ); | ||
| expect(chip).toBeDefined(); | ||
| expect(chip?.props.size).toBe("xxs"); | ||
| }); | ||
|
|
||
| it("does nothing if block not found", () => { | ||
| const editor = makeFakeEditor([]); | ||
| expect(() => | ||
| insertWpChipIntoBlock(editor as any, "wrong-id", { id: 1 } as any, "xxs") | ||
| ).not.toThrow(); | ||
| expect(insertSpy).toHaveBeenCalledWith([ | ||
| { | ||
| type: "openProjectWorkPackageInline", | ||
| props: { wpid: "1", instanceId: expect.any(String), size: "xxs" }, | ||
| }, | ||
| { type: "text", text: " ", styles: {} }, | ||
| ]); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.