Skip to content

Commit 2990657

Browse files
YousefEDclaude
andcommitted
feat(core): allow comment & suggestion marks on plain blocks
"plain" blocks (e.g. code blocks) previously disallowed all marks (`marks: ""`), which also blocked comments and suggestions/diffs. Allow the non-formatting marks while still excluding formatting. - Add a shared "annotation" mark group to the comment mark and the suggestion marks (insertion/deletion/modification), and reference that group from plain blocks' node spec. The always-present suggestion marks keep the group non-empty, so the reference resolves even when comments aren't configured (the comment mark is conditional). - Update the stripDisallowedMarks migration to keep these non-formatting marks and strip only the disallowed formatting marks, so legacy code blocks retain their comments/suggestions. - Tests: code block allows insertion/deletion/modification (and comment when comments are enabled) but not bold; migration keeps comment + insertion while stripping bold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3e590bd commit 2990657

7 files changed

Lines changed: 150 additions & 16 deletions

File tree

packages/core/src/comments/mark.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import { Mark, mergeAttributes } from "@tiptap/core";
22

3+
import { NON_FORMATTING_MARK_GROUP } from "../schema/markGroups.js";
4+
35
export const CommentMark = Mark.create({
46
name: "comment",
57
excludes: "",
68
inclusive: false,
79
keepOnSplit: true,
10+
// Allowed on "plain" blocks (e.g. code blocks) via this group.
11+
group: NON_FORMATTING_MARK_GROUP,
812

913
addAttributes() {
1014
// Return an object with attribute configuration

packages/core/src/extensions/tiptap-extensions/Suggestions/SuggestionMarks.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Mark } from "@tiptap/core";
22
import { MarkSpec } from "prosemirror-model";
33

4+
import { NON_FORMATTING_MARK_GROUP } from "../../../schema/markGroups.js";
5+
46
// This copies the marks from @handlewithcare/prosemirror-suggest-changes,
57
// but uses the Tiptap Mark API instead so we can use them in BlockNote
68

@@ -10,6 +12,7 @@ export const SuggestionAddMark = Mark.create({
1012
name: "insertion",
1113
inclusive: false,
1214
excludes: "deletion modification insertion",
15+
group: NON_FORMATTING_MARK_GROUP,
1316
addAttributes() {
1417
return {
1518
id: { default: null, validate: "number" }, // note: validate is supported in prosemirror but not in tiptap, so this doesn't actually work (considered not critical)
@@ -55,6 +58,7 @@ export const SuggestionDeleteMark = Mark.create({
5558
name: "deletion",
5659
inclusive: false,
5760
excludes: "insertion modification deletion",
61+
group: NON_FORMATTING_MARK_GROUP,
5862
addAttributes() {
5963
return {
6064
id: { default: null, validate: "number" }, // note: validate is supported in prosemirror but not in tiptap
@@ -103,6 +107,7 @@ export const SuggestionModificationMark = Mark.create({
103107
name: "modification",
104108
inclusive: false,
105109
excludes: "deletion insertion",
110+
group: NON_FORMATTING_MARK_GROUP,
106111
addAttributes() {
107112
// note: validate is supported in prosemirror but not in tiptap
108113
return {

packages/core/src/schema/blocks/createSpec.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
Extension,
77
ExtensionFactoryInstance,
88
} from "../../editor/BlockNoteExtension.js";
9+
import { NON_FORMATTING_MARK_GROUP } from "../markGroups.js";
910
import { PropSchema } from "../propTypes.js";
1011
import {
1112
getBlockFromPos,
@@ -171,9 +172,13 @@ export function addNodeAndExtensionsToSpec<
171172
: TContent extends "plain"
172173
? "text*"
173174
: "",
174-
// "plain" blocks hold unstyled text only, so disallow marks on them.
175-
// TODO: this might need to allow marks for comments and suggestions / diffs
176-
marks: blockConfig.content === "plain" ? "" : undefined,
175+
// "plain" blocks hold unstyled text, so they disallow formatting marks.
176+
// They still allow the non-formatting marks (comments and
177+
// suggestions/diffs) — those annotate content without changing it and are
178+
// ignored by the block model. The group's always-present suggestion marks
179+
// keep this reference valid even when comments aren't configured.
180+
marks:
181+
blockConfig.content === "plain" ? NON_FORMATTING_MARK_GROUP : undefined,
177182
group: "blockContent",
178183
selectable: blockImplementation.meta?.selectable ?? true,
179184
isolating: blockImplementation.meta?.isolating ?? true,
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { describe, expect, it } from "vite-plus/test";
2+
import * as Y from "yjs";
3+
4+
import { CommentsExtension } from "../comments/extension.js";
5+
import { DefaultThreadStoreAuth } from "../comments/threadstore/DefaultThreadStoreAuth.js";
6+
import { BlockNoteEditor } from "../editor/BlockNoteEditor.js";
7+
import { YjsThreadStore } from "../yjs/comments/YjsThreadStore.js";
8+
9+
describe("non-formatting marks on plain blocks", () => {
10+
it("allows suggestion marks but not formatting marks on a code block", () => {
11+
const editor = BlockNoteEditor.create();
12+
const codeBlock = editor.pmSchema.nodes.codeBlock;
13+
14+
// Suggestion/diff marks (in the "annotation" group) are allowed.
15+
expect(codeBlock.allowsMarkType(editor.pmSchema.marks.insertion)).toBe(
16+
true,
17+
);
18+
expect(codeBlock.allowsMarkType(editor.pmSchema.marks.deletion)).toBe(true);
19+
expect(codeBlock.allowsMarkType(editor.pmSchema.marks.modification)).toBe(
20+
true,
21+
);
22+
23+
// Formatting marks are not.
24+
expect(codeBlock.allowsMarkType(editor.pmSchema.marks.bold)).toBe(false);
25+
});
26+
27+
it("allows the comment mark on a code block when comments are enabled", () => {
28+
const ydoc = new Y.Doc();
29+
const threadStore = new YjsThreadStore(
30+
"user-1",
31+
ydoc.getMap("threads"),
32+
new DefaultThreadStoreAuth("user-1", "editor"),
33+
);
34+
const editor = BlockNoteEditor.create({
35+
extensions: [
36+
CommentsExtension({ threadStore, resolveUsers: async () => [] }),
37+
],
38+
});
39+
40+
const commentMark = editor.pmSchema.marks.comment;
41+
expect(commentMark).toBeDefined();
42+
expect(commentMark.spec.group).toBe("annotation");
43+
expect(editor.pmSchema.nodes.codeBlock.allowsMarkType(commentMark)).toBe(
44+
true,
45+
);
46+
});
47+
});
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* ProseMirror mark group for "non-formatting" marks: comments and
3+
* suggestion/diff marks (`insertion`/`deletion`/`modification`). These annotate
4+
* content without representing inline formatting and are ignored by BlockNote's
5+
* content model (`blocknoteIgnore`).
6+
*
7+
* They are the only marks allowed on `"plain"` blocks (e.g. code blocks), which
8+
* otherwise disallow all marks. A block references this group rather than the
9+
* individual marks because the comment mark is only present when comments are
10+
* configured — and the always-present suggestion marks keep the group non-empty
11+
* so the reference never resolves to an unknown mark/group.
12+
*/
13+
export const NON_FORMATTING_MARK_GROUP = "annotation";
14+
15+
/**
16+
* The marks that belong to {@link NON_FORMATTING_MARK_GROUP}, by name.
17+
*
18+
* Yjs schema migrations run before the ProseMirror schema is available, so they
19+
* can't read group membership from the schema and use this list instead. Keep
20+
* it in sync with the marks that declare `group: NON_FORMATTING_MARK_GROUP`.
21+
*/
22+
export const NON_FORMATTING_MARK_NAMES: readonly string[] = [
23+
"comment",
24+
"insertion",
25+
"deletion",
26+
"modification",
27+
];

packages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,45 @@ describe("stripDisallowedMarks rule", () => {
5858
);
5959
});
6060

61+
it("keeps comment and suggestion marks while stripping formatting", () => {
62+
const doc = new Y.Doc();
63+
const fragment = doc.getXmlFragment("doc");
64+
const blockGroup = new Y.XmlElement("blockGroup");
65+
const container = new Y.XmlElement("blockContainer");
66+
container.setAttribute("id", "code-0");
67+
const codeBlock = new Y.XmlElement("codeBlock");
68+
codeBlock.setAttribute("language", "javascript");
69+
const text = new Y.XmlText();
70+
text.insert(0, "const x = 1;");
71+
text.format(0, 5, { bold: {} }); // formatting — should be stripped
72+
text.format(0, 5, { comment: { threadId: "t1", orphan: false } }); // keep
73+
text.format(6, 1, { insertion: { id: 1 } }); // keep
74+
codeBlock.insert(0, [text]);
75+
container.insert(0, [codeBlock]);
76+
blockGroup.insert(0, [container]);
77+
fragment.insert(0, [blockGroup]);
78+
79+
const editor = BlockNoteEditor.create();
80+
stripDisallowedMarks(fragment, editor.schema.blockSchema);
81+
82+
const delta = text.toDelta() as {
83+
insert: string;
84+
attributes?: Record<string, unknown>;
85+
}[];
86+
const remaining = new Set<string>();
87+
for (const op of delta) {
88+
if (op.attributes) {
89+
Object.keys(op.attributes).forEach((k) => remaining.add(k));
90+
}
91+
}
92+
93+
expect(remaining.has("bold")).toBe(false); // formatting stripped
94+
expect(remaining.has("comment")).toBe(true); // comment kept
95+
expect(remaining.has("insertion")).toBe(true); // suggestion kept
96+
// Text preserved (delta inserts concatenated, ignoring mark tags).
97+
expect(delta.map((op) => op.insert).join("")).toBe("const x = 1;");
98+
});
99+
61100
it("leaves an already-clean code block untouched", () => {
62101
const doc = new Y.Doc();
63102
const fragment = doc.getXmlFragment("doc");

packages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as Y from "yjs";
22

33
import { BlockSchema } from "../../../../schema/index.js";
4+
import { NON_FORMATTING_MARK_NAMES } from "../../../../schema/markGroups.js";
45
import { PreSyncMigrationRule } from "./migrationRule.js";
56

67
// Recursively traverses a `Y.XmlElement` and its descendant elements.
@@ -16,8 +17,9 @@ const traverseElement = (
1617
});
1718
};
1819

19-
// Removes all formatting (marks) from a `Y.XmlText`. Returns whether anything
20-
// was removed.
20+
// Removes formatting marks from a `Y.XmlText`, keeping the non-formatting marks
21+
// (comments and suggestions/diffs) that a `"plain"` block still allows. Returns
22+
// whether anything was removed.
2123
const stripFormatting = (text: Y.XmlText) => {
2224
const markKeys = new Set<string>();
2325
for (const op of text.toDelta() as {
@@ -30,31 +32,36 @@ const stripFormatting = (text: Y.XmlText) => {
3032
}
3133
}
3234

33-
if (markKeys.size === 0) {
35+
const keysToStrip = [...markKeys].filter(
36+
(key) => !NON_FORMATTING_MARK_NAMES.includes(key),
37+
);
38+
39+
if (keysToStrip.length === 0) {
3440
return false;
3541
}
3642

3743
// Setting each attribute to `null` removes that mark across the range.
3844
text.format(
3945
0,
4046
text.length,
41-
Object.fromEntries([...markKeys].map((key) => [key, null])),
47+
Object.fromEntries(keysToStrip.map((key) => [key, null])),
4248
);
4349

4450
return true;
4551
};
4652

47-
// Strips marks from the text of `"plain"` content blocks (e.g. code blocks).
53+
// Strips formatting marks from the text of `"plain"` content blocks (e.g. code
54+
// blocks).
4855
//
4956
// Older documents may contain blocks whose content was previously `"inline"`
50-
// (and so could hold marks like bold) but is now `"plain"` — whose ProseMirror
51-
// node disallows marks (`marks: ""`). y-prosemirror does not strip disallowed
52-
// marks; instead it rejects the whole node (`createChecked` throws) and its
53-
// error handler DELETES that node from the Yjs document, propagating the
54-
// deletion to all peers. This rule removes those marks from the fragment so the
55-
// node stays valid and survives reconstruction (the text itself is preserved;
56-
// only the formatting — which `"plain"` content cannot represent anyway — is
57-
// dropped).
57+
// (and so could hold formatting marks like bold) but is now `"plain"` — whose
58+
// ProseMirror node only allows the non-formatting marks (comments and
59+
// suggestions/diffs). y-prosemirror does not strip disallowed marks; instead it
60+
// rejects the whole node (`createChecked` throws) and its error handler DELETES
61+
// that node from the Yjs document, propagating the deletion to all peers. This
62+
// rule removes the disallowed (formatting) marks from the fragment so the node
63+
// stays valid and survives reconstruction — preserving the text as well as the
64+
// comment/suggestion marks the block still allows.
5865
//
5966
// Unlike the post-sync `MigrationRule`s, this must run BEFORE y-prosemirror
6067
// reconstructs the document, so it mutates the Yjs fragment directly.

0 commit comments

Comments
 (0)