From b7d78ae6ded6600ab884586554f75d10f21d2558 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 15 Sep 2026 09:56:12 +0200 Subject: [PATCH 01/42] feat(core): add composable read-only restrictions Keep feature-owned read-only restrictions separate from the application editable setting. Notify transaction subscribers without document-change events and prevent link editing while locked. --- packages/core/src/editor/BlockNoteEditor.ts | 2 + .../managers/ExtensionManager/extensions.ts | 2 + .../core/src/editor/managers/StateManager.ts | 20 +++- .../src/extensions/ReadOnly/ReadOnly.test.ts | 108 +++++++++++++++++ .../core/src/extensions/ReadOnly/ReadOnly.ts | 54 +++++++++ packages/core/src/extensions/index.ts | 1 + .../DefaultButtons/CreateLinkButton.tsx | 7 +- .../LinkToolbar/LinkToolbarController.tsx | 30 ++++- pnpm-lock.yaml | 6 + tests/package.json | 14 ++- .../unit/react/LinkToolbarReadOnly.test.tsx | 112 ++++++++++++++++++ 11 files changed, 344 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/extensions/ReadOnly/ReadOnly.test.ts create mode 100644 packages/core/src/extensions/ReadOnly/ReadOnly.ts create mode 100644 tests/src/unit/react/LinkToolbarReadOnly.test.tsx diff --git a/packages/core/src/editor/BlockNoteEditor.ts b/packages/core/src/editor/BlockNoteEditor.ts index 25b93d03f4..72066aec44 100644 --- a/packages/core/src/editor/BlockNoteEditor.ts +++ b/packages/core/src/editor/BlockNoteEditor.ts @@ -1040,6 +1040,8 @@ export class BlockNoteEditor< /** * Makes the editor editable or locks it, depending on the argument passed. + * Plugins can temporarily prevent editing without changing this setting. + * The getter reports whether editing is currently allowed by both. * @param editable True to make the editor editable, or false to lock it. */ public set isEditable(editable: boolean) { diff --git a/packages/core/src/editor/managers/ExtensionManager/extensions.ts b/packages/core/src/editor/managers/ExtensionManager/extensions.ts index 853cca2493..780cb6892e 100644 --- a/packages/core/src/editor/managers/ExtensionManager/extensions.ts +++ b/packages/core/src/editor/managers/ExtensionManager/extensions.ts @@ -21,6 +21,7 @@ import { PlaceholderExtension, PositionMappingExtension, PreviousBlockTypeExtension, + ReadOnlyExtension, ShowSelectionExtension, SideMenuExtension, SourceBlockWithPreviewExtension, @@ -168,6 +169,7 @@ export function getDefaultExtensions( LinkToolbarExtension(options), NodeSelectionKeyboardExtension(), PlaceholderExtension(options), + ReadOnlyExtension(), ShowSelectionExtension(options), SideMenuExtension(options), SourceBlockWithPreviewExtension(), diff --git a/packages/core/src/editor/managers/StateManager.ts b/packages/core/src/editor/managers/StateManager.ts index 9dc3eebff2..dee85e2829 100644 --- a/packages/core/src/editor/managers/StateManager.ts +++ b/packages/core/src/editor/managers/StateManager.ts @@ -205,8 +205,24 @@ export class StateManager { // not relevant on headless return; } - if (this.editor._tiptapEditor.options.editable !== editable) { - this.editor._tiptapEditor.setEditable(editable); + if (this.editor._tiptapEditor.options.editable === editable) { + return; + } + // Not through tiptap's `update` event: to every `onChange` subscriber that + // event means "the document changed", and nothing did. Dispatch an empty + // transaction instead, so a selector reading `isEditable` (the link + // toolbar's read-only gate, a host's own UI) sees the change through the + // same `transaction` event as any other state change — and nothing else + // fires. + this.editor._tiptapEditor.setEditable(editable, false); + this.notifyEditableChanged(); + } + + /** Recompute plugin editability and notify transaction subscribers. */ + private notifyEditableChanged() { + const view = this.prosemirrorView; + if (view && !view.isDestroyed) { + this.transact((tr) => tr.setMeta("editable", true)); } } diff --git a/packages/core/src/extensions/ReadOnly/ReadOnly.test.ts b/packages/core/src/extensions/ReadOnly/ReadOnly.test.ts new file mode 100644 index 0000000000..d0791ae063 --- /dev/null +++ b/packages/core/src/extensions/ReadOnly/ReadOnly.test.ts @@ -0,0 +1,108 @@ +/** @vitest-environment jsdom */ +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, +} from "vite-plus/test"; +import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { ReadOnlyExtension } from "./ReadOnly.js"; + +describe("ReadOnlyExtension", () => { + let editor: BlockNoteEditor; + let readOnly: ReturnType>; + + beforeEach(() => { + editor = BlockNoteEditor.create(); + editor.mount(document.createElement("div")); + readOnly = editor.getExtension(ReadOnlyExtension)!; + }); + + afterEach(() => editor.unmount()); + + it("keeps editing disabled until every feature releases its restriction", () => { + readOnly.setReadOnly(true, "preview"); + readOnly.setReadOnly(true, "upload"); + readOnly.setReadOnly(true, "upload"); + expect(editor.isEditable).toBe(false); + + readOnly.setReadOnly(false, "preview"); + readOnly.setReadOnly(false, "unrelated"); + expect(editor.isEditable).toBe(false); + + readOnly.setReadOnly(false, "upload"); + expect(editor.isEditable).toBe(true); + }); + + it("preserves the application's latest editable setting", () => { + readOnly.setReadOnly(true, "preview"); + editor.isEditable = true; + expect(editor.isEditable).toBe(false); + + editor.isEditable = false; + readOnly.setReadOnly(false, "preview"); + expect(editor.isEditable).toBe(false); + + editor.isEditable = true; + expect(editor.isEditable).toBe(true); + }); + + it("notifies transaction subscribers without reporting document changes", () => { + const changes = vi.fn(); + const editableStates: boolean[] = []; + editor.onChange(changes); + editor._tiptapEditor.on("transaction", () => { + editableStates.push(editor.isEditable); + }); + + readOnly.setReadOnly(true, "preview"); + expect(editableStates.length).toBeGreaterThan(0); + expect(editableStates.every((editable) => !editable)).toBe(true); + editableStates.length = 0; + readOnly.setReadOnly(true, "preview"); + expect(editableStates).toEqual([]); + readOnly.setReadOnly(false, "preview"); + + expect(editableStates.length).toBeGreaterThan(0); + expect(editableStates.every((editable) => editable)).toBe(true); + expect(changes).not.toHaveBeenCalled(); + }); + + it("groups application editability changes into the pending transaction", () => { + const transactions = vi.fn(); + const changes = vi.fn(); + editor._tiptapEditor.on("transaction", transactions); + editor.onChange(changes); + + editor.transact(() => { + editor.isEditable = false; + }); + expect(editor.isEditable).toBe(false); + expect(transactions).toHaveBeenCalled(); + expect(changes).not.toHaveBeenCalled(); + + transactions.mockClear(); + editor.transact((tr) => { + tr.insertText("hello", 1); + editor.isEditable = true; + }); + expect(editor.isEditable).toBe(true); + expect(editor.prosemirrorState.doc.textContent).toContain("hello"); + expect(transactions).toHaveBeenCalled(); + expect(changes).toHaveBeenCalledTimes(1); + }); + + it("composes with pending document and metadata-only transactions", () => { + editor.transact((tr) => { + tr.insertText("hello", 1); + readOnly.setReadOnly(true, "preview"); + }); + expect(editor.prosemirrorState.doc.textContent).toContain("hello"); + expect(editor.isEditable).toBe(false); + + editor.transact(() => readOnly.setReadOnly(false, "preview")); + expect(editor.isEditable).toBe(true); + }); +}); diff --git a/packages/core/src/extensions/ReadOnly/ReadOnly.ts b/packages/core/src/extensions/ReadOnly/ReadOnly.ts new file mode 100644 index 0000000000..7771befe7f --- /dev/null +++ b/packages/core/src/extensions/ReadOnly/ReadOnly.ts @@ -0,0 +1,54 @@ +import { Plugin, PluginKey } from "prosemirror-state"; +import { + createExtension, + createStore, +} from "../../editor/BlockNoteExtension.js"; + +const PLUGIN_KEY = new PluginKey("bn-read-only"); + +/** Temporarily prevent editing without changing the application's editable setting. */ +export const ReadOnlyExtension = createExtension(({ editor }) => { + const store = createStore( + { enabledSet: new Set() }, + { + onUpdate(state, prevState) { + if ( + (state.enabledSet.size === 0) === + (prevState.enabledSet.size === 0) + ) { + return; + } + const view = editor.prosemirrorView; + if (view && !view.isDestroyed) { + // Recompute plugin editability and notify UI subscribers without a + // document change. Reuse any transaction already in progress. + editor.transact((tr) => tr.setMeta(PLUGIN_KEY, {})); + } + }, + }, + ); + + return { + key: "readOnly", + store, + prosemirrorPlugins: [ + new Plugin({ + key: PLUGIN_KEY, + props: { editable: () => store.state.enabledSet.size === 0 }, + }), + ], + /** + * Enable or disable read-only mode for a feature identified by key. + * Passing false releases only that feature's restriction; other features + * and the application's editor.isEditable setting still apply. + * Repeated calls with the same key are idempotent. + */ + setReadOnly(readOnly: boolean, key: string) { + store.setState({ + enabledSet: readOnly + ? new Set([...store.state.enabledSet, key]) + : new Set([...store.state.enabledSet].filter((k) => k !== key)), + }); + }, + } as const; +}); diff --git a/packages/core/src/extensions/index.ts b/packages/core/src/extensions/index.ts index eb1d455e33..93a9d8c232 100644 --- a/packages/core/src/extensions/index.ts +++ b/packages/core/src/extensions/index.ts @@ -10,6 +10,7 @@ export * from "./NodeSelectionKeyboard/NodeSelectionKeyboard.js"; export * from "./Placeholder/Placeholder.js"; export * from "./PositionMapping/PositionMapping.js"; export * from "./PreviousBlockType/PreviousBlockType.js"; +export * from "./ReadOnly/ReadOnly.js"; export * from "./ShowSelection/ShowSelection.js"; export * from "./SideMenu/SideMenu.js"; export * from "./SourceBlockWithPreview/SourceBlockWithPreview.js"; diff --git a/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx b/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx index a03eab06ce..6a18824b99 100644 --- a/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx +++ b/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx @@ -94,6 +94,11 @@ export const CreateLinkButton = () => { // Makes Ctrl+K/Meta+K open link creation popover. useEffect(() => { const callback = (event: KeyboardEvent) => { + // A read-only editor (e.g. while a version preview is open) has no link + // creation to offer, so leave the shortcut to the browser. + if (!editor.isEditable) { + return; + } if ((event.ctrlKey || event.metaKey) && event.key === "k") { setShowPopover(true); event.preventDefault(); @@ -105,7 +110,7 @@ export const CreateLinkButton = () => { return () => { editorDOMElement?.removeEventListener("keydown", callback); }; - }, [editorDOMElement]); + }, [editor, editorDOMElement]); if (state === undefined) { return null; diff --git a/packages/react/src/components/LinkToolbar/LinkToolbarController.tsx b/packages/react/src/components/LinkToolbar/LinkToolbarController.tsx index fbe789a544..cb2cd23498 100644 --- a/packages/react/src/components/LinkToolbar/LinkToolbarController.tsx +++ b/packages/react/src/components/LinkToolbar/LinkToolbarController.tsx @@ -5,6 +5,7 @@ import { FC, useEffect, useMemo, useState } from "react"; import { useBlockNoteEditor } from "../../hooks/useBlockNoteEditor.js"; import { useEditorDOMElement } from "../../hooks/useEditorDomElement.js"; +import { useEditorState } from "../../hooks/useEditorState.js"; import { useExtension } from "../../hooks/useExtension.js"; import { FloatingUIOptions } from "../Popovers/FloatingUIOptions.js"; import { @@ -26,6 +27,14 @@ export const LinkToolbarController = (props: { }) => { const editor = useBlockNoteEditor(); + // Reactive, not a one-off read: the editor can be locked and unlocked while + // this component stays mounted (a version preview does exactly that), and a + // read-only editor must not offer link editing. + const isEditable = useEditorState({ + editor, + selector: ({ editor: current }) => current?.isEditable ?? false, + }); + const [toolbarOpen, setToolbarOpen] = useState(false); const [toolbarPositionFrozen, setToolbarPositionFrozen] = useState(false); @@ -49,6 +58,15 @@ export const LinkToolbarController = (props: { // cursor position. If there is none, uses the link hovered by the mouse // cursor. Otherwise, the toolbar remains closed. useEffect(() => { + if (!isEditable) { + // Nothing to open a toolbar for, and any link picked up while the editor + // was still editable has to go — otherwise it would reappear the moment + // editing resumes. + setLink(undefined); + setToolbarOpen(false); + return; + } + const textCursorCallback = () => { const textCursorLink = linkToolbar.getLinkAtSelection(); if (!textCursorLink) { @@ -114,7 +132,14 @@ export const LinkToolbarController = (props: { destroyOnSelectionChangeHandler(); editorDOMElement?.removeEventListener("mouseover", mouseCursorCallback); }; - }, [editor, editorDOMElement, linkToolbar, link, toolbarPositionFrozen]); + }, [ + editor, + editorDOMElement, + isEditable, + linkToolbar, + link, + toolbarPositionFrozen, + ]); const floatingUIOptions = useMemo( () => ({ @@ -176,8 +201,7 @@ export const LinkToolbarController = (props: { [link?.element], ); - // TODO: this should be a hook to be reactive - if (!editor.isEditable) { + if (!isEditable) { return null; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c704abb1be..9642bd70ca 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6576,6 +6576,12 @@ importers: '@tailwindcss/vite': specifier: ^4.1.14 version: 4.2.2(vite@8.0.8(@types/node@25.9.5)(esbuild@0.27.5)(jiti@2.6.1)(terser@5.51.2)(tsx@4.21.0)(yaml@2.9.0)) + '@testing-library/dom': + specifier: ^10.4.0 + version: 10.4.1 + '@testing-library/react': + specifier: ^16.3.0 + version: 16.3.2(@testing-library/dom@10.4.1)(@types/react-dom@19.2.3(@types/react@19.2.14))(@types/react@19.2.14)(react-dom@19.2.5(react@19.2.5))(react@19.2.5) '@tiptap/pm': specifier: ^3.29.2 version: 3.29.2 diff --git a/tests/package.json b/tests/package.json index 68c8844a79..1a017f98c1 100644 --- a/tests/package.json +++ b/tests/package.json @@ -11,17 +11,19 @@ "devDependencies": { "@blocknote/ariakit": "workspace:^", "@blocknote/core": "workspace:^", - "@blocknote/mantine": "workspace:^", "@blocknote/diagram-block": "workspace:^", - "@blocknote/xl-email-exporter": "workspace:^", - "@blocknote/xl-pdf-exporter": "workspace:^", - "pdfjs-dist": "^4.10.38", + "@blocknote/mantine": "workspace:^", "@blocknote/math-block": "workspace:^", "@blocknote/react": "workspace:^", "@blocknote/shadcn": "workspace:^", + "@blocknote/xl-email-exporter": "workspace:^", "@blocknote/xl-multi-column": "workspace:^", + "@blocknote/xl-pdf-exporter": "workspace:^", + "@blocknote/xl-typst-exporter": "workspace:^", "@playwright/test": "1.60.0", "@tailwindcss/vite": "^4.1.14", + "@testing-library/dom": "^10.4.0", + "@testing-library/react": "^16.3.0", "@tiptap/pm": "^3.29.2", "@types/node": "^20.19.22", "@types/react": "^19.2.3", @@ -31,13 +33,13 @@ "@y/protocols": "^1.0.6-rc.1", "@y/y": "^14.0.0-rc.23", "htmlfy": "^0.6.7", + "pdfjs-dist": "^4.10.38", "react": "^19.2.5", "react-dom": "^19.2.5", "react-icons": "^5.5.0", "rimraf": "^5.0.10", "vite-plus": "catalog:", - "vitest-browser-react": "^2.2.0", - "@blocknote/xl-typst-exporter": "workspace:^" + "vitest-browser-react": "^2.2.0" }, "dependencies": { "get-port-please": "3.2.0", diff --git a/tests/src/unit/react/LinkToolbarReadOnly.test.tsx b/tests/src/unit/react/LinkToolbarReadOnly.test.tsx new file mode 100644 index 0000000000..44858db29f --- /dev/null +++ b/tests/src/unit/react/LinkToolbarReadOnly.test.tsx @@ -0,0 +1,112 @@ +import { BlockNoteEditor } from "@blocknote/core"; +import { BlockNoteView } from "@blocknote/mantine"; +import "@blocknote/mantine/style.css"; +import { LinkToolbarController } from "@blocknote/react"; +import { TextSelection } from "@tiptap/pm/state"; +import { act, cleanup, fireEvent, render } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vite-plus/test"; + +/** + * A read-only editor must not open the link toolbar: it only offers editing + * actions, and a version preview keeps the editor read-only for as long as the + * history panel is open. + */ +describe("LinkToolbarController in a read-only editor", () => { + afterEach(() => { + cleanup(); + }); + + function setup(editable: boolean) { + const editor = BlockNoteEditor.create({ + initialContent: [ + { + type: "paragraph", + content: [ + { + type: "link", + href: "https://example.com", + content: "a link", + }, + ], + }, + ], + }); + + render( + + + , + ); + + return editor; + } + + /** The toolbar's own root, rendered only while it's open. */ + function toolbar() { + return document.querySelector(".bn-link-toolbar"); + } + + /** + * Put the text cursor inside the link. That's the deterministic half of the + * controller's two open paths; the mouse path shares the same `isEditable` + * gate (and the same effect), it just also needs floating-ui's hover delay. + */ + async function putCursorInLink(editor: BlockNoteEditor) { + let linkPos: number | undefined; + editor.prosemirrorState.doc.descendants((node, pos) => { + if (linkPos === undefined && node.isText && node.marks.length > 0) { + linkPos = pos + 1; + } + return linkPos === undefined; + }); + + await act(async () => { + editor.transact((tr) => + tr.setSelection(TextSelection.create(tr.doc, linkPos!)), + ); + }); + } + + it("opens the toolbar when the editor is editable", async () => { + const editor = setup(true); + + await putCursorInLink(editor); + + expect(toolbar()).not.toBeNull(); + }); + + it("does not open the toolbar when the editor is read-only", async () => { + const editor = setup(false); + + await putCursorInLink(editor); + + expect(toolbar()).toBeNull(); + }); + + it("ignores a hovered link when the editor is read-only", async () => { + const editor = setup(false); + const link = editor.domElement!.querySelector("a")!; + + fireEvent.mouseOver(link, { bubbles: true }); + fireEvent.pointerEnter(link, { pointerType: "mouse", bubbles: true }); + fireEvent.mouseEnter(link); + // Past floating-ui's 250ms open delay. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 400)); + }); + + expect(toolbar()).toBeNull(); + }); + + it("closes the toolbar when the editor becomes read-only", async () => { + const editor = setup(true); + await putCursorInLink(editor); + expect(toolbar()).not.toBeNull(); + + await act(async () => { + editor.isEditable = false; + }); + + expect(toolbar()).toBeNull(); + }); +}); From 72a170296916115f8d83d7bcc8f9906f323e29de Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 15 Sep 2026 09:57:18 +0200 Subject: [PATCH 02/42] feat(core)!: refine attribution colors and deleted-block styling Use consistent author colors and derived light tints for marks and tooltips. Compact deleted-block cards and update the corresponding visual baselines. --- packages/core/src/editor/Block.css | 34 +++++-- packages/core/src/user/userColors.test.ts | 94 ++++++++++++++++++ packages/core/src/user/userColors.ts | 54 +++++++--- .../y/extensions/AttributionExtension.test.ts | 44 +++++++- .../src/y/extensions/AttributionExtension.ts | 8 +- .../DiffVersioningExtension.test.ts | 20 ++++ .../y/extensions/DiffVersioningExtension.ts | 10 +- ...d-remove-delete-divider-chromium-linux.png | Bin 11863 -> 10435 bytes ...dd-remove-delete-divider-firefox-linux.png | Bin 12188 -> 10783 bytes ...add-remove-delete-divider-webkit-linux.png | Bin 12763 -> 11369 bytes ...add-remove-delete-image-chromium-linux.png | Bin 10857 -> 9814 bytes .../add-remove-delete-image-firefox-linux.png | Bin 11172 -> 10152 bytes .../add-remove-delete-image-webkit-linux.png | Bin 11401 -> 10386 bytes ...ove-delete-mixed-parent-chromium-linux.png | Bin 15885 -> 21636 bytes ...move-delete-mixed-parent-firefox-linux.png | Bin 16343 -> 21666 bytes ...emove-delete-mixed-parent-webkit-linux.png | Bin 17084 -> 22399 bytes ...d-remove-insert-divider-chromium-linux.png | Bin 8624 -> 8422 bytes ...dd-remove-insert-divider-firefox-linux.png | Bin 9579 -> 9350 bytes ...add-remove-insert-divider-webkit-linux.png | Bin 10190 -> 9935 bytes ...add-remove-insert-image-chromium-linux.png | Bin 10828 -> 10269 bytes .../add-remove-insert-image-firefox-linux.png | Bin 11421 -> 10813 bytes .../add-remove-insert-image-webkit-linux.png | Bin 11334 -> 10883 bytes .../basicText.concurrent.test.tsx | 4 +- .../fixtures/concurrentSuggestionFixture.tsx | 3 +- 24 files changed, 238 insertions(+), 33 deletions(-) create mode 100644 packages/core/src/user/userColors.test.ts diff --git a/packages/core/src/editor/Block.css b/packages/core/src/editor/Block.css index ef2867121d..864c7c7a20 100644 --- a/packages/core/src/editor/Block.css +++ b/packages/core/src/editor/Block.css @@ -1149,11 +1149,12 @@ that wrap blocks get a per-block badge below instead. display: inline-block; margin-right: 6px; padding: 0 4px; + /* Matches the block-level badge below, so the two read as one label style. */ font-size: 11px; - font-weight: bold; + font-weight: 600; text-transform: uppercase; letter-spacing: 0.04em; - line-height: 1.4; + line-height: 16px; vertical-align: middle; /* Use the editor's text color (themed for light/dark) rather than inheriting, which would pick up the deleted content's user color. */ @@ -1228,9 +1229,17 @@ only non-collapsing properties — background / radius / padding never depend on content's intrinsic size, so no block can break. */ .bn-suggestion-node .bn-block-content:not(:has(.bn-inline-content)) { + /* The card bleeds this far into the gutters on both sides, so tinting a block + never shifts its content sideways. */ + --bn-suggestion-card-inset: 6px; background-color: color-mix(in srgb, var(--user-color-light) 50%, white); - border-radius: 16px; - padding: 12px; + border-radius: 6px; + /* A hairline in the author's color, so a pale tint still reads as a card. */ + box-shadow: 0 0 0 1px + color-mix(in srgb, var(--user-color-dark) 25%, transparent); + padding: 3px var(--bn-suggestion-card-inset); + margin-left: calc(-1 * var(--bn-suggestion-card-inset)); + width: calc(100% + 2 * var(--bn-suggestion-card-inset)); } .dark.bn-root @@ -1256,11 +1265,12 @@ gated on a wrapper that only width-bearing blocks have. /* A deletion additionally flags the block with the localized "Deleted" label, placed -above the content (out of flow) with extra top padding reserving its row. +above the content (out of flow) with extra top padding reserving its row: the +card's own 3px, the label's 16px line box, and 2px of breathing room under it. */ .bn-suggestion-node--delete .bn-block-content:not(:has(.bn-inline-content)) { position: relative; - padding: 48px 24px 24px; + padding-top: calc(3px + 16px + 2px); } .bn-suggestion-node--delete @@ -1269,11 +1279,13 @@ above the content (out of flow) with extra top padding reserving its row. /* Sits in the reserved top padding, above the content. Out of flow so it never becomes a flex item beside the block. */ position: absolute; - top: 16px; - left: 24px; - font-size: 18px; - font-weight: 500; - line-height: 1.2; + top: 3px; + left: var(--bn-suggestion-card-inset); + font-size: 11px; + font-weight: 600; + line-height: 16px; + letter-spacing: 0.04em; + text-transform: uppercase; /* Use the editor's text color (themed for light/dark) rather than inheriting, which would pick up the suggestion's user color. */ color: var(--bn-colors-editor-text); diff --git a/packages/core/src/user/userColors.test.ts b/packages/core/src/user/userColors.test.ts new file mode 100644 index 0000000000..2d130d1257 --- /dev/null +++ b/packages/core/src/user/userColors.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from "vite-plus/test"; + +import type { User } from "./UserStore.js"; +import { createUserStore } from "./UserStore.js"; +import { + colorsForUserIds, + fallbackColorForUserId, + userColorPalette, + userMarkColors, +} from "./userColors.js"; + +describe("userColorPalette", () => { + it("contains no red", () => { + // The palette tints insertions as well as deletions, so a red entry would + // make one author's additions read as errors. Guard the property rather + // than the exact hexes: red is any entry whose hue is near 0°/360°. + for (const { light, dark } of userColorPalette) { + for (const color of [light, dark]) { + const [r, g, b] = [1, 3, 5].map((offset) => + parseInt(color.slice(offset, offset + 2), 16), + ); + const isRed = r > g + 40 && r > b + 40; + expect(isRed, `${color} reads as red`).toBe(false); + } + } + }); + + it("assigns a stable entry per user id", () => { + expect(fallbackColorForUserId("alice")).toEqual( + fallbackColorForUserId("alice"), + ); + expect(userColorPalette).toContainEqual(fallbackColorForUserId("alice")); + }); +}); + +describe("userMarkColors", () => { + it("is undefined for a user with no color", () => { + expect(userMarkColors(undefined)).toBeUndefined(); + expect(userMarkColors({})).toBeUndefined(); + }); + + it("uses both colors when the app supplies both", () => { + expect(userMarkColors({ color: "#123456", colorLight: "#abcdef" })).toEqual( + { + light: "#abcdef", + dark: "#123456", + }, + ); + }); + + it("derives the light tint when only `color` is set", () => { + expect(userMarkColors({ color: "#123456" })).toEqual({ + light: "color-mix(in srgb, #123456 30%, white)", + dark: "#123456", + }); + }); +}); + +describe("colorsForUserIds", () => { + it("falls back to the first palette entry with no ids", () => { + const store = createUserStore(async () => []); + expect(colorsForUserIds(store, undefined)).toEqual(userColorPalette[0]); + expect(colorsForUserIds(store, [])).toEqual(userColorPalette[0]); + }); + + it("falls back to the id's palette entry for an unresolved user", () => { + const store = createUserStore(async () => []); + expect(colorsForUserIds(store, ["alice"])).toEqual( + fallbackColorForUserId("alice"), + ); + }); + + it("uses the resolved user's own colors, deriving the tint when needed", async () => { + const store = createUserStore(async (ids: string[]) => + ids.map((id) => ({ + id, + username: id, + avatarUrl: "", + color: id === "both" ? "#123456" : "#654321", + ...(id === "both" ? { colorLight: "#abcdef" } : {}), + })), + ); + await store.loadUsers(["both", "dark-only"]); + + expect(colorsForUserIds(store, ["both"])).toEqual({ + light: "#abcdef", + dark: "#123456", + }); + expect(colorsForUserIds(store, ["dark-only"])).toEqual({ + light: "color-mix(in srgb, #654321 30%, white)", + dark: "#654321", + }); + }); +}); diff --git a/packages/core/src/user/userColors.ts b/packages/core/src/user/userColors.ts index b63e909238..fdad687cc9 100644 --- a/packages/core/src/user/userColors.ts +++ b/packages/core/src/user/userColors.ts @@ -1,5 +1,5 @@ import { digestString } from "lib0/hash/fnv1a"; -import type { UserStore } from "./UserStore.js"; +import type { User, UserStore } from "./UserStore.js"; /** * Deterministic hash of a string to an unsigned 32-bit integer. @@ -12,13 +12,21 @@ const hashStr = (s: string): number => { return Math.abs(hash); }; -/** Fallback palette used when a user has no resolved color of their own. */ +/** + * Fallback palette used when a user has no resolved color of their own. + * + * Deliberately red-free: these colors tint *insertions* as well as deletions, + * and a red insertion reads as an error rather than as one author's + * contribution. The hues are spread far enough apart to stay distinguishable + * for the most common forms of colour-vision deficiency. + */ export const userColorPalette: Array<{ light: string; dark: string }> = [ - { light: "#fff0c2", dark: "#8a6d1a" }, - { light: "#fcc9c3", dark: "#8a2e24" }, - { light: "#d4e8eb", dark: "#4a7178" }, - { light: "#c2eeff", dark: "#1a6e8a" }, - { light: "#bef3ff", dark: "#0a7a8a" }, + { light: "#fff0c2", dark: "#8a6d1a" }, // amber + { light: "#dcdefc", dark: "#3b3f9c" }, // indigo + { light: "#c9efe9", dark: "#0f6e62" }, // teal + { light: "#c9dcff", dark: "#1e4fb0" }, // blue + { light: "#eadcfb", dark: "#6b2fa3" }, // violet + { light: "#dfe4ea", dark: "#46525f" }, // slate ]; /** The deterministic {@link userColorPalette} entry for a single user id. */ @@ -28,7 +36,28 @@ export const fallbackColorForUserId = ( userColorPalette[hashStr(id) % userColorPalette.length]; /** - * The (first) user's resolved color from the {@link UserStore}, or their + * A user's own mark colors, or `undefined` when they have none. + * + * `color` is the saturated color the app already uses for that user (cursors, + * avatars); `colorLight` is the pale background a mark is highlighted with. Most + * applications only set the former, so derive the latter rather than fall back + * to a palette entry that has nothing to do with the user's actual color — a + * user whose cursor is green shouldn't have amber marks. + */ +export const userMarkColors = ( + user: Pick | undefined, +): { light: string; dark: string } | undefined => { + if (!user?.color) { + return undefined; + } + return { + light: user.colorLight ?? `color-mix(in srgb, ${user.color} 30%, white)`, + dark: user.color, + }; +}; + +/** + * The (first) user's {@link userMarkColors}, or their * {@link fallbackColorForUserId} palette entry. Used where a concrete color * string is needed (the portaled hover tooltip); marks themselves use the * cascaded {@link userColorVarNames} properties instead. @@ -41,11 +70,10 @@ export const colorsForUserIds = ( return userColorPalette[0]; } const firstId = userIds[0]; - const user = userStore.getUser(firstId); - if (user?.color && user.colorLight) { - return { light: user.colorLight, dark: user.color }; - } - return fallbackColorForUserId(firstId); + return ( + userMarkColors(userStore.getUser(firstId)) ?? + fallbackColorForUserId(firstId) + ); }; /** diff --git a/packages/core/src/y/extensions/AttributionExtension.test.ts b/packages/core/src/y/extensions/AttributionExtension.test.ts index f752b48182..19cf9051f7 100644 --- a/packages/core/src/y/extensions/AttributionExtension.test.ts +++ b/packages/core/src/y/extensions/AttributionExtension.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vite-plus/test"; import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; import type { User } from "../../user/index.js"; +import { cssVarUserId } from "../../user/index.js"; import { AttributionExtension } from "./AttributionExtension.js"; // Editors created during a test, destroyed in afterEach: an undestroyed @@ -16,7 +17,7 @@ const editors: BlockNoteEditor[] = []; // A `resolveUsers` spy plus an editor with the AttributionExtension registered. // No Yjs/collaboration needed — the extension's load plugin only cares that a // transaction adds a `y-attributed-*` mark, which we do directly below. -function createEditor() { +function createEditor(user?: Partial) { const resolveUsers = vi.fn(async (ids: string[]): Promise => ids.map((id) => ({ id, @@ -24,6 +25,7 @@ function createEditor() { avatarUrl: "", color: "#123456", colorLight: "#abcdef", + ...user, })), ); @@ -36,6 +38,16 @@ function createEditor() { return { editor, resolveUsers }; } +/** The `--user-color--{light,dark}` values on the editor root. */ +function rootColorVars(editor: BlockNoteEditor, userId: string) { + const root = editor.prosemirrorView!.dom as HTMLElement; + const key = cssVarUserId(userId); + return { + light: root.style.getPropertyValue(`--user-color-${key}-light`), + dark: root.style.getPropertyValue(`--user-color-${key}-dark`), + }; +} + // Add a `y-attributed-insert` mark carrying `userIds` over the first block's // text, mirroring how the sync reconcile applies attribution marks. function addInsertMark(editor: BlockNoteEditor, userIds: string[]) { @@ -92,4 +104,34 @@ describe("AttributionExtension user loading", () => { // The user store dedupes already-cached ids, so `alice` is fetched once. expect(resolveUsers).toHaveBeenCalledTimes(1); }); + + it("writes both of a resolved author's colors to the editor root", async () => { + const { editor } = createEditor(); + editor.replaceBlocks(editor.document, [{ content: "hello" }]); + + addInsertMark(editor, ["alice"]); + await vi.waitFor(() => + expect(rootColorVars(editor, "alice").dark).not.toBe(""), + ); + + expect(rootColorVars(editor, "alice")).toEqual({ + light: "#abcdef", + dark: "#123456", + }); + }); + + it("derives the light tint for an author that only has a `color`", async () => { + const { editor } = createEditor({ colorLight: undefined }); + editor.replaceBlocks(editor.document, [{ content: "hello" }]); + + addInsertMark(editor, ["alice"]); + await vi.waitFor(() => + expect(rootColorVars(editor, "alice").dark).not.toBe(""), + ); + + expect(rootColorVars(editor, "alice")).toEqual({ + light: "color-mix(in srgb, #123456 30%, white)", + dark: "#123456", + }); + }); }); diff --git a/packages/core/src/y/extensions/AttributionExtension.ts b/packages/core/src/y/extensions/AttributionExtension.ts index 10e888830e..9ae77106ab 100644 --- a/packages/core/src/y/extensions/AttributionExtension.ts +++ b/packages/core/src/y/extensions/AttributionExtension.ts @@ -8,6 +8,7 @@ import { import { colorsForUserIds, userColorVarNames, + userMarkColors, normalizeToUserStore, type UserStoreOrResolver, } from "../../user/index.js"; @@ -213,9 +214,10 @@ export const AttributionExtension = createExtension( const syncRootVars = () => { for (const [id, user] of userStore.store.state) { const { light, dark } = userColorVarNames(id); - if (user.color && user.colorLight) { - dom.style.setProperty(light, user.colorLight); - dom.style.setProperty(dark, user.color); + const colors = userMarkColors(user); + if (colors) { + dom.style.setProperty(light, colors.light); + dom.style.setProperty(dark, colors.dark); } else { dom.style.removeProperty(light); dom.style.removeProperty(dark); diff --git a/packages/core/src/y/extensions/DiffVersioningExtension.test.ts b/packages/core/src/y/extensions/DiffVersioningExtension.test.ts index 968193b2bd..5c97740294 100644 --- a/packages/core/src/y/extensions/DiffVersioningExtension.test.ts +++ b/packages/core/src/y/extensions/DiffVersioningExtension.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test"; import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; import type { Block } from "../../blocks/defaultBlocks.js"; +import { colorsForUserIds } from "../../user/index.js"; import { AttributionExtension } from "./AttributionExtension.js"; import { DiffVersioningExtension } from "./DiffVersioningExtension.js"; @@ -165,6 +166,25 @@ describe("DiffVersioningExtension", () => { expect(attribution.userStore.getUser(authorId)?.username).toBe("Draft 3"); }); + it("colors the diff author with the palette's blue, tint included", async () => { + const baseline = blocksFromText("hello world"); + const target = blocksFromText("hello brave new world"); + + const diff = editor.getExtension(DiffVersioningExtension)!; + diff.renderDiff(target, baseline, "Draft 3"); + + const attribution = editor.getExtension(AttributionExtension)!; + const authorId = "version:Draft 3"; + await attribution.userStore.loadUsers([authorId]); + + // Both halves are set, so the marks and their tooltip use the tuned pair + // rather than a tint derived from the saturated colour. + expect(colorsForUserIds(attribution.userStore, [authorId])).toEqual({ + light: "#c9dcff", + dark: "#1e4fb0", + }); + }); + it("produces no attribution marks when the docs are identical", () => { const same = blocksFromText("nothing changes here"); diff --git a/packages/core/src/y/extensions/DiffVersioningExtension.ts b/packages/core/src/y/extensions/DiffVersioningExtension.ts index 651a11a205..5e8df768c7 100644 --- a/packages/core/src/y/extensions/DiffVersioningExtension.ts +++ b/packages/core/src/y/extensions/DiffVersioningExtension.ts @@ -29,8 +29,9 @@ const diffAuthorId = (label: string) => DIFF_AUTHOR_ID_PREFIX + label; /** Fallback label used when a diff is rendered without a version name. */ const DEFAULT_DIFF_LABEL = "This version"; -/** Color used for the version diff marks. */ -const DIFF_AUTHOR_COLOR = "#4363d8"; +/** Colors used for the version diff marks — the palette's blue. */ +const DIFF_AUTHOR_COLOR = "#1e4fb0"; +const DIFF_AUTHOR_COLOR_LIGHT = "#c9dcff"; export type DiffVersioningExtensionOptions = { /** @@ -112,6 +113,10 @@ export const DiffVersioningExtension = createExtension( editor: BlockNoteEditor; }) => { const color = options?.color ?? DIFF_AUTHOR_COLOR; + // Only the default pairs with a hand-tuned light tint; a caller-supplied + // colour gets the derived one (see `userMarkColors`). + const colorLight = + options?.color === undefined ? DIFF_AUTHOR_COLOR_LIGHT : undefined; // Resolve a synthetic author id back to its version label. The id encodes // the label (`version: