From 82411f870756991d3c2d3dcec8ee953c8be5e693 Mon Sep 17 00:00:00 2001 From: claude_code Date: Thu, 2 Jul 2026 17:14:19 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(editor):=20=D0=BD=D0=B5=20=D0=BF=D0=BE?= =?UTF-8?q?=D0=B4=D1=81=D1=82=D0=B0=D0=B2=D0=BB=D1=8F=D1=82=D1=8C=20=D1=82?= =?UTF-8?q?=D0=B8=D0=BF=D0=BE=D0=B3=D1=80=D0=B0=D1=84=D0=B8=D0=BA=D1=83=20?= =?UTF-8?q?=D0=BF=D0=BE=D0=B2=D1=82=D0=BE=D1=80=D0=BD=D0=BE=20=D0=BF=D0=BE?= =?UTF-8?q?=D1=81=D0=BB=D0=B5=20=D0=B5=D1=91=20=D0=BE=D1=82=D0=BC=D0=B5?= =?UTF-8?q?=D0=BD=D1=8B=20(Ctrl+Z)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit После срабатывания авто-подстановки Typography (например «1/2 » → «½») и её отмены через Ctrl+Z повторное нажатие пробела снова триггерило то же input-rule и подставляло символ заново. Добавлено клиентское расширение CustomTypography (обёртка над @tiptap/extension-typography) с ProseMirror-плагином «undo guard»: - запоминает диапазон текста, восстановленный отменой (undo/redo), и подавляет typography input-rules, чьё совпадение пересекается с этим диапазоном, пока восстановленный текст не отредактируют; - поддерживает обе системы истории: prosemirror-history (шаблонные редакторы) и yjs UndoManager (основной collab-редактор). Undo в yjs приходит как замена всего документа, поэтому регион вычисляется диффом документов (findDiffStart/findDiffEnd), а не по step-map; - детекция yjs-транзакций — через импортированный ySyncPluginKey и канонический isChangeOrigin, без хрупких строковых ключей. Co-Authored-By: Claude Fable 5 --- .../editor/extensions/custom-typography.ts | 186 ++++++++++++++++++ .../features/editor/extensions/extensions.ts | 6 +- 2 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 apps/client/src/features/editor/extensions/custom-typography.ts diff --git a/apps/client/src/features/editor/extensions/custom-typography.ts b/apps/client/src/features/editor/extensions/custom-typography.ts new file mode 100644 index 00000000..33186aa3 --- /dev/null +++ b/apps/client/src/features/editor/extensions/custom-typography.ts @@ -0,0 +1,186 @@ +import { InputRule } from "@tiptap/core"; +import { + Plugin, + PluginKey, + type EditorState, + type Transaction, +} from "@tiptap/pm/state"; +import { Typography } from "@tiptap/extension-typography"; +import { isChangeOrigin } from "@tiptap/extension-collaboration"; +import { ySyncPluginKey } from "@tiptap/y-tiptap"; + +// Region restored by the latest undo — while it is intact, typography +// input rules overlapping it must not fire again. +interface UndoGuardRange { + from: number; + to: number; +} + +const undoGuardKey = new PluginKey( + "typographyUndoGuard", +); + +// prosemirror-history does not export its plugin key, so template-editor +// undo/redo is detected via the stable stringified key. Only one +// PluginKey("history") exists in the dependency tree, so "history$" is stable. +const HISTORY_META = "history$"; + +const isUndoRedoTransaction = (tr: Transaction): boolean => { + if (tr.getMeta(HISTORY_META)) { + return true; + } + // Read yjs undo/redo meta via the real ySyncPluginKey object (imported, not + // a fragile stringified key), which y-tiptap sets on Y.UndoManager changes. + const ySyncMeta = tr.getMeta(ySyncPluginKey) as + | { isUndoRedoOperation?: boolean } + | undefined; + return !!ySyncMeta?.isUndoRedoOperation; +}; + +interface DocChange { + from: number; + oldTo: number; + newTo: number; +} + +// Compute the minimal changed region between two docs. yjs undo/redo (and any +// remote change) arrives as a whole-document replace step, so the transaction +// step maps are useless — diff the docs to recover the real minimal change. +// Returns null when the docs are identical. +const findChangedRange = ( + oldState: EditorState, + newState: EditorState, +): DocChange | null => { + const start = oldState.doc.content.findDiffStart(newState.doc.content); + const end = oldState.doc.content.findDiffEnd(newState.doc.content); + if (start == null || end == null) { + return null; + } + let { a: oldTo, b: newTo } = end; + // Normalize overlapping diff bounds (repeated-content edge case). + if (oldTo < start) { + newTo += start - oldTo; + oldTo = start; + } + return { from: start, oldTo, newTo }; +}; + +// Map an armed guard range across a single document change described by a diff. +// Returns null when the change touches the guarded text itself (the restored +// substitution was edited, so the guard must be released). +const mapRangeThroughChange = ( + range: UndoGuardRange, + change: DocChange, +): UndoGuardRange | null => { + // Strict intersection: an edit exactly at a guard boundary (e.g. the user + // typing the suppressed space right after the restored text, or deleting it) + // must NOT drop the guard. + if (change.from < range.to && change.oldTo > range.from) { + return null; + } + // Change fully before the guard: shift the guard by the length delta. + if (change.oldTo <= range.from) { + const delta = change.newTo - change.oldTo; + return { from: range.from + delta, to: range.to + delta }; + } + // Change fully after the guard: positions are unaffected. + return range; +}; + +// Detect history/remote transactions that may arrive as a whole-document +// replace step: prosemirror-history undo/redo, or any yjs remote-origin change +// (isChangeOrigin is the canonical predicate already used across the app). +const isHistoryOrRemoteTransaction = (tr: Transaction): boolean => + !!tr.getMeta(HISTORY_META) || isChangeOrigin(tr); + +export const CustomTypography = Typography.extend({ + addProseMirrorPlugins() { + return [ + ...(this.parent?.() ?? []), + new Plugin({ + key: undoGuardKey, + state: { + init: () => null, + apply(tr, prev, oldState, newState): UndoGuardRange | null { + if (tr.docChanged && isHistoryOrRemoteTransaction(tr)) { + const change = findChangedRange(oldState, newState); + if (change == null) { + // Attribute-only or otherwise content-neutral change: keep the + // guard. + return prev; + } + // Arm the guard only when the LOCAL user's undo/redo REPLACED text + // (deleted + inserted) — the signature of reverting an input-rule + // substitution. Pure insertions/deletions and remote peer edits + // must not arm it. + if ( + isUndoRedoTransaction(tr) && + change.oldTo > change.from && + change.newTo > change.from + ) { + return { from: change.from, to: change.newTo }; + } + // Non-arming history/remote change: map the existing guard through + // the real diff instead of the (whole-document) step map. + if (!prev) { + return null; + } + return mapRangeThroughChange(prev, change); + } + if (!prev) { + return null; + } + if (!tr.docChanged) { + return prev; + } + // Ordinary local edit: minimal step maps are accurate and cheap. + let range: UndoGuardRange | null = prev; + for (const stepMap of tr.mapping.maps) { + const { from: rangeFrom, to: rangeTo } = range; + let touched = false; + stepMap.forEach((fromA, toA) => { + if (fromA < rangeTo && toA > rangeFrom) { + touched = true; + } + }); + if (touched) { + range = null; + break; + } + range = { + from: stepMap.map(rangeFrom, 1), + to: stepMap.map(rangeTo, -1), + }; + } + return range && range.to > range.from ? range : null; + }, + }, + }), + ]; + }, + + addInputRules() { + // Wrap every typography rule: skip it when its match overlaps the text + // just restored by undo, so an undone substitution is not re-applied. + return (this.parent?.() ?? []).map( + (rule) => + new InputRule({ + find: rule.find, + undoable: rule.undoable, + handler: (props) => { + const guard = undoGuardKey.getState(props.state); + if ( + guard && + props.range.from < guard.to && + props.range.to > guard.from + ) { + // Returning null skips this rule and lets the typed character + // be inserted as plain text. + return null; + } + return rule.handler(props); + }, + }), + ); + }, +}); diff --git a/apps/client/src/features/editor/extensions/extensions.ts b/apps/client/src/features/editor/extensions/extensions.ts index d09821d0..4232307c 100644 --- a/apps/client/src/features/editor/extensions/extensions.ts +++ b/apps/client/src/features/editor/extensions/extensions.ts @@ -6,7 +6,7 @@ import { TaskList, TaskItem } from "@tiptap/extension-list"; import { Placeholder, CharacterCount, UndoRedo } from "@tiptap/extensions"; import { Superscript } from "@tiptap/extension-superscript"; import SubScript from "@tiptap/extension-subscript"; -import { Typography } from "@tiptap/extension-typography"; +import { CustomTypography } from "./custom-typography"; import { TextStyle } from "@tiptap/extension-text-style"; import { Color } from "@tiptap/extension-color"; import { Youtube } from "@tiptap/extension-youtube"; @@ -245,7 +245,9 @@ export const mainExtensions = [ return ReactMarkViewRenderer(SpoilerView); }, }), - Typography, + // Typography with an undo guard: does not re-apply a substitution the user + // just undid (e.g. Ctrl+Z on "1/2" -> "½" followed by another space). + CustomTypography, TrailingNode, GlobalDragHandle.configure({ customNodes: ["transclusionSource", "transclusionReference", "pageEmbed"], -- 2.52.0 From db9ed51e0169081413abaa7bc0d38228267849ac Mon Sep 17 00:00:00 2001 From: agent_coder Date: Thu, 2 Jul 2026 22:18:14 +0300 Subject: [PATCH 2/2] fix(#296 review): symmetric diff-bound normalization + tests (F1/F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F2: findChangedRange only normalized the repeated-content INSERTION case (oldTofrom AND newTo>from; normalization leaves exactly one end ==from). F1: add custom-typography.test.ts (15 tests) via the real Editor path (mirrors intentional-clear.test.ts): findChangedRange normalization (insertion + the fixed deletion), mapRangeThroughChange release/boundary/shift, and arming (local undo-replace arms; remote y-sync change-origin does NOT; ordinary edit does NOT). Adds test-only exports (undoGuardKey, findChangedRange, mapRangeThroughChange). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../extensions/custom-typography.test.ts | 231 ++++++++++++++++++ .../editor/extensions/custom-typography.ts | 21 +- 2 files changed, 245 insertions(+), 7 deletions(-) create mode 100644 apps/client/src/features/editor/extensions/custom-typography.test.ts diff --git a/apps/client/src/features/editor/extensions/custom-typography.test.ts b/apps/client/src/features/editor/extensions/custom-typography.test.ts new file mode 100644 index 00000000..b391a55e --- /dev/null +++ b/apps/client/src/features/editor/extensions/custom-typography.test.ts @@ -0,0 +1,231 @@ +import { describe, it, expect } from "vitest"; +import { Editor } from "@tiptap/core"; +import { Document } from "@tiptap/extension-document"; +import { Paragraph } from "@tiptap/extension-paragraph"; +import { Text } from "@tiptap/extension-text"; +import { ySyncPluginKey } from "@tiptap/y-tiptap"; +import { + CustomTypography, + undoGuardKey, + findChangedRange, + mapRangeThroughChange, +} from "./custom-typography"; + +/** + * PR #296 — the collab-safe typography undo-guard is exercised through the REAL + * editor path: a fresh Editor with the CustomTypography extension, transactions + * tagged exactly the way prosemirror-history / y-tiptap tag undo & remote + * changes (`setMeta("history$", …)` and `setMeta(ySyncPluginKey, …)`), plus + * direct unit tests of the two pure diff helpers. No hand-poke of plugin state. + * + * ARMING MECHANISM (verified against custom-typography.ts source): + * - A transaction arms the guard only when it is BOTH history/remote + * (`getMeta("history$")` truthy, or `isChangeOrigin` via the ySync meta) + * AND an undo/redo (`getMeta("history$")` truthy, or ySync + * `isUndoRedoOperation`), AND its whole-doc diff is a REPLACE + * (change.oldTo > change.from && change.newTo > change.from). + * - `history$` is the stringified PluginKey of the single prosemirror-history + * plugin; ProseMirror stores meta under `key.key`, so setMeta("history$") + * in a test is read identically by the extension's getMeta("history$"). + */ + +const singlePara = (text: string) => ({ + type: "doc", + content: [{ type: "paragraph", content: [{ type: "text", text }] }], +}); + +const makeEditor = (text: string) => + new Editor({ + extensions: [Document, Paragraph, Text, CustomTypography], + content: singlePara(text), + }); + +// Build a before/after EditorState pair by applying one plain transaction. +const mutate = (text: string, apply: (tr: any, schema: any) => void) => { + const editor = new Editor({ + extensions: [Document, Paragraph, Text], + content: singlePara(text), + }); + const before = editor.state; + const tr = before.tr; + apply(tr, before.schema); + editor.view.dispatch(tr); + const after = editor.state; + return { before, after, editor }; +}; + +describe("findChangedRange", () => { + it("returns null for identical docs", () => { + const editor = new Editor({ + extensions: [Document, Paragraph, Text], + content: singlePara("hello"), + }); + expect(findChangedRange(editor.state, editor.state)).toBeNull(); + editor.destroy(); + }); + + it("returns the minimal range for a normal middle insertion", () => { + // "hello world" (text at 1..12); insert "there " at pos 6. + const { before, after, editor } = mutate("hello world", (tr) => + tr.insertText("there ", 6), + ); + expect(findChangedRange(before, after)).toEqual({ + from: 6, + oldTo: 6, + newTo: 12, + }); + editor.destroy(); + }); + + it("normalizes the INSERTION overlapping-bounds branch (repeated content)", () => { + // Insert one more 'a' into "aaaaa" at pos 3. findDiffStart lands at the end + // (6) while findDiffEnd reports an end BEFORE it ({a:1,b:2}); both ends must + // be pushed forward by the same delta -> a non-degenerate range. + const { before, after, editor } = mutate("aaaaa", (tr) => + tr.insertText("a", 3), + ); + const change = findChangedRange(before, after)!; + expect(change).toEqual({ from: 6, oldTo: 6, newTo: 7 }); + // Invariant the guard logic relies on: never degenerate. + expect(change.from).toBeLessThanOrEqual(change.oldTo); + expect(change.from).toBeLessThanOrEqual(change.newTo); + editor.destroy(); + }); + + it("normalizes the DELETION overlapping-bounds branch (F2 fix)", () => { + // Delete one repeated 'a' from the middle of "aaaaa" ([3,4)). Here + // findDiffEnd reports newTo < start, the symmetric case the old one-sided + // normalization missed -> it used to yield a degenerate range (newTo < from). + const { before, after, editor } = mutate("aaaaa", (tr) => tr.delete(3, 4)); + const change = findChangedRange(before, after)!; + expect(change).toEqual({ from: 5, oldTo: 6, newTo: 5 }); + // The whole point of F2: from <= newTo (and from <= oldTo) still holds. + expect(change.from).toBeLessThanOrEqual(change.newTo); + expect(change.from).toBeLessThanOrEqual(change.oldTo); + editor.destroy(); + }); + + it("normalizes a multi-char repeated deletion (F2 fix)", () => { + const { before, after, editor } = mutate("aaaaa", (tr) => tr.delete(2, 4)); + const change = findChangedRange(before, after)!; + expect(change).toEqual({ from: 4, oldTo: 6, newTo: 4 }); + expect(change.from).toBeLessThanOrEqual(change.newTo); + editor.destroy(); + }); +}); + +describe("mapRangeThroughChange", () => { + const range = { from: 5, to: 10 }; + + it("RELEASES on a strict intersection (edit inside the guarded range)", () => { + // change straddles the interior of the guard. + expect( + mapRangeThroughChange(range, { from: 6, oldTo: 8, newTo: 7 }), + ).toBeNull(); + }); + + it("does NOT release on a boundary touch at the guard END", () => { + // Edit begins exactly at range.to (10): from < to is false -> no intersect. + expect( + mapRangeThroughChange(range, { from: 10, oldTo: 10, newTo: 12 }), + ).toEqual(range); + }); + + it("does NOT release on a boundary touch at the guard START", () => { + // Edit ends exactly at range.from (5): oldTo > from is false -> no intersect; + // it is treated as a change fully before, shifting the guard. + expect( + mapRangeThroughChange(range, { from: 3, oldTo: 5, newTo: 8 }), + ).toEqual({ from: 8, to: 13 }); + }); + + it("SHIFTS the guard for a change fully before it", () => { + // Insert 2 chars entirely before the range (oldTo 3 <= from 5): +2 delta. + expect( + mapRangeThroughChange(range, { from: 2, oldTo: 3, newTo: 5 }), + ).toEqual({ from: 7, to: 12 }); + }); + + it("leaves the guard untouched for a change fully after it", () => { + expect( + mapRangeThroughChange(range, { from: 12, oldTo: 14, newTo: 16 }), + ).toBe(range); + }); +}); + +describe("undo-guard arming (integration)", () => { + it("arms {from, to:newTo} on a LOCAL undo-replace (history meta)", () => { + // Undo of an em-dash substitution: "a—b" restored to "a--b" — the em-dash + // (pos 2..3) is REPLACED by "--", tagged with the history plugin's meta. + const editor = makeEditor("a—b"); + const { state } = editor; + const tr = state.tr + .replaceWith(2, 3, state.schema.text("--")) + .setMeta("history$", { redo: false }); + editor.view.dispatch(tr); + + expect(editor.state.doc.textContent).toBe("a--b"); + // from = diff start (2), to = newTo = end of the inserted "--" (4). + expect(undoGuardKey.getState(editor.state)).toEqual({ from: 2, to: 4 }); + editor.destroy(); + }); + + it("does NOT arm on a REMOTE change-origin replace (no undo meta)", () => { + // Same replace, but tagged only as a y-sync remote change: history/remote + // yes, undo/redo NO -> must not arm. + const editor = makeEditor("a—b"); + const { state } = editor; + const tr = state.tr + .replaceWith(2, 3, state.schema.text("--")) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + editor.view.dispatch(tr); + + expect(editor.state.doc.textContent).toBe("a--b"); + expect(undoGuardKey.getState(editor.state)).toBeNull(); + editor.destroy(); + }); + + it("does NOT arm on an ordinary local edit", () => { + const editor = makeEditor("a—b"); + editor.view.dispatch( + editor.state.tr.replaceWith(2, 3, editor.state.schema.text("--")), + ); + expect(undoGuardKey.getState(editor.state)).toBeNull(); + editor.destroy(); + }); +}); + +describe("undo-guard release / shift (integration)", () => { + it("RELEASES when a later edit lands inside the guarded region", () => { + const editor = makeEditor("a—b"); + editor.view.dispatch( + editor.state.tr + .replaceWith(2, 3, editor.state.schema.text("--")) + .setMeta("history$", { redo: false }), + ); + const guard = undoGuardKey.getState(editor.state)!; + expect(guard).toEqual({ from: 2, to: 4 }); + + // Type a character inside the restored region -> guard is dropped. + editor.view.dispatch(editor.state.tr.insertText("x", guard.from + 1)); + expect(undoGuardKey.getState(editor.state)).toBeNull(); + editor.destroy(); + }); + + it("keeps and SHIFTS the guard when a later edit lands before it", () => { + const editor = makeEditor("zz a—b"); + // "zz a—b": em-dash at pos 5; replace the 'a' at 4..5 with "--" to arm. + editor.view.dispatch( + editor.state.tr + .replaceWith(4, 5, editor.state.schema.text("--")) + .setMeta("history$", { redo: false }), + ); + const guard = undoGuardKey.getState(editor.state)!; + expect(guard).toEqual({ from: 4, to: 6 }); + + // Insert one char at the very start (before the guard) -> guard shifts +1. + editor.view.dispatch(editor.state.tr.insertText("Q", 1)); + expect(undoGuardKey.getState(editor.state)).toEqual({ from: 5, to: 7 }); + editor.destroy(); + }); +}); diff --git a/apps/client/src/features/editor/extensions/custom-typography.ts b/apps/client/src/features/editor/extensions/custom-typography.ts index 33186aa3..aab93f16 100644 --- a/apps/client/src/features/editor/extensions/custom-typography.ts +++ b/apps/client/src/features/editor/extensions/custom-typography.ts @@ -16,7 +16,9 @@ interface UndoGuardRange { to: number; } -const undoGuardKey = new PluginKey( +// Exported for tests: the plugin key lets a test read the armed guard state, +// and the two pure helpers below are unit-tested directly. +export const undoGuardKey = new PluginKey( "typographyUndoGuard", ); @@ -47,7 +49,7 @@ interface DocChange { // remote change) arrives as a whole-document replace step, so the transaction // step maps are useless — diff the docs to recover the real minimal change. // Returns null when the docs are identical. -const findChangedRange = ( +export const findChangedRange = ( oldState: EditorState, newState: EditorState, ): DocChange | null => { @@ -57,10 +59,15 @@ const findChangedRange = ( return null; } let { a: oldTo, b: newTo } = end; - // Normalize overlapping diff bounds (repeated-content edge case). - if (oldTo < start) { - newTo += start - oldTo; - oldTo = start; + // findDiffEnd can report an end BEFORE the diff start when the changed text + // abuts repeated content (insertion -> oldTo newTo { -- 2.52.0