Skip to content

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.

Sign up for GitHub

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

Open
ihordubas99 wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Bug/74325 inserting hash inside text removes content after cursor #122

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
699e333
fix: preserve text after cursor when inserting `#` trigger
ihordubas99 Apr 23, 2026
c047379
test: adapt unit tests to TipTap-based clearTriggerText
ihordubas99 Apr 23, 2026
0dd1e54
refactor(editor): simplify hash menu state and use real editor in tests
ihordubas99 Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 34 additions & 16 deletions lib/components/HashMenu/HashWpMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import type { FC, RefObject } from "react";
import { useRef } from "react";
import type { SuggestionMenuProps } from "@blocknote/react";
import styled from "styled-components";
import { BlockCard } from "../BlockWorkPackage/BlockCard";
import { defaultWpVariables } from "../WorkPackage/atoms";
import type { WorkPackage } from "../../openProjectTypes";
import type { HashMenuItem } from "./types";
import type { AnyEditor } from "./editorUtils";
import type { InlineWpSize } from "../WorkPackage/types";
import {
getSizeFromCurrentBlock,
clearTriggerText,
insertWpChip,
insertWpChipIntoBlock,
} from "./editorUtils";

const Menu = styled.div.attrs({ className: "op-bn-hash-menu" })`
Expand Down Expand Up @@ -54,27 +54,48 @@ export function createHashWpMenuComponent(
const searchQuery = items[0]?.title ?? "";
const visibleResults = (resultsRef.current ?? []).slice(0, MAX_RESULTS);

const pendingSizeRef = useRef<InlineWpSize>("xxs");
const originalBlockIdRef = useRef<string | undefined>(undefined);
const savedSelectionRef = useRef<any>(null);

const currentSize = getSizeFromCurrentBlock(editor);
const currentBlockId = editor.getTextCursorPosition()?.block?.id;
Copy link
Copy Markdown
Contributor

@judithroth judithroth Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what reason do we need pendingSizeRef and currentSize and what's the difference?
Same for originalBlockIdRef, savedSelectionRef, currentBlockId

Copy link
Copy Markdown
Collaborator Author

@ihordubas99 ihordubas99 Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, having both was confusing. I've completely removed the redundant useRef hooks (pendingSizeRef, originalBlockIdRef, savedSelectionRef).

The logic is now much simpler: we rely on the closure variables (currentSize, currentBlockId) from the render cycle. This makes the code cleaner and eliminates the need for manual state resets


// 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 = () => {
const size = pendingSizeRef.current !== "xxs"
? pendingSizeRef.current
: currentSize;
const originalBlockId = originalBlockIdRef.current ?? currentBlockId;
const savedSelection = savedSelectionRef.current;

pendingSizeRef.current = "xxs";
Copy link
Copy Markdown
Contributor

@judithroth judithroth Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be covered by const pendingSizeRef = useRef<InlineWpSize>("xxs"); already?

originalBlockIdRef.current = undefined;
savedSelectionRef.current = null;

requestAnimationFrame(() => {
if (!blockId) return;
if (!originalBlockId) return;
editor.focus();

// BlockNote splits the block on Enter - remove the new empty block it created.
const currentBlock = editor.getTextCursorPosition()?.block;
if (currentBlock && currentBlock.id !== blockId) {
if (currentBlock && currentBlock.id !== originalBlockId) {
editor.removeBlocks([currentBlock.id]);
}

clearTriggerText(editor);
insertWpChipIntoBlock(editor, blockId, wp, size);
if (savedSelection) {
const tiptap = (editor as any)._tiptapEditor;
Copy link
Copy Markdown
Contributor

@judithroth judithroth Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nick from BlockNote wrote that they are not sure if they will keep the tiptap APIs (https://matrix.to/#/!yWsguKHvTVyakhzlui:openproject.org/$namCWPK10P-jifVJIjDTaj7bYnnw19XbSC2pSjO8JkY?via=openproject.org&via=matrix.org) and that we should not rely on them.

Is it possible to achieve this without tiptap? If not, why?

Copy link
Copy Markdown
Collaborator Author

@ihordubas99 ihordubas99 Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good news: I removed the TipTap dependency from the UI component (HashWpMenu.tsx). By keeping focus on the editor during a click, we no longer need TipTap to manage the cursor there.

However, for clearTriggerText, using TipTap remains the most reliable approach at this stage. Standard browser tools (DOM) get confused by our custom WorkPackage chips: they count every letter inside the chip, while BlockNote's internal model sees the whole chip as a single item. This "offset mismatch" makes it very risky to delete the # trigger using native methods, as it often leads to deleting the wrong text or data loss.

TipTap is currently the most stable way to find the exact cursor position within BlockNote's structure and ensure we only delete the trigger text

if (tiptap) {
const tr = tiptap.state.tr.setSelection(savedSelection);
tiptap.view.dispatch(tr);
}
}

insertWpChip(editor, wp, size);
});
};
});
Expand Down Expand Up @@ -105,13 +126,10 @@ export function createHashWpMenuComponent(
// cleanup, so we clear the trigger text manually before inserting.
onMouseDown={(e) => {
e.preventDefault();
const size = getSizeFromCurrentBlock(editor);
const blockId = clearTriggerText(editor);
if (blockId) {
editor.focus();
editor.setTextCursorPosition(blockId, "end");
}
insertWpChip(editor, wp, size);
pendingSizeRef.current = getSizeFromCurrentBlock(editor);
originalBlockIdRef.current = editor.getTextCursorPosition()?.block?.id;
savedSelectionRef.current = (editor as any)._tiptapEditor?.state?.selection ?? null;
items[index]?.onItemClick();
}}
>
<BlockCard workPackage={wp} inDropdown />
Expand Down
86 changes: 25 additions & 61 deletions lib/components/HashMenu/editorUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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(
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;
}

/**
Expand All @@ -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);
}
83 changes: 74 additions & 9 deletions test/lib/components/hashMenu.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,88 @@ import {

type FakeContent = { type: string; text?: string; styles?: any; props?: any }[];

function findCursorPos(fullText: string): number {
const match = fullText.match(/#+/);
if (!match || match.index === undefined) return fullText.length;
return match.index + match[0].length;
}

function makeFakeTiptap(block: { id: string; content: FakeContent }) {
const getFullText = () =>
block.content
.filter((n) => n.type === "text")
.map((n) => n.text ?? "")
.join("");

return {
get state() {
const fullText = getFullText();
const cursorPos = findCursorPos(fullText);

return {
selection: {
from: cursorPos,
$from: {
parentOffset: cursorPos,
parent: {
textBetween(start: number, end: number) {
const text = getFullText();
const absStart = Math.max(0, cursorPos - (end - start));
return text.slice(absStart, cursorPos);
},
},
},
},
tr: {
delete(start: number, end: number) {
return { _start: start, _end: end };
},
},
};
},
view: {
dispatch(tr: { _start: number; _end: number }) {
const fullText = getFullText();
const hashIndex = fullText.search(/#+/);
const newText = hashIndex <= 0 ? "" : fullText.slice(0, hashIndex);

if (newText === "") {
block.content = [];
} else {
const firstTextNode = block.content.find((n) => n.type === "text");
const nonTextNodes = block.content.filter((n) => n.type !== "text");
block.content = [
...(firstTextNode ? [{ ...firstTextNode, text: newText }] : []),
...nonTextNodes,
];
}
},
},
};
}

function makeFakeEditor(content: FakeContent = []) {
let block = { id: "block-1", content };
const block = { id: "block-1", content: [...content] };
const inserted: FakeContent = [];

return {
const editor: any = {
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);
}
if (id === block.id) block.content = update.content;
},
insertInlineContent: (content: FakeContent) => {
inserted.push(...content);
},
focus: vi.fn(),
setTextCursorPosition: vi.fn(),
};

editor._tiptapEditor = makeFakeTiptap(block);
Copy link
Copy Markdown
Contributor

@judithroth judithroth Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to use a real BlockNote editor for the tests. Using a fake TipTap here could make these tests useless in the future - if BlockNote decides to remove some TipTap functionality, these tests could still be green but the functionality be broken in the "real world", because the tests are using TipTap directly instead of BlockNote.


return editor;
}

describe("getSizeFromCurrentBlock", () => {
Expand Down Expand Up @@ -65,7 +126,11 @@ describe("clearTriggerText", () => {
});

it("does nothing if no block", () => {
const editor = { getTextCursorPosition: () => null, updateBlock: vi.fn() };
const editor = {
_tiptapEditor: null,
getTextCursorPosition: () => null,
updateBlock: vi.fn(),
};
expect(clearTriggerText(editor as any)).toBeNull();
});

Expand Down Expand Up @@ -98,8 +163,8 @@ describe("insertWpChipIntoBlock", () => {
expect(inserted.length).toBe(2);

const chip = inserted.find(
(c): c is { type: string; props: { size: string } } =>
c.type === "openProjectWorkPackageInline" && c.props?.size
(item: any): item is { type: string; props: { size: string } } =>
Copy link
Copy Markdown
Contributor

@judithroth judithroth Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for no longer using abbreviations here 🤩

item.type === "openProjectWorkPackageInline" && item.props?.size
);
expect(chip).toBeDefined();
expect(chip?.props.size).toBe("xxs");
Expand Down
14 changes: 7 additions & 7 deletions test/lib/components/integration/editor.inlineChip.browser.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('Inline chip - insert', () => {
renderEditor();
await insertInlineChipViaSlashMenu();

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
Copy link
Copy Markdown
Contributor

@judithroth judithroth Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering why this change is necessary?

Copy link
Copy Markdown
Collaborator Author

@ihordubas99 ihordubas99 Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially added .first() to avoid strict-mode errors because the menu and the new chip were present at the same time. However, after simplifying the focus logic and refactoring the component, the tests now pass reliably without it. I've reverted this change to keep the tests clean

await expect.element(page.getByTestId('op-bn-work-package--type')).toBeVisible();
await expect.element(page.getByText('In Progress')).toBeVisible();
await expect.element(page.getByText('Fix login bug')).toBeVisible();
Expand All @@ -23,7 +23,7 @@ describe('Inline chip - insert', () => {
renderEditor();
await insertInlineChipViaHash('#');

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
await expect.element(page.getByTestId('op-bn-work-package--type')).not.toBeInTheDocument();
await expect.element(page.getByText('In Progress')).not.toBeInTheDocument();
});
Expand All @@ -32,7 +32,7 @@ describe('Inline chip - insert', () => {
renderEditor();
await insertInlineChipViaHash('##');

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
await expect.element(page.getByTestId('op-bn-work-package--type')).toBeVisible();
await expect.element(page.getByText('In Progress')).not.toBeInTheDocument();
});
Expand All @@ -41,7 +41,7 @@ describe('Inline chip - insert', () => {
renderEditor();
await insertInlineChipViaHash('###');

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
await expect.element(page.getByTestId('op-bn-work-package--type')).toBeVisible();
await expect.element(page.getByText('In Progress')).toBeVisible();
});
Expand All @@ -55,7 +55,7 @@ describe('Inline chip - resize', () => {
await openInlineChipSizeMenu();
await userEvent.click(page.getByRole('button', { name: 'Tiny (inline)', exact: true }));

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
await expect.element(page.getByTestId('op-bn-work-package--type')).not.toBeInTheDocument();
await expect.element(page.getByText('In Progress')).not.toBeInTheDocument();
await expect.element(page.getByText('Fix login bug')).not.toBeInTheDocument();
Expand All @@ -68,7 +68,7 @@ describe('Inline chip - resize', () => {
await openInlineChipSizeMenu();
await userEvent.click(page.getByRole('button', { name: 'Compact (inline)', exact: true }));

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
await expect.element(page.getByTestId('op-bn-work-package--type')).toBeVisible();
await expect.element(page.getByText('In Progress')).not.toBeInTheDocument();
await expect.element(page.getByText('Fix login bug')).toBeVisible();
Expand All @@ -81,7 +81,7 @@ describe('Inline chip - resize', () => {
await openInlineChipSizeMenu();
await userEvent.click(page.getByRole('button', { name: 'Tiny (inline)', exact: true }));

await expect.element(page.getByText('#123')).toBeVisible();
await expect.element(page.getByText('#123').first()).toBeVisible();
await expect.element(page.getByTestId('op-bn-work-package--type')).not.toBeInTheDocument();
await expect.element(page.getByText('In Progress')).not.toBeInTheDocument();
await expect.element(page.getByText('Fix login bug')).not.toBeInTheDocument();
Expand Down
Loading
⚡