From 676fdd92ce4b2e774be4ffd0a9f3907fdf6128f0 Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 8 Oct 2026 16:19:24 +0200 Subject: [PATCH 1/3] feat(versioning): show copied blocks once in version diffs (experimental) Behind `experimental.blockCopyDiffs` in the collaboration options; it only changes what a version diff shows. A type change or a move stores a copy of the block, so the diff showed it as a deleted and an inserted block, with the copied content credited to whoever made the change. Pair each copy with its original (by block id) and show it once: - a type change between text blocks as a formatting change; - a move, including an indent, as "Moved"; - a copy in the same place (its children changed) as unchanged. Copied content keeps its authors. Content the earlier version had that no copy kept still shows as deleted. --- .../14-suggestion-gallery/src/App.tsx | 4 + .../14-suggestion-gallery/src/scenarios.ts | 60 +++ packages/core/src/editor/Block.css | 26 + packages/core/src/i18n/locales/ar.ts | 2 + packages/core/src/i18n/locales/de.ts | 2 + packages/core/src/i18n/locales/en.ts | 2 + packages/core/src/i18n/locales/es.ts | 2 + packages/core/src/i18n/locales/fa.ts | 2 + packages/core/src/i18n/locales/fr.ts | 2 + packages/core/src/i18n/locales/he.ts | 2 + packages/core/src/i18n/locales/hr.ts | 2 + packages/core/src/i18n/locales/is.ts | 2 + packages/core/src/i18n/locales/it.ts | 2 + packages/core/src/i18n/locales/ja.ts | 2 + packages/core/src/i18n/locales/ko.ts | 2 + packages/core/src/i18n/locales/nl.ts | 2 + packages/core/src/i18n/locales/no.ts | 2 + packages/core/src/i18n/locales/pl.ts | 2 + packages/core/src/i18n/locales/pt.ts | 2 + packages/core/src/i18n/locales/ru.ts | 2 + packages/core/src/i18n/locales/sk.ts | 2 + packages/core/src/i18n/locales/uk.ts | 2 + packages/core/src/i18n/locales/uz.ts | 2 + packages/core/src/i18n/locales/vi.ts | 2 + packages/core/src/i18n/locales/zh-tw.ts | 2 + packages/core/src/i18n/locales/zh.ts | 2 + .../src/y/extensions/AttributionExtension.ts | 25 +- .../src/y/extensions/YAttributionMarks.ts | 7 + packages/core/src/y/extensions/YSync.ts | 13 +- .../versionDiffFlags.test.ts.snap | 84 ++++ .../src/y/extensions/nestingChanges.test.ts | 9 +- .../core/src/y/extensions/snapshotPreview.ts | 468 +++++++++++++++++- .../extensions/versionDiffAttribution.test.ts | 31 +- .../src/y/extensions/versionDiffFlags.test.ts | 201 ++++++++ packages/core/src/y/utils.test.ts | 11 +- .../AttributionTooltip/AttributionTooltip.tsx | 3 + .../__snapshots__/versioning.test.tsx.snap | 118 +---- .../y-prosemirror/versioning.test.tsx | 8 +- 38 files changed, 967 insertions(+), 147 deletions(-) create mode 100644 packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap create mode 100644 packages/core/src/y/extensions/versionDiffFlags.test.ts diff --git a/examples/07-collaboration/14-suggestion-gallery/src/App.tsx b/examples/07-collaboration/14-suggestion-gallery/src/App.tsx index aed1c13dfc..2cb7e11d9f 100644 --- a/examples/07-collaboration/14-suggestion-gallery/src/App.tsx +++ b/examples/07-collaboration/14-suggestion-gallery/src/App.tsx @@ -34,6 +34,10 @@ type Mode = "suggestions" | "versioning"; const FIXES: { value: VersionDiffFixes | undefined; label: string }[] = [ { value: undefined, label: "Default" }, { value: "implicitDeleteAttribution", label: "Implicit delete attribution" }, + { + value: "implicitDeleteAttributionAndRecreatedBlocks", + label: "+ re-created blocks", + }, ]; const ALL_FIXES: ExperimentalVersionDiffs = { versionDiffFixes: FIXES[FIXES.length - 1].value, diff --git a/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts b/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts index b18318cb40..c6887b570f 100644 --- a/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts +++ b/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts @@ -117,6 +117,26 @@ function posBeforeText(editor: GalleryEditor, text: string): number { return pos; } +// Versioning notes for a block copy: a type change or a move stores a copy of +// the block, which the `recreatedBlocks` fix shows once. +const typeChangeNote: Feedback = { + severity: "info", + when: { recreatedBlocks: true }, + note: "Versioning shows the type change as a formatting change.", +}; +const moveNote: Feedback = { + severity: "info", + when: { recreatedBlocks: true }, + note: "Versioning shows the block as moved: at its new place, and struck through at its old place unless it was only indented or outdented.", +}; +function lostEditNote(user: string): Feedback { + return { + severity: "info", + when: { recreatedBlocks: true }, + note: `Versioning shows only the type change: the result doesn't have ${user}'s edit, so the diff doesn't show it.`, + }; +} + export const scenarios: SuggestionScenario[] = [ { kind: "single", @@ -241,12 +261,14 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "nest-bullet-existing", feedback: [ + moveNote, { severity: "low", note: "Nested bullets all render as • instead of •/◦/▪ — the suggestion-mark wrappers (display: contents) break the depth-detecting CSS chains. Fix: compute each bullet's nesting level in JS and expose it as data-bullet-level, then pick the glyph with a wrapper-independent attribute selector (as numbered lists do with data-index).", }, { severity: "high", + when: { recreatedBlocks: false }, note: "Indenting re-creates Parent and Child as new blocks (the schema fix stores a block that gains or loses its children as a new block). The diff therefore shows both as deleted and inserted again, all credited to whoever indented.", }, ], @@ -413,8 +435,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "type-list-to-paragraph", feedback: [ + typeChangeNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Changing the type re-creates the block as a new one (with the schema fix, a block's type can't change in place). The diff therefore shows it as deleted and inserted again, all credited to whoever changed the type.", }, ], @@ -435,8 +459,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "type-paragraph-to-heading", feedback: [ + typeChangeNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Changing the type re-creates the block as a new one (with the schema fix, a block's type can't change in place). The diff therefore shows it as deleted and inserted again, all credited to whoever changed the type.", }, ], @@ -599,8 +625,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "move-paragraph-up", feedback: [ + moveNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Moving re-creates the block as a new one at its new place. The diff therefore shows it as deleted at its old place and inserted at its new one, all credited to the mover.", }, ], @@ -620,8 +648,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "move-paragraph-with-children", feedback: [ + moveNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Moving re-creates the block, with its child, as a new one at its new place. The diff therefore shows it as deleted at its old place and inserted at its new one, all credited to the mover.", }, ], @@ -646,8 +676,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "nesting-indent", feedback: [ + moveNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Indenting re-creates N0 and N1 as new blocks (the schema fix stores a block that gains or loses its children as a new block). The diff therefore shows both as deleted and inserted again, all credited to whoever indented.", }, ], @@ -669,8 +701,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "nesting-unindent", feedback: [ + moveNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Outdenting re-creates N0 and N1 as new blocks (the schema fix stores a block that gains or loses its children as a new block). The diff therefore shows N0 and N1 as deleted and inserted again, all credited to whoever outdented.", }, ], @@ -694,8 +728,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "nesting-change-parent-type", feedback: [ + typeChangeNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Changing the type re-creates N0, children included, as a new block (with the schema fix, a block's type can't change in place). The diff therefore shows it as deleted and inserted again, all credited to whoever changed the type.", }, ], @@ -1047,6 +1083,12 @@ export const scenarios: SuggestionScenario[] = [ feedback: [ { severity: "high", + when: { recreatedBlocks: true }, + note: "Versioning shows N0 unchanged and N2 as moved, but N1's two copies as inserted by A and by B.", + }, + { + severity: "high", + when: { recreatedBlocks: false }, note: "The diff also shows N0 and N2 as deleted and inserted again: indenting re-creates blocks (see Indent a block).", }, { @@ -1075,12 +1117,18 @@ export const scenarios: SuggestionScenario[] = [ kind: "concurrent", id: "concurrent-indent-vs-edit", feedback: [ + { + severity: "info", + when: { recreatedBlocks: true }, + note: "Versioning shows N0 unchanged and N1 as moved. The result doesn't have B's edit, so the diff doesn't show it.", + }, { severity: "low", note: "B's edit is lost: A's indent re-creates N1 as a new block, which doesn't have B's concurrent edit.", }, { severity: "high", + when: { recreatedBlocks: false }, note: "The diff also shows N0 and N1 as deleted and inserted again, all credited to A: indenting re-creates blocks (see Indent a block).", }, ], @@ -1106,6 +1154,12 @@ export const scenarios: SuggestionScenario[] = [ feedback: [ { severity: "high", + when: { recreatedBlocks: true }, + note: "Versioning shows R unchanged and B1–B3 as moved, but Q's two copies as inserted by B and by A.", + }, + { + severity: "high", + when: { recreatedBlocks: false }, note: "The diff also shows R and B1–B3 as deleted and inserted again: indenting and moving re-create blocks.", }, { @@ -1136,8 +1190,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "concurrent", id: "concurrent-parent-type-vs-child-edit", feedback: [ + lostEditNote("B"), { severity: "high", + when: { recreatedBlocks: false }, note: "The diff also shows Parent and Child as deleted and inserted again, all credited to A: changing the type re-creates blocks (see Change type of a parent block).", }, { @@ -1354,8 +1410,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "concurrent", id: "concurrent-text-vs-heading", feedback: [ + lostEditNote("A"), { severity: "high", + when: { recreatedBlocks: false }, note: "The diff shows the block as deleted and inserted again, all credited to B: changing the type re-creates it.", }, { @@ -1735,8 +1793,10 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "remove-1-column", feedback: [ + moveNote, { severity: "high", + when: { recreatedBlocks: false }, note: "Removing the column re-creates Left column as a new block outside the columns. The diff therefore shows it as inserted, as if it were new, credited to whoever removed the column.", }, ], diff --git a/packages/core/src/editor/Block.css b/packages/core/src/editor/Block.css index 401ed05a03..f5b291ca8a 100644 --- a/packages/core/src/editor/Block.css +++ b/packages/core/src/editor/Block.css @@ -1122,6 +1122,32 @@ content span instead, using the `--user-color-*` properties that cascade down from the wrapper. The `.bn-root ins, .bn-root del` rules above still style serialized/static output, where the wrapper is a real, painted box. */ +/* A moved block's text keeps its authors, so it has no mark of its own: its + text element gets the inserted-text highlight instead. It shrinks to the text + (a flex item), or breaks per line in code blocks (an inline ). */ +ins[data-moved] > .bn-suggestion-node .bn-inline-content { + --bn-suggestion-token-color: currentColor; + box-decoration-break: clone; + -webkit-box-decoration-break: clone; + background-color: color-mix(in srgb, var(--user-color-light) 50%, white); + color: var(--user-color-dark); + border-radius: 4px; +} + +.dark.bn-root ins[data-moved] > .bn-suggestion-node .bn-inline-content, +ins[data-moved] + > .bn-suggestion-node + .bn-block-content[data-content-type="codeBlock"] + > pre + .bn-inline-content { + background-color: color-mix( + in srgb, + var(--user-color-light) 25%, + var(--bn-suggestion-surface, var(--bn-colors-editor-background)) + ); + color: white; +} + .bn-suggestion-mark { --bn-suggestion-token-color: currentColor; background-color: color-mix(in srgb, var(--user-color-light) 50%, white); diff --git a/packages/core/src/i18n/locales/ar.ts b/packages/core/src/i18n/locales/ar.ts index 9bc6ba0de7..1e6601c828 100644 --- a/packages/core/src/i18n/locales/ar.ts +++ b/packages/core/src/i18n/locales/ar.ts @@ -399,6 +399,8 @@ export const ar: Dictionary = { deleted_by: (users: string) => `حُذف بواسطة: ${users}`, changed: "مُعدَّل", changed_by: (users: string) => `عُدّل بواسطة: ${users}`, + moved: "تم النقل", + moved_by: (users: string) => `نقله: ${users}`, formatting_change_by: (formats: string, users: string) => `تغيير التنسيق (${formats}) بواسطة: ${users}`, }, diff --git a/packages/core/src/i18n/locales/de.ts b/packages/core/src/i18n/locales/de.ts index 537bc5acd1..38d8d38bea 100644 --- a/packages/core/src/i18n/locales/de.ts +++ b/packages/core/src/i18n/locales/de.ts @@ -433,6 +433,8 @@ export const de: Dictionary = { deleted_by: (users: string) => `Gelöscht von: ${users}`, changed: "Geändert", changed_by: (users: string) => `Geändert von: ${users}`, + moved: "Verschoben", + moved_by: (users: string) => `Verschoben von: ${users}`, formatting_change_by: (formats: string, users: string) => `Formatierungsänderung (${formats}) von: ${users}`, }, diff --git a/packages/core/src/i18n/locales/en.ts b/packages/core/src/i18n/locales/en.ts index c87731fcf1..e73c2d53a4 100644 --- a/packages/core/src/i18n/locales/en.ts +++ b/packages/core/src/i18n/locales/en.ts @@ -414,6 +414,8 @@ export const en = { deleted_by: (users: string) => `Deleted by: ${users}`, changed: "Changed", changed_by: (users: string) => `Changed by: ${users}`, + moved: "Moved", + moved_by: (users: string) => `Moved by: ${users}`, formatting_change_by: (formats: string, users: string) => `Formatting change (${formats}) by: ${users}`, }, diff --git a/packages/core/src/i18n/locales/es.ts b/packages/core/src/i18n/locales/es.ts index 96fa83e013..094f6b9bd3 100644 --- a/packages/core/src/i18n/locales/es.ts +++ b/packages/core/src/i18n/locales/es.ts @@ -412,6 +412,8 @@ export const es: Dictionary = { deleted_by: (users: string) => `Eliminado por: ${users}`, changed: "Modificado", changed_by: (users: string) => `Modificado por: ${users}`, + moved: "Movido", + moved_by: (users: string) => `Movido por: ${users}`, formatting_change_by: (formats: string, users: string) => `Cambio de formato (${formats}) por: ${users}`, }, diff --git a/packages/core/src/i18n/locales/fa.ts b/packages/core/src/i18n/locales/fa.ts index 35b907ac13..7c3d043249 100644 --- a/packages/core/src/i18n/locales/fa.ts +++ b/packages/core/src/i18n/locales/fa.ts @@ -383,6 +383,8 @@ export const fa = { deleted_by: (users: string) => `حذف‌شده توسط: ${users}`, changed: "تغییر\u200cیافته", changed_by: (users: string) => `تغییر\u200cیافته توسط: ${users}`, + moved: "منتقل شد", + moved_by: (users: string) => `منتقل شده توسط: ${users}`, formatting_change_by: (formats: string, users: string) => `تغییر قالب‌بندی (${formats}) توسط: ${users}`, }, diff --git a/packages/core/src/i18n/locales/fr.ts b/packages/core/src/i18n/locales/fr.ts index 653fd9581b..b184550d71 100644 --- a/packages/core/src/i18n/locales/fr.ts +++ b/packages/core/src/i18n/locales/fr.ts @@ -460,6 +460,8 @@ export const fr: Dictionary = { deleted_by: (users: string) => `Supprimé par : ${users}`, changed: "Modifié", changed_by: (users: string) => `Modifié par : ${users}`, + moved: "Déplacé", + moved_by: (users: string) => `Déplacé par : ${users}`, formatting_change_by: (formats: string, users: string) => `Modification de mise en forme (${formats}) par : ${users}`, }, diff --git a/packages/core/src/i18n/locales/he.ts b/packages/core/src/i18n/locales/he.ts index 0e51f20987..46e0792ead 100644 --- a/packages/core/src/i18n/locales/he.ts +++ b/packages/core/src/i18n/locales/he.ts @@ -414,6 +414,8 @@ export const he: Dictionary = { deleted_by: (users: string) => `נמחק על ידי: ${users}`, changed: "שונה", changed_by: (users: string) => `שונה על ידי: ${users}`, + moved: "הועבר", + moved_by: (users: string) => `הועבר על ידי: ${users}`, formatting_change_by: (formats: string, users: string) => `שינוי עיצוב (${formats}) על ידי: ${users}`, }, diff --git a/packages/core/src/i18n/locales/hr.ts b/packages/core/src/i18n/locales/hr.ts index e7e1dc1425..27b64b09e3 100644 --- a/packages/core/src/i18n/locales/hr.ts +++ b/packages/core/src/i18n/locales/hr.ts @@ -428,6 +428,8 @@ export const hr: Dictionary = { deleted_by: (users: string) => `Izbrisao/la: ${users}`, changed: "Promijenjeno", changed_by: (users: string) => `Promijenio/la: ${users}`, + moved: "Premješteno", + moved_by: (users: string) => `Premjestio: ${users}`, formatting_change_by: (formats: string, users: string) => `Promjena oblikovanja (${formats}) od: ${users}`, }, diff --git a/packages/core/src/i18n/locales/is.ts b/packages/core/src/i18n/locales/is.ts index 55d3a860aa..416507a257 100644 --- a/packages/core/src/i18n/locales/is.ts +++ b/packages/core/src/i18n/locales/is.ts @@ -428,6 +428,8 @@ export const is: Dictionary = { deleted_by: (users: string) => `Eytt af: ${users}`, changed: "Breytt", changed_by: (users: string) => `Breytt af: ${users}`, + moved: "Fært", + moved_by: (users: string) => `Fært af: ${users}`, formatting_change_by: (formats: string, users: string) => `Sniðbreyting (${formats}) af: ${users}`, }, diff --git a/packages/core/src/i18n/locales/it.ts b/packages/core/src/i18n/locales/it.ts index 005ceb6f7a..ceb9e98283 100644 --- a/packages/core/src/i18n/locales/it.ts +++ b/packages/core/src/i18n/locales/it.ts @@ -436,6 +436,8 @@ export const it: Dictionary = { deleted_by: (users: string) => `Eliminato da: ${users}`, changed: "Modificato", changed_by: (users: string) => `Modificato da: ${users}`, + moved: "Spostato", + moved_by: (users: string) => `Spostato da: ${users}`, formatting_change_by: (formats: string, users: string) => `Modifica formattazione (${formats}) da: ${users}`, }, diff --git a/packages/core/src/i18n/locales/ja.ts b/packages/core/src/i18n/locales/ja.ts index 93aa440e43..4dee0de0a4 100644 --- a/packages/core/src/i18n/locales/ja.ts +++ b/packages/core/src/i18n/locales/ja.ts @@ -454,6 +454,8 @@ export const ja: Dictionary = { deleted_by: (users: string) => `削除者: ${users}`, changed: "変更済み", changed_by: (users: string) => `変更者: ${users}`, + moved: "移動済み", + moved_by: (users: string) => `移動者：${users}`, formatting_change_by: (formats: string, users: string) => `書式の変更 (${formats}) 変更者: ${users}`, }, diff --git a/packages/core/src/i18n/locales/ko.ts b/packages/core/src/i18n/locales/ko.ts index 85c4addb75..cb98342c44 100644 --- a/packages/core/src/i18n/locales/ko.ts +++ b/packages/core/src/i18n/locales/ko.ts @@ -427,6 +427,8 @@ export const ko: Dictionary = { deleted_by: (users: string) => `삭제한 사람: ${users}`, changed: "변경됨", changed_by: (users: string) => `변경한 사람: ${users}`, + moved: "이동됨", + moved_by: (users: string) => `이동한 사용자: ${users}`, formatting_change_by: (formats: string, users: string) => `서식 변경 (${formats}) 변경한 사람: ${users}`, }, diff --git a/packages/core/src/i18n/locales/nl.ts b/packages/core/src/i18n/locales/nl.ts index 15a65c98b2..daac1b45fb 100644 --- a/packages/core/src/i18n/locales/nl.ts +++ b/packages/core/src/i18n/locales/nl.ts @@ -415,6 +415,8 @@ export const nl: Dictionary = { deleted_by: (users: string) => `Verwijderd door: ${users}`, changed: "Gewijzigd", changed_by: (users: string) => `Gewijzigd door: ${users}`, + moved: "Verplaatst", + moved_by: (users: string) => `Verplaatst door: ${users}`, formatting_change_by: (formats: string, users: string) => `Opmaakwijziging (${formats}) door: ${users}`, }, diff --git a/packages/core/src/i18n/locales/no.ts b/packages/core/src/i18n/locales/no.ts index d67066c260..d31a2206d8 100644 --- a/packages/core/src/i18n/locales/no.ts +++ b/packages/core/src/i18n/locales/no.ts @@ -432,6 +432,8 @@ export const no: Dictionary = { deleted_by: (users: string) => `Slettet av: ${users}`, changed: "Endret", changed_by: (users: string) => `Endret av: ${users}`, + moved: "Flyttet", + moved_by: (users: string) => `Flyttet av: ${users}`, formatting_change_by: (formats: string, users: string) => `Formateringsendring (${formats}) av: ${users}`, }, diff --git a/packages/core/src/i18n/locales/pl.ts b/packages/core/src/i18n/locales/pl.ts index f9da477084..5c348fbb26 100644 --- a/packages/core/src/i18n/locales/pl.ts +++ b/packages/core/src/i18n/locales/pl.ts @@ -405,6 +405,8 @@ export const pl: Dictionary = { deleted_by: (users: string) => `Usunięte przez: ${users}`, changed: "Zmieniono", changed_by: (users: string) => `Zmienione przez: ${users}`, + moved: "Przeniesiono", + moved_by: (users: string) => `Przeniesione przez: ${users}`, formatting_change_by: (formats: string, users: string) => `Zmiana formatowania (${formats}) przez: ${users}`, }, diff --git a/packages/core/src/i18n/locales/pt.ts b/packages/core/src/i18n/locales/pt.ts index cb5d10361e..46367cfc75 100644 --- a/packages/core/src/i18n/locales/pt.ts +++ b/packages/core/src/i18n/locales/pt.ts @@ -407,6 +407,8 @@ export const pt: Dictionary = { deleted_by: (users: string) => `Excluído por: ${users}`, changed: "Alterado", changed_by: (users: string) => `Alterado por: ${users}`, + moved: "Movido", + moved_by: (users: string) => `Movido por: ${users}`, formatting_change_by: (formats: string, users: string) => `Alteração de formatação (${formats}) por: ${users}`, }, diff --git a/packages/core/src/i18n/locales/ru.ts b/packages/core/src/i18n/locales/ru.ts index 1e942be3c8..15534a94cd 100644 --- a/packages/core/src/i18n/locales/ru.ts +++ b/packages/core/src/i18n/locales/ru.ts @@ -458,6 +458,8 @@ export const ru: Dictionary = { deleted_by: (users: string) => `Удалено: ${users}`, changed: "Изменено", changed_by: (users: string) => `Изменено: ${users}`, + moved: "Перемещено", + moved_by: (users: string) => `Перемещено пользователем: ${users}`, formatting_change_by: (formats: string, users: string) => `Изменение форматирования (${formats}): ${users}`, }, diff --git a/packages/core/src/i18n/locales/sk.ts b/packages/core/src/i18n/locales/sk.ts index d3b3e3ce03..b65eb1da4f 100644 --- a/packages/core/src/i18n/locales/sk.ts +++ b/packages/core/src/i18n/locales/sk.ts @@ -412,6 +412,8 @@ export const sk = { deleted_by: (users: string) => `Odstránil: ${users}`, changed: "Zmenené", changed_by: (users: string) => `Zmenil: ${users}`, + moved: "Presunuté", + moved_by: (users: string) => `Presunul: ${users}`, formatting_change_by: (formats: string, users: string) => `Zmena formátovania (${formats}) od: ${users}`, }, diff --git a/packages/core/src/i18n/locales/uk.ts b/packages/core/src/i18n/locales/uk.ts index b71784b1fd..93dbb930a2 100644 --- a/packages/core/src/i18n/locales/uk.ts +++ b/packages/core/src/i18n/locales/uk.ts @@ -438,6 +438,8 @@ export const uk: Dictionary = { deleted_by: (users: string) => `Видалено користувачем: ${users}`, changed: "Змінено", changed_by: (users: string) => `Змінено користувачем: ${users}`, + moved: "Переміщено", + moved_by: (users: string) => `Переміщено користувачем: ${users}`, formatting_change_by: (formats: string, users: string) => `Зміна форматування (${formats}) користувачем: ${users}`, }, diff --git a/packages/core/src/i18n/locales/uz.ts b/packages/core/src/i18n/locales/uz.ts index f057f88ca9..26146c843e 100644 --- a/packages/core/src/i18n/locales/uz.ts +++ b/packages/core/src/i18n/locales/uz.ts @@ -448,6 +448,8 @@ export const uz: Dictionary = { deleted_by: (users: string) => `O'chirgan: ${users}`, changed: "O'zgartirildi", changed_by: (users: string) => `O'zgartirgan: ${users}`, + moved: "Ko'chirildi", + moved_by: (users: string) => `Ko'chirgan: ${users}`, formatting_change_by: (formats: string, users: string) => `Formatlash o'zgarishi (${formats}), o'zgartirgan: ${users}`, }, diff --git a/packages/core/src/i18n/locales/vi.ts b/packages/core/src/i18n/locales/vi.ts index 801ce1bdf6..218632b2fc 100644 --- a/packages/core/src/i18n/locales/vi.ts +++ b/packages/core/src/i18n/locales/vi.ts @@ -413,6 +413,8 @@ export const vi: Dictionary = { deleted_by: (users: string) => `Được xóa bởi: ${users}`, changed: "Đã thay đổi", changed_by: (users: string) => `Được thay đổi bởi: ${users}`, + moved: "Đã di chuyển", + moved_by: (users: string) => `Được di chuyển bởi: ${users}`, formatting_change_by: (formats: string, users: string) => `Thay đổi định dạng (${formats}) bởi: ${users}`, }, diff --git a/packages/core/src/i18n/locales/zh-tw.ts b/packages/core/src/i18n/locales/zh-tw.ts index 7f3626c55c..aa7946da15 100644 --- a/packages/core/src/i18n/locales/zh-tw.ts +++ b/packages/core/src/i18n/locales/zh-tw.ts @@ -455,6 +455,8 @@ export const zhTW: Dictionary = { deleted_by: (users: string) => `刪除者：${users}`, changed: "已變更", changed_by: (users: string) => `變更者：${users}`, + moved: "已移動", + moved_by: (users: string) => `移動者：${users}`, formatting_change_by: (formats: string, users: string) => `格式變更（${formats}），變更者：${users}`, }, diff --git a/packages/core/src/i18n/locales/zh.ts b/packages/core/src/i18n/locales/zh.ts index e485eb943b..4416142c46 100644 --- a/packages/core/src/i18n/locales/zh.ts +++ b/packages/core/src/i18n/locales/zh.ts @@ -455,6 +455,8 @@ export const zh: Dictionary = { deleted_by: (users: string) => `删除者：${users}`, changed: "已更改", changed_by: (users: string) => `更改者：${users}`, + moved: "已移动", + moved_by: (users: string) => `移动者：${users}`, formatting_change_by: (formats: string, users: string) => `格式更改（${formats}），更改者：${users}`, }, diff --git a/packages/core/src/y/extensions/AttributionExtension.ts b/packages/core/src/y/extensions/AttributionExtension.ts index 4f47e96a8b..4c1a3a5b39 100644 --- a/packages/core/src/y/extensions/AttributionExtension.ts +++ b/packages/core/src/y/extensions/AttributionExtension.ts @@ -80,7 +80,8 @@ export type AttributionChange = | { // `change`: a preview (e.g. a diagram) can't show what was inserted or // deleted in its hidden source, only that it changed. - modificationType: "insert" | "delete" | "change"; + // `move`: a moved block at its new place, or its struck-through original. + modificationType: "insert" | "delete" | "change" | "move"; format?: never; attributes?: never; } @@ -239,7 +240,8 @@ export const AttributionExtension = createExtension( const attributionIdentity = (wrapper: HTMLElement) => { const ids = parseUserIds(wrapper.dataset["userIds"]); const format = parseFormatKeys(wrapper.dataset["format"]); - return `${wrapper.tagName}:${wrapper.dataset["attributes"] ?? ""}:${format.join(",")}:${ids.join(",")}`; + const moved = wrapper.dataset["moved"] !== undefined ? "moved" : ""; + return `${wrapper.tagName}${moved}:${wrapper.dataset["attributes"] ?? ""}:${format.join(",")}:${ids.join(",")}`; }; // Build the tooltip state from a wrapper's `data-*` attributes. A @@ -249,7 +251,9 @@ export const AttributionExtension = createExtension( preview?: Element, ): AttributionTooltipState => { const markChange: AttributionChange & { - modificationType: AttributionMarkStyleInfo["modificationType"]; + modificationType: + | AttributionMarkStyleInfo["modificationType"] + | "move"; } = anchor.dataset["attributes"] !== undefined ? { @@ -263,9 +267,20 @@ export const AttributionExtension = createExtension( } : { modificationType: - anchor.tagName === "INS" ? "insert" : "delete", + anchor.dataset["moved"] !== undefined + ? "move" + : anchor.tagName === "INS" + ? "insert" + : "delete", }; - const { modificationType } = markChange; + // For styling, a moved block is inserted at its new place and + // deleted at its old one. + const modificationType = + markChange.modificationType === "move" + ? anchor.tagName === "INS" + ? "insert" + : "delete" + : markChange.modificationType; const change: AttributionChange = preview ? { modificationType: "change" } : markChange; diff --git a/packages/core/src/y/extensions/YAttributionMarks.ts b/packages/core/src/y/extensions/YAttributionMarks.ts index 2b88529f6d..d2bc75e4c7 100644 --- a/packages/core/src/y/extensions/YAttributionMarks.ts +++ b/packages/core/src/y/extensions/YAttributionMarks.ts @@ -140,6 +140,9 @@ const createAttributionMarkView = userIds: JSON.stringify(getAttributionUserIds(mark)), inline: String(inline), }); + if (mark.attrs["moved"]) { + dom.dataset["moved"] = ""; + } if (type === "attrs") { dom.dataset["type"] = "attributes"; dom.dataset["attributes"] = JSON.stringify(changes); @@ -255,6 +258,8 @@ export const YAttributedInsertion = Mark.create<{ addAttributes() { return { userIds: { default: null }, + // A moved block, at its new place (see `showSnapshotPreview`). + moved: { default: null }, }; }, addMarkView() { @@ -282,6 +287,8 @@ export const YAttributedDeletion = Mark.create<{ addAttributes() { return { userIds: { default: null }, + // The original of a moved block (see `showSnapshotPreview`). + moved: { default: null }, }; }, addMarkView() { diff --git a/packages/core/src/y/extensions/YSync.ts b/packages/core/src/y/extensions/YSync.ts index 9e2f13d06b..3fac5bb557 100644 --- a/packages/core/src/y/extensions/YSync.ts +++ b/packages/core/src/y/extensions/YSync.ts @@ -68,16 +68,25 @@ export const mapAttributionToMark = ( insertAt?: number; deleteAt?: number; formatAt?: number; + /** Set on a moved block and its original, see `showSnapshotPreview`. */ + moved?: boolean; }, ): Record => { const out: Record = { ...format }; if (attribution.insert) { - out["y-attributed-insert"] = { userIds: attribution.insert }; + // Every mark attribute is listed, so the mark equals this value. + out["y-attributed-insert"] = { + userIds: attribution.insert, + moved: attribution.moved === true ? true : null, + }; } if (attribution.delete) { - out["y-attributed-delete"] = { userIds: attribution.delete }; + out["y-attributed-delete"] = { + userIds: attribution.delete, + moved: attribution.moved === true ? true : null, + }; } if (attribution.format) { diff --git a/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap b/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap new file mode 100644 index 0000000000..3e08b40a3f --- /dev/null +++ b/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap @@ -0,0 +1,84 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > indent 1`] = ` +[ + "insert block X: bob", + "insert X: bob", + "delete block X: bob", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > move into a concurrently deleted block 1`] = ` +[ + "delete block Parent: alice", + "delete block X: ", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > text edit 1`] = ` +[ + "insert !: bob", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > type change 1`] = ` +[ + "delete block X: bob", + "insert block X: bob", + "insert X: bob", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRecreatedBlocks' } > indent 1`] = ` +[ + "moved block X: bob", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRecreatedBlocks' } > move into a concurrently deleted block 1`] = ` +[ + "delete block Parent: alice", + "delete block X: ", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRecreatedBlocks' } > text edit 1`] = ` +[ + "insert !: bob", +] +`; + +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRecreatedBlocks' } > type change 1`] = ` +[ + "formatting X", +] +`; + +exports[`experimental diffs {} > indent 1`] = ` +[ + "insert block X: bob", + "insert X: bob", + "delete block X: bob", +] +`; + +exports[`experimental diffs {} > move into a concurrently deleted block 1`] = ` +[ + "delete block Parent: alice", + "delete block X: bob", +] +`; + +exports[`experimental diffs {} > text edit 1`] = ` +[ + "insert !: bob", +] +`; + +exports[`experimental diffs {} > type change 1`] = ` +[ + "delete block X: bob", + "insert block X: bob", + "insert X: bob", +] +`; diff --git a/packages/core/src/y/extensions/nestingChanges.test.ts b/packages/core/src/y/extensions/nestingChanges.test.ts index 220e3ab27a..d0b779be01 100644 --- a/packages/core/src/y/extensions/nestingChanges.test.ts +++ b/packages/core/src/y/extensions/nestingChanges.test.ts @@ -24,6 +24,9 @@ function collaborativeEditor(doc: Y.Doc): Editor { collaboration: { fragment: doc.get("doc"), user: { name: "Test", color: "#ff0000" }, + experimental: { + versionDiffFixes: "implicitDeleteAttributionAndRecreatedBlocks", + }, }, }), ); @@ -220,8 +223,7 @@ describe("version diff of a nesting change", () => { return changed; } - // To be fixed by #3172. - it.fails("shows an indent as a moved block, leaving the new parent unchanged", () => { + it("shows an indent as a moved block, leaving the new parent unchanged", () => { expect( diffOf( (editor) => { @@ -236,8 +238,7 @@ describe("version diff of a nesting change", () => { ).toEqual([">X"]); }); - // To be fixed by #3172. - it.fails("shows an unindent as a moved block, leaving the old parent unchanged", () => { + it("shows an unindent as a moved block, leaving the old parent unchanged", () => { expect( diffOf( (editor) => { diff --git a/packages/core/src/y/extensions/snapshotPreview.ts b/packages/core/src/y/extensions/snapshotPreview.ts index 30d4afe6e4..7187d49b27 100644 --- a/packages/core/src/y/extensions/snapshotPreview.ts +++ b/packages/core/src/y/extensions/snapshotPreview.ts @@ -1,5 +1,6 @@ import { configureYProsemirror } from "@y/prosemirror"; import * as Y from "@y/y"; +import { diff } from "lib0/diff/patience"; import type { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; import { findTypeInOtherYdoc } from "../utils.js"; @@ -16,14 +17,23 @@ export type VersionDiffFix = * block, or with a moved block's lost copy) isn't credited to the user whose * change removed it. */ - "implicitDeleteAttribution"; + | "implicitDeleteAttribution" + /** + * A block that a type change or a move re-created shows once: a type change + * between text blocks as a formatting change, a move (including indenting) + * as a move, and a copy in the same place (its children changed) as + * unchanged. Its content keeps its authors. + */ + | "recreatedBlocks"; /** * Experimental fixes of how a diff between two versions is shown, each * including the ones before it. They only change what a diff shows, never * what is stored, so they can be turned on or off at any time. */ -export type VersionDiffFixes = "implicitDeleteAttribution"; +export type VersionDiffFixes = + | "implicitDeleteAttribution" + | "implicitDeleteAttributionAndRecreatedBlocks"; /** The fixes each option turns on. */ export const versionDiffFixesIncluded: Record< @@ -31,6 +41,10 @@ export const versionDiffFixesIncluded: Record< VersionDiffFix[] > = { implicitDeleteAttribution: ["implicitDeleteAttribution"], + implicitDeleteAttributionAndRecreatedBlocks: [ + "implicitDeleteAttribution", + "recreatedBlocks", + ], }; export type ExperimentalVersionDiffs = { versionDiffFixes?: VersionDiffFixes }; @@ -90,6 +104,74 @@ class SnapshotDiffRenderer extends Y.DiffRenderer { const [own] = this.deletes.slice(parent.id.client, parent.id.clock, 1); return sameAttributes(own?.attrs ?? null, attrs); } + + /** + * Show `unchanged` content as unchanged (or not at all, if deleted), credit + * each copied item to the authors that inserted its original, and mark the + * `moved` blocks' insertion as a move. + */ + adjust( + unchanged: Y.IdSet, + credits: Array<[original: Y.ID, copy: Y.ID]>, + moved: Y.ID[], + movedFrom: Y.Item[], + ) { + this.inserts = Y.diffIdMap(this.inserts, unchanged); + this.deletes = Y.diffIdMap(this.deletes, unchanged); + this.attributed = Y.diffIdSet(this.attributed, unchanged); + const credited = Y.createIdSet(); + const credit: Y.IdMap = Y.createIdMap(); + for (const [original, copy] of credits) { + const attrs = this.inserts.slice(original.client, original.clock, 1)[0] + ?.attrs; + const ids = Y.createIdSet(); + ids.add(copy.client, copy.clock, 1); + credited.add(copy.client, copy.clock, 1); + Y.insertIntoIdMap(credit, Y.createIdMapFromIdSet(ids, attrs ?? [])); + } + for (const id of moved) { + const attrs = this.inserts.slice(id.client, id.clock, 1)[0]?.attrs ?? []; + const ids = Y.createIdSet(); + ids.add(id.client, id.clock, 1); + credited.add(id.client, id.clock, 1); + Y.insertIntoIdMap( + credit, + Y.createIdMapFromIdSet(ids, [ + ...attrs, + Y.createContentAttribute("moved", true), + ]), + ); + } + this.inserts = Y.mergeIdMaps([Y.diffIdMap(this.inserts, credited), credit]); + // The struck-through originals of moved blocks: their deletion is a move. + const relabeled = Y.createIdSet(); + const relabel: Y.IdMap = Y.createIdMap(); + for (const item of movedFrom) { + const { client } = item.id; + for (const range of this.deletes.slice( + client, + item.id.clock, + item.length, + )) { + if (range.attrs) { + const ids = Y.createIdSet(); + ids.add(client, range.clock, range.len); + relabeled.add(client, range.clock, range.len); + Y.insertIntoIdMap( + relabel, + Y.createIdMapFromIdSet(ids, [ + ...range.attrs, + Y.createContentAttribute("moved", true), + ]), + ); + } + } + } + this.deletes = Y.mergeIdMaps([ + Y.diffIdMap(this.deletes, relabeled), + relabel, + ]); + } } /** Whether two attributions show the same, ignoring timestamps (`deleteAt`, ...). */ @@ -223,6 +305,216 @@ function blockId(block: Y.Node): unknown { return block._map.get("id")?.content.getContent().at(-1); } +/** + * The items of a block's content, one unit per character, mark or node. Text + * the old binding wrapped in anonymous nodes is included. + */ +function units( + node: Y.Node, + out: Array<{ id: Y.ID; key: string }> = [], +): Array<{ id: Y.ID; key: string }> { + for (let item = node._start; item !== null; item = item.right) { + const content = item.content; + if (content instanceof Y.ContentType && content.type.name == null) { + out.push({ id: item.id, key: "n" }); + units(content.type, out); + continue; + } + for (let i = 0; i < item.length; i++) { + out.push({ + id: Y.createID(item.id.client, item.id.clock + i), + key: + content instanceof Y.ContentString + ? `s${content.str[i]}` + : content instanceof Y.ContentFormat + ? `f${content.key}=${JSON.stringify(content.value)}` + : content instanceof Y.ContentType + ? `n${content.type.name}` + : `e${JSON.stringify(content.getContent()[i] ?? null)}`, + }); + } + } + return out; +} + +/** Pairs of equal units, in order. */ +function matchUnits( + a: Array<{ id: Y.ID; key: string }>, + b: Array<{ id: Y.ID; key: string }>, + pairs: Array<[Y.ID, Y.ID]>, +) { + let i = 0; + let j = 0; + const pairUntil = (end: number) => { + for (; i < end; i++, j++) { + pairs.push([a[i].id, b[j].id]); + } + }; + for (const change of diff( + a.map((unit) => unit.key), + b.map((unit) => unit.key), + )) { + pairUntil(change.index); + i += change.remove.length; + j += change.insert.length; + } + pairUntil(a.length); +} + +/** The attribute items of `a` and `b` that hold equal values. */ +function matchAttributes(a: Y.Node, b: Y.Node, pairs: Array<[Y.ID, Y.ID]>) { + for (const [key, copy] of b._map) { + const original = a._map.get(key); + if ( + original && + JSON.stringify(original.content.getContent()) === + JSON.stringify(copy.content.getContent()) + ) { + pairs.push([original.id, copy.id]); + } + } +} + +/** A node's child nodes, deleted ones included. */ +function childNodes(node: Y.Node): Y.Node[] { + const out: Y.Node[] = []; + for (let item = node._start; item !== null; item = item.right) { + if (item.content instanceof Y.ContentType) { + out.push(item.content.type); + } + } + return out; +} + +/** + * A block's content node (paragraph, heading, ...). A block from the old + * binding can hold several after a type change: prefer the one in `baseline`. + */ +function contentOf(block: Y.Node, baseline: Y.Doc): Y.Node | undefined { + const contents = childNodes(block).filter((n) => n.name !== "blockGroup"); + return ( + contents.find((n) => inBaseline(baseline, n._item!.id)) ?? + contents.find((n) => !n._item!.deleted) ?? + contents[0] + ); +} + +/** + * Pair a block's copy with its original: the blocks, their content, and their + * children (by id). A reformatted block's own content attributes are left + * unpaired, so they show as the change. + */ +function matchCopy( + original: Y.Node, + copy: Y.Node, + baseline: Y.Doc, + pairs: Array<[Y.ID, Y.ID]>, + reformatted: boolean, +) { + pairs.push([original._item!.id, copy._item!.id]); + matchAttributes(original, copy, pairs); + const originalContent = contentOf(original, baseline); + const copyContent = contentOf(copy, baseline); + if (originalContent && copyContent) { + pairs.push([originalContent._item!.id, copyContent._item!.id]); + if (!reformatted) { + matchAttributes(originalContent, copyContent, pairs); + } + matchUnits(units(originalContent), units(copyContent), pairs); + } + const originalGroup = childNodes(original).find( + (n) => n.name === "blockGroup", + ); + const copyGroup = childNodes(copy).find((n) => n.name === "blockGroup"); + if (originalGroup && copyGroup) { + pairs.push([originalGroup._item!.id, copyGroup._item!.id]); + const originals = new Map( + childNodes(originalGroup).map((child) => [blockId(child), child]), + ); + for (const child of childNodes(copyGroup)) { + const match = originals.get(blockId(child)); + if (match) { + matchCopy(match, child, baseline, pairs, false); + } + } + } +} + +/** + * Blocks that were copied since `baseline`, with the original they copy. + * + * Changing a block's type or moving it (including indenting it) deletes the + * block and inserts a copy with the same id: a Yjs node can't change its name + * or position. The copy's content is then credited to whoever made the change, + * and the diff shows the block twice. The pairs let the diff show the block + * once, as a formatting change or a move, and credit copied content to its + * authors. + */ +function copiedBlocks( + doc: Y.Doc, + baseline: Y.Doc, + holdsText: (type: string | undefined) => boolean, +): Array<{ original: Y.Node; copy: Y.Node; typeChanged: boolean }> { + const { inserted, deleted } = changesSince(doc, baseline); + const originals = new Map(); + for (const item of itemsIn(doc, deleted)) { + if (isBlock(item)) { + originals.set(blockId(item.content.type), item.content.type); + } + } + const copies = itemsIn(doc, inserted).flatMap((item) => + isBlock(item) && !item.deleted ? [item.content.type] : [], + ); + return copies.flatMap((copy) => { + const original = originals.get(blockId(copy)); + // Concurrent changes can leave several copies: showing each as the + // original would hide that the block is now there more than once. + if ( + !original || + copies.filter((other) => blockId(other) === blockId(copy)).length > 1 + ) { + return []; + } + const from = contentOf(original, baseline)?.name; + const to = contentOf(copy, baseline)?.name; + // Only text blocks change type in place: an image turned into a paragraph + // has lost the image. + if (from !== to && !(holdsText(from) && holdsText(to))) { + return []; + } + return [{ original, copy, typeChanged: from !== to }]; + }); +} + +/** Whether some unit of `item` is in `lost` but not in `kept`. */ +function losesUnit(item: Y.Item, lost: Y.IdSet, kept: Y.IdSet): boolean { + for (let i = 0; i < item.length; i++) { + const clock = item.id.clock + i; + if (lost.has(item.id.client, clock) && !kept.has(item.id.client, clock)) { + return true; + } + } + return false; +} + +/** + * Whether `copy` replaced `original` where it stood: in the same list, with + * only deleted items between them. Changing a block's children does that. + */ +function inPlace(original: Y.Node, copy: Y.Node): boolean { + for (const side of ["left", "right"] as const) { + for (let item = copy._item?.[side]; item; item = item[side]) { + if (item === original._item) { + return true; + } + if (!item.deleted) { + break; + } + } + } + return false; +} + /** Whether the item with this id was already in `baseline`. */ function inBaseline(baseline: Y.Doc, id: Y.ID): boolean { const last = baseline.store.clients.get(id.client)?.at(-1); @@ -342,6 +634,155 @@ function splitChangedBlocks( } } +/** + * Show each copied block (see {@link copiedBlocks}) once: when the earlier + * version has the original, render the copy as the original plus the change + * (formatting change or move); otherwise credit the copied content to the + * original's authors. + */ +function showCopiesOnce( + editor: BlockNoteEditor, + snapshot: { doc: Y.Doc; fragment: Y.Node }, + earlier: { doc: Y.Doc; fragment: Y.Node }, + renderer: SnapshotDiffRenderer, +) { + const doc = snapshot.doc; + const baseline = earlier.doc; + // Deleted content that was in the earlier version. + const { inserted, deleted } = changesSince(doc, baseline); + const removed = Y.diffIdSet(deleted, inserted); + const removedContent = itemsIn(doc, removed).filter( + // Content only: not attributes, nor structure (child groups, whose + // children are checked themselves, and the old binding's anonymous text + // wrappers). + (item) => + item.parentSub === null && + !( + item.content instanceof Y.ContentType && + (item.content.type.name == null || + item.content.type.name === "blockGroup") + ), + ); + const matches = copiedBlocks( + doc, + baseline, + (type) => + type !== undefined && + editor.schema.blockSpecs[type]?.config.content === "inline", + ).map(({ original, copy, typeChanged }) => { + const pairs: Array<[Y.ID, Y.ID]> = []; + matchCopy(original, copy, baseline, pairs, typeChanged); + return { original, copy, typeChanged, pairs }; + }); + + // An original in the earlier version is shown as its copy, so only the + // change shows. Unless content it had made it into no copy (it was lost + // with the original): then show both, so the loss shows too. Content that + // moved out (into another shown copy) isn't lost. + let shown = matches.filter((match) => + inBaseline(baseline, match.original._item!.id), + ); + for (let changed = true; changed;) { + const kept = Y.createIdSet(); + for (const { pairs } of shown) { + for (const [a] of pairs) { + kept.add(a.client, a.clock, 1); + } + } + const next = shown.filter( + ({ original }) => + !removedContent.some( + (item) => + (item === original._item || Y.isParentOf(original, item)) && + // Per unit: a deleted item can span content deleted before. + losesUnit(item, removed, kept), + ), + ); + changed = next.length !== shown.length; + shown = next; + } + + // A block that moved elsewhere also shows its original, struck through, at + // the old place. One indented or outdented keeps its place in reading + // order, so it only shows at the new place. Blocks moved with a parent are + // struck through with it. + const before = readingOrder(earlier.fragment); + const after = readingOrder(snapshot.fragment); + function isMove({ original, copy, typeChanged }: (typeof matches)[number]) { + return !typeChanged && !inPlace(original, copy); + } + const reorderedOriginals = shown + .filter( + (match) => isMove(match) && reordered(blockId(match.copy), before, after), + ) + .map(({ original }) => original); + function struck(original: Y.Node) { + return reorderedOriginals.some( + (other) => other === original || Y.isParentOf(other, original._item!), + ); + } + + const unchanged = Y.createIdSet(); + const credits: Array<[Y.ID, Y.ID]> = []; + const moved: Y.ID[] = []; + for (const match of matches) { + const { original, copy, pairs } = match; + if (!inBaseline(baseline, original._item!.id)) { + credits.push(...pairs); + continue; + } + if (!shown.includes(match)) { + continue; + } + // A block that moved stays marked, as a move. One copied in place (its + // children changed) shows unchanged. + const move = isMove(match); + for (const [a, b] of pairs) { + if (!(move && struck(original))) { + unchanged.add(a.client, a.clock, 1); + } + if (!move || b !== copy._item!.id) { + unchanged.add(b.client, b.clock, 1); + } + } + if (move) { + moved.push(copy._item!.id); + } + } + const movedFrom = itemsIn(doc, removed).filter((item) => + reorderedOriginals.some( + (original) => item === original._item || Y.isParentOf(original, item), + ), + ); + renderer.adjust(unchanged, credits, moved, movedFrom); +} + +/** The ids of the blocks under `node`, top to bottom, nesting ignored. */ +function readingOrder(node: Y.Node, out: unknown[] = []): unknown[] { + for (let item = node._start; item !== null; item = item.right) { + if (!item.deleted && item.content instanceof Y.ContentType) { + if (isBlock(item)) { + out.push(blockId(item.content.type)); + } + readingOrder(item.content.type, out); + } + } + return out; +} + +/** + * Whether the block with this id has another block above it than before, + * counting only blocks in both versions. An indent or outdent keeps it. + */ +function reordered(id: unknown, before: unknown[], after: unknown[]): boolean { + function previous(order: unknown[], other: unknown[]) { + const inOther = new Set(other); + const common = order.filter((block) => inOther.has(block)); + return common[common.indexOf(id) - 1]; + } + return previous(before, after) !== previous(after, before); +} + /** * Decode a snapshot, diff it against a baseline if given, and render it. * @@ -402,15 +843,22 @@ export function showSnapshotPreview( const options = renderAttributions ? { attributions: renderAttributions } : undefined; + let renderer: Y.DiffRenderer | undefined; + if (baseline && keepDeleted) { + const snapshotRenderer = new SnapshotDiffRenderer( + baseline.doc, + snapshot.doc, + options, + ); + if (hasFix(experimental, "recreatedBlocks")) { + showCopiesOnce(editor, snapshot, baseline, snapshotRenderer); + } + renderer = snapshotRenderer; + } else if (baseline) { + renderer = Y.createDiffRenderer(baseline.doc, snapshot.doc, options); + } editor.exec( - configureYProsemirror({ - ytype: snapshot.fragment, - renderer: baseline - ? keepDeleted - ? new SnapshotDiffRenderer(baseline.doc, snapshot.doc, options) - : Y.createDiffRenderer(baseline.doc, snapshot.doc, options) - : undefined, - }), + configureYProsemirror({ ytype: snapshot.fragment, renderer }), ); } finally { destroyDecodedFragment(snapshot); diff --git a/packages/core/src/y/extensions/versionDiffAttribution.test.ts b/packages/core/src/y/extensions/versionDiffAttribution.test.ts index 0e3462a123..41053c9de5 100644 --- a/packages/core/src/y/extensions/versionDiffAttribution.test.ts +++ b/packages/core/src/y/extensions/versionDiffAttribution.test.ts @@ -23,7 +23,9 @@ function collaborativeEditor(doc: Y.Doc) { collaboration: { fragment: doc.get("doc"), user: { name: "Test", color: "#ff0000" }, - experimental: { versionDiffFixes: "implicitDeleteAttribution" }, + experimental: { + versionDiffFixes: "implicitDeleteAttributionAndRecreatedBlocks", + }, }, }), ); @@ -301,15 +303,6 @@ describe("version diff of a moved block", () => { .map((change) => `${change.text}: ${change.users.join(", ")}`); } - it("attributes a move to the mover", () => { - const base = blocks(); - const server = history(base); - const after = server.apply(editOf(base, 2, nest), "bob"); - expect( - deletions(Y.encodeStateAsUpdateV2(base), after, server.attributions), - ).toEqual(["[block moved]: bob"]); - }); - it("names no author for a block moved into a concurrently deleted one", () => { const base = blocks(); const bob = editOf(base, 2, nest); @@ -433,8 +426,7 @@ describe("version diff of a type change", () => { return out; } - // To be fixed by #3172. - it.fails("shows a type change as a formatting change, not as replaced text", () => { + it("shows a type change as a formatting change, not as replaced text", () => { const base = blocks(); const server = history(base); const after = server.apply(editOf(base, 2, toHeading), "bob"); @@ -443,8 +435,7 @@ describe("version diff of a type change", () => { ).toEqual(["attrs : bob"]); }); - // To be fixed by #3172. - it.fails("credits a type-changed block's text to its writer, from before it existed", () => { + it("credits a type-changed block's text to its writer, from before it existed", () => { const base = baseDocument([ { id: "next", type: "paragraph", content: "Next" }, ]); @@ -469,8 +460,7 @@ describe("version diff of a type change", () => { ]); }); - // To be fixed by #3172. - it.fails("keeps later edits to a type-changed block as their author's", () => { + it("keeps later edits to a type-changed block as their author's", () => { const base = blocks(); const server = history(base); server.apply(editOf(base, 2, toHeading), "bob"); @@ -514,8 +504,7 @@ describe("version diff of a type change", () => { ]); }); - // To be fixed by #3172. - it.fails("credits an indented block's text to its writer, from before it existed", () => { + it("credits an indented block's text to its writer, from before it existed", () => { const base = blocks(); const server = history(base); server.apply( @@ -546,8 +535,7 @@ describe("version diff of a type change", () => { ]); }); - // To be fixed by #3172. - it.fails("shows a block moved among its siblings as a move at both places", () => { + it("shows a block moved among its siblings as a move at both places", () => { const base = baseDocument([ { id: "first", type: "paragraph", content: "First" }, { id: "second", type: "paragraph", content: "Second" }, @@ -569,8 +557,7 @@ describe("version diff of a type change", () => { ]); }); - // To be fixed by #3172. - it.fails("shows a type change as a formatting change after the text was rewritten", () => { + it("shows a type change as a formatting change after the text was rewritten", () => { const base = blocks(); const server = history(base); // The rewrite reuses some characters, so the block's stored text mixes diff --git a/packages/core/src/y/extensions/versionDiffFlags.test.ts b/packages/core/src/y/extensions/versionDiffFlags.test.ts new file mode 100644 index 0000000000..04ee9189e8 --- /dev/null +++ b/packages/core/src/y/extensions/versionDiffFlags.test.ts @@ -0,0 +1,201 @@ +/** + * @vitest-environment jsdom + */ +import * as Y from "@y/y"; +import { afterEach, describe, expect, it } from "vite-plus/test"; + +import { BlockNoteEditor } from "../../editor/BlockNoteEditor.js"; +import { blocksToYType } from "../utils.js"; +import { withCollaboration } from "./index.js"; +import type { ExperimentalVersionDiffs } from "./snapshotPreview.js"; +import { createYVersionView } from "./Versioning.js"; + +/** + * The experimental version-diff options only change how a diff is shown, so + * every combination must work, and none changes what an edit stores. + */ + +const editors: BlockNoteEditor[] = []; +afterEach(() => { + for (const editor of editors.splice(0)) { + editor.unmount(); + } +}); + +function editorOn(doc: Y.Doc, experimental: ExperimentalVersionDiffs) { + const editor = BlockNoteEditor.create( + withCollaboration({ + collaboration: { + fragment: doc.get("doc"), + user: { name: "Test", color: "#ff0000" }, + experimental, + }, + }), + ); + editor.mount(document.body.appendChild(document.createElement("div"))); + editors.push(editor); + return editor; +} + +const base = () => { + const doc = new Y.Doc({ gc: false }); + doc.clientID = 100; + const seed = BlockNoteEditor.create(); + seed.replaceBlocks(seed.document, [ + { + id: "parent", + type: "paragraph", + content: "Parent", + children: [{ id: "child", type: "paragraph", content: "Child" }], + }, + { id: "x", type: "paragraph", content: "X" }, + { id: "next", type: "paragraph", content: "Next" }, + ]); + blocksToYType(seed, seed.document, doc.get("doc")); + return doc; +}; + +/** One user's edit of `from`, as the updates their client sends. */ +function edit( + from: Y.Doc, + client: number, + experimental: ExperimentalVersionDiffs, + change: (editor: BlockNoteEditor) => void, +) { + const doc = new Y.Doc({ gc: false }); + doc.clientID = client; + Y.applyUpdateV2(doc, Y.encodeStateAsUpdateV2(from)); + const editor = editorOn(doc, experimental); + const updates: Uint8Array[] = []; + doc.on("updateV2", (update: Uint8Array) => updates.push(update)); + change(editor); + return Y.mergeUpdatesV2(updates); +} + +/** Changes shown in the diff, as `kind text: users`. */ +function diff( + experimental: ExperimentalVersionDiffs, + edits: Array<[client: number, user: string, change: (e: any) => void]>, +) { + const start = base(); + const server = new Y.Doc({ gc: false }); + Y.applyUpdateV2(server, Y.encodeStateAsUpdateV2(start)); + const attributions = Y.createContentMap(); + const updates = edits.map(([client, user, change]) => { + const update = edit(start, client, experimental, change); + return { update, user }; + }); + for (const { update, user } of updates) { + const before = Y.createInsertSetFromStructStore(server.store, false); + Y.applyUpdateV2(server, update); + Y.insertIntoIdMap( + attributions.inserts, + Y.createIdMapFromIdSet( + Y.diffIdSet( + Y.createInsertSetFromStructStore(server.store, false), + before, + ), + [Y.createContentAttribute("insert", user)], + ), + ); + Y.insertIntoIdMap( + attributions.deletes, + Y.createIdMapFromIdSet(Y.decodeUpdateV2(update).ds, [ + Y.createContentAttribute("delete", user), + ]), + ); + } + const after = Y.encodeStateAsUpdateV2(server); + const viewDoc = new Y.Doc(); + Y.applyUpdateV2(viewDoc, after); + const editor = editorOn(viewDoc, experimental); + const view = createYVersionView(editor, viewDoc.get("doc")).open(); + view.show({ + content: after, + comparison: { content: Y.encodeStateAsUpdateV2(start), attributions }, + target: { type: "snapshot", id: "after" }, + }); + const out: string[] = []; + editor.prosemirrorState.doc.descendants((node) => { + for (const mark of node.marks) { + if (!node.isText || !mark.type.name.startsWith("y-attributed-")) { + continue; + } + const kind = mark.type.name.slice(13); + out.push( + `${kind} ${node.text}: ${(mark.attrs["userIds"] ?? []).join(",")}`, + ); + } + // Block-level: inserted, deleted and moved blocks, and formatting changes. + if (node.type.name === "blockContainer") { + for (const mark of node.marks) { + const kind = mark.attrs["moved"] ? "moved" : mark.type.name.slice(13); + if (["insert", "delete", "moved"].includes(kind)) { + out.push( + `${kind} block ${node.firstChild!.textContent}: ${(mark.attrs["userIds"] ?? []).join(",")}`, + ); + } + } + } + if ( + node.isTextblock && + node.marks.some((mark) => mark.type.name === "y-attributed-attrs") && + !node.marks.some((mark) => mark.type.name !== "y-attributed-attrs") + ) { + out.push(`formatting ${node.textContent}`); + } + return true; + }); + view.close(); + return out; +} + +const nest = (id: string) => (editor: any) => { + editor.setTextCursorPosition(id); + editor.nestBlock(); +}; +const scenarios: Record void]>> = { + "move into a concurrently deleted block": [ + [1, "alice", (editor) => editor.removeBlocks(["parent"])], + [2, "bob", nest("x")], + ], + "type change": [ + [2, "bob", (editor) => editor.updateBlock("x", { type: "heading" })], + ], + indent: [[2, "bob", nest("x")]], + "text edit": [ + [ + 2, + "bob", + (editor) => { + editor.setTextCursorPosition("x", "end"); + editor.insertInlineContent("!"); + }, + ], + ], +}; + +const combinations: ExperimentalVersionDiffs[] = [ + {}, + { versionDiffFixes: "implicitDeleteAttribution" }, + { versionDiffFixes: "implicitDeleteAttributionAndRecreatedBlocks" }, +]; + +describe.each(combinations)("experimental diffs %o", (experimental) => { + it.each(Object.entries(scenarios))("%s", (_, edits) => { + expect(diff(experimental, edits)).toMatchSnapshot(); + }); +}); + +it("stores the same edits with any combination", () => { + for (const edits of Object.values(scenarios)) { + const stored = combinations.map((experimental) => + edits.map(([client, , change]) => + Array.from(edit(base(), client, experimental, change)), + ), + ); + for (const other of stored.slice(1)) { + expect(other).toEqual(stored[0]); + } + } +}); diff --git a/packages/core/src/y/utils.test.ts b/packages/core/src/y/utils.test.ts index 2174c6ae53..90cf35d4f3 100644 --- a/packages/core/src/y/utils.test.ts +++ b/packages/core/src/y/utils.test.ts @@ -1289,12 +1289,19 @@ describe("yNodeToTransaction", () => { deleted.marks .filter((mark) => mark.type.name === "y-attributed-delete") .map((mark) => mark.toJSON()), - ).toEqual([{ type: "y-attributed-delete", attrs: { userIds: ["bob"] } }]); + ).toEqual([ + { type: "y-attributed-delete", attrs: { userIds: ["bob"], moved: null } }, + ]); expect( inserted.marks .filter((mark) => mark.type.name === "y-attributed-insert") .map((mark) => mark.toJSON()), - ).toEqual([{ type: "y-attributed-insert", attrs: { userIds: ["bob"] } }]); + ).toEqual([ + { + type: "y-attributed-insert", + attrs: { userIds: ["bob"], moved: null }, + }, + ]); const heading = inserted.firstChild!; expect(heading.type.name).toBe("heading"); expect(heading.attrs.level).toBe(2); diff --git a/packages/react/src/components/AttributionTooltip/AttributionTooltip.tsx b/packages/react/src/components/AttributionTooltip/AttributionTooltip.tsx index 15866aeae3..c89a0993bc 100644 --- a/packages/react/src/components/AttributionTooltip/AttributionTooltip.tsx +++ b/packages/react/src/components/AttributionTooltip/AttributionTooltip.tsx @@ -37,6 +37,9 @@ export const AttributionTooltip = (props: AttributionTooltipProps) => { if (props.modificationType === "delete") { return users ? changes.deleted_by(users) : changes.deleted; } + if (props.modificationType === "move") { + return users ? changes.moved_by(users) : changes.moved; + } if (props.modificationType === "change") { return users ? changes.changed_by(users) : changes.changed; } diff --git a/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap b/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap index 7b6aa8ba50..8c599595f1 100644 --- a/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap +++ b/tests/src/end-to-end/y-prosemirror/__snapshots__/versioning.test.tsx.snap @@ -542,23 +542,16 @@ exports[`versioning diff (experimental): Both nest a new block under N0 1`] = ` exports[`versioning diff (experimental): Cascading indents 1`] = ` [ - "delete block "N0" A", "delete block "N1" A,B", - "insert block "N0" A", - "insert A", - "insert "N0" A", "insert A", "insert block "N1" A", "insert A", "insert "N1" A", - "delete block "N2" B", "insert block "N1" B", "insert B", "insert "N1" B", "insert B", - "insert block "N2" B", - "insert B", - "insert "N2" B", + "moved block "N2" B", ] `; @@ -570,14 +563,7 @@ exports[`versioning diff (experimental): Center-align 1`] = ` exports[`versioning diff (experimental): Change a parent's type vs edit its child 1`] = ` [ - "delete block "Parent" A", - "insert block "Parent" A", - "insert A", - "insert "Parent" A", - "insert A", - "insert block "Child" A", - "insert A", - "insert "Child" A", + "attrs backgroundColor:A textColor:A textAlignment:A level:A isToggleable:A", ] `; @@ -589,14 +575,7 @@ exports[`versioning diff (experimental): Change image source 1`] = ` exports[`versioning diff (experimental): Change type of a parent block 1`] = ` [ - "delete block "N0" A", - "insert block "N0" A", - "insert A", - "insert "N0" A", - "insert A", - "insert block "N1" A", - "insert A", - "insert "N1" A", + "attrs backgroundColor:A textColor:A textAlignment:A level:A isToggleable:A", ] `; @@ -737,12 +716,7 @@ exports[`versioning diff (experimental): Edit a link 1`] = ` exports[`versioning diff (experimental): Edit text vs change to heading 1`] = ` [ - "delete block "hello world" B", - "delete "wo" B,A", - "delete "ld" B,A", - "insert block "hello world" B", - "insert B", - "insert "hello world" B", + "attrs backgroundColor:B textColor:B textAlignment:B level:B isToggleable:B", ] `; @@ -786,29 +760,15 @@ exports[`versioning diff (experimental): Highlight a column 1`] = ` exports[`versioning diff (experimental): Indent a block 1`] = ` [ - "delete block "N0" A", - "delete block "N1" A", - "insert block "N0" A", - "insert A", - "insert "N0" A", "insert A", - "insert block "N1" A", - "insert A", - "insert "N1" A", + "moved block "N1" A", ] `; exports[`versioning diff (experimental): Indent a block vs edit its text 1`] = ` [ - "delete block "N0" A", - "delete block "N1" A", - "insert block "N0" A", - "insert A", - "insert "N0" A", "insert A", - "insert block "N1" A", - "insert A", - "insert "N1" A", + "moved block "N1" A", ] `; @@ -829,10 +789,7 @@ exports[`versioning diff (experimental): Insert an image 1`] = ` exports[`versioning diff (experimental): List item → paragraph 1`] = ` [ - "delete block "hello world" A", - "insert block "hello world" A", - "insert A", - "insert "hello world" A", + "attrs backgroundColor:A textColor:A textAlignment:A", ] `; @@ -862,85 +819,52 @@ exports[`versioning diff (experimental): Move a block into a block that is delet exports[`versioning diff (experimental): Move paragraph up 1`] = ` [ - "insert block "Middle" A", - "insert A", - "insert "Middle" A", - "delete block "Middle" A", + "moved block "Middle" A", + "moved from block "Middle" A", ] `; exports[`versioning diff (experimental): Move paragraph with children 1`] = ` [ - "insert block "Parent" A", - "insert A", - "insert "Parent" A", - "insert A", - "insert block "Child" A", - "insert A", - "insert "Child" A", - "delete block "Parent" A", + "moved block "Parent" A", + "moved from block "Parent" A", ] `; exports[`versioning diff (experimental): Nest a bullet under another 1`] = ` [ - "delete block "Parent" A", - "delete block "Child" A", - "insert block "Parent" A", - "insert A", - "insert "Parent" A", "insert A", - "insert block "Child" A", - "insert A", - "insert "Child" A", + "moved block "Child" A", ] `; exports[`versioning diff (experimental): Nest blocks into a block that is moved 1`] = ` [ - "delete block "R" B", "delete block "Q" B,A", - "insert block "R" B", - "insert B", - "insert "R" B", "insert B", "insert block "Q" B", "insert B", "insert "Q" B", - "delete block "B1" A", - "delete block "B2" A", - "delete block "B3" A", "insert block "Q" A", "insert A", "insert "Q" A", "insert A", - "insert block "B1" A", - "insert A", - "insert "B1" A", - "insert block "B2" A", - "insert A", - "insert "B2" A", - "insert block "B3" A", - "insert A", - "insert "B3" A", + "moved block "B1" A", + "moved block "B2" A", + "moved block "B3" A", ] `; exports[`versioning diff (experimental): Paragraph → heading 1`] = ` [ - "delete block "hello world" A", - "insert block "hello world" A", - "insert A", - "insert "hello world" A", + "attrs backgroundColor:A textColor:A textAlignment:A level:A isToggleable:A", ] `; exports[`versioning diff (experimental): Remove a column 1`] = ` [ "delete A", - "insert block "Left column" A", - "insert A", - "insert "Left column" A", + "moved block "Left column" A", ] `; @@ -1031,13 +955,7 @@ exports[`versioning diff (experimental): Text color vs background color 1`] = ` exports[`versioning diff (experimental): Unindent a block 1`] = ` [ - "delete block "N0" A", - "insert block "N0" A", - "insert A", - "insert "N0" A", - "insert block "N1" A", - "insert A", - "insert "N1" A", + "moved block "N1" A", ] `; diff --git a/tests/src/end-to-end/y-prosemirror/versioning.test.tsx b/tests/src/end-to-end/y-prosemirror/versioning.test.tsx index 0b5a420729..76efa4c65d 100644 --- a/tests/src/end-to-end/y-prosemirror/versioning.test.tsx +++ b/tests/src/end-to-end/y-prosemirror/versioning.test.tsx @@ -101,7 +101,7 @@ const propertyChanges = new Map([ // Each scenario's diff with the experimental flags off (as in the editor) and // all on. const ALL_FIXES: ExperimentalVersionDiffs = { - versionDiffFixes: "implicitDeleteAttribution", + versionDiffFixes: "implicitDeleteAttributionAndRecreatedBlocks", }; const cases = scenarios.flatMap((scenario) => [ { scenario, name: "versioning diff", experimental: {} }, @@ -186,7 +186,11 @@ for (const { scenario, name, experimental } of cases) { ? `block ${JSON.stringify(node.firstChild?.textContent ?? "")}` : `<${node.type.name}>`; for (const mark of marks) { - const kind = mark.type.name.replace("y-attributed-", ""); + const kind = !mark.attrs["moved"] + ? mark.type.name.replace("y-attributed-", "") + : mark.type.name === "y-attributed-delete" + ? "moved from" + : "moved"; if (kind === "attrs" && replaced) { continue; } From 6953d6d505b0fe4378387c3df268698c812e5983 Mon Sep 17 00:00:00 2001 From: yousefed Date: Thu, 8 Oct 2026 23:43:39 +0200 Subject: [PATCH 2/3] fix(versioning): pair a block copied twice with the block of the earlier version A block copied twice between two versions (indented, then outdented; retyped twice) leaves an intermediate copy that is deleted too. The pairing could pick that copy as the original, and credit the final copy to whoever made the intermediate one. It now pairs with the block of the earlier version; without one, several candidates aren't paired. Also from the stack review: the version diff never writes to the document (checked for each value of the option), the moved-text highlight shares the inserted-text rules, and Remove a column has its own note. --- .../14-suggestion-gallery/src/scenarios.ts | 6 ++++- packages/core/src/editor/Block.css | 27 ++++++------------- .../versionDiffFlags.test.ts.snap | 23 ++++++++++++++++ .../core/src/y/extensions/snapshotPreview.ts | 18 +++++++++++-- .../extensions/versionDiffAttribution.test.ts | 9 +++---- .../src/y/extensions/versionDiffFlags.test.ts | 24 +++++++++++++++-- 6 files changed, 77 insertions(+), 30 deletions(-) diff --git a/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts b/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts index c6887b570f..79f54da6f5 100644 --- a/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts +++ b/examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts @@ -1793,7 +1793,11 @@ export const scenarios: SuggestionScenario[] = [ kind: "single", id: "remove-1-column", feedback: [ - moveNote, + { + severity: "info", + when: { recreatedBlocks: true }, + note: "Versioning shows Left column as moved out of the columns, and the columns as deleted.", + }, { severity: "high", when: { recreatedBlocks: false }, diff --git a/packages/core/src/editor/Block.css b/packages/core/src/editor/Block.css index f5b291ca8a..0a6612fcc1 100644 --- a/packages/core/src/editor/Block.css +++ b/packages/core/src/editor/Block.css @@ -1125,16 +1125,22 @@ serialized/static output, where the wrapper is a real, painted box. /* A moved block's text keeps its authors, so it has no mark of its own: its text element gets the inserted-text highlight instead. It shrinks to the text (a flex item), or breaks per line in code blocks (an inline ). */ +.bn-suggestion-mark, ins[data-moved] > .bn-suggestion-node .bn-inline-content { --bn-suggestion-token-color: currentColor; - box-decoration-break: clone; - -webkit-box-decoration-break: clone; background-color: color-mix(in srgb, var(--user-color-light) 50%, white); color: var(--user-color-dark); border-radius: 4px; } +ins[data-moved] > .bn-suggestion-node .bn-inline-content { + box-decoration-break: clone; + -webkit-box-decoration-break: clone; +} + +.dark.bn-root .bn-suggestion-mark, .dark.bn-root ins[data-moved] > .bn-suggestion-node .bn-inline-content, +.bn-block-content[data-content-type="codeBlock"] > pre .bn-suggestion-mark, ins[data-moved] > .bn-suggestion-node .bn-block-content[data-content-type="codeBlock"] @@ -1148,23 +1154,6 @@ ins[data-moved] color: white; } -.bn-suggestion-mark { - --bn-suggestion-token-color: currentColor; - background-color: color-mix(in srgb, var(--user-color-light) 50%, white); - color: var(--user-color-dark); - border-radius: 4px; -} - -.dark.bn-root .bn-suggestion-mark, -.bn-block-content[data-content-type="codeBlock"] > pre .bn-suggestion-mark { - background-color: color-mix( - in srgb, - var(--user-color-light) 25%, - var(--bn-suggestion-surface, var(--bn-colors-editor-background)) - ); - color: white; -} - /* Block-level (over a node) suggestion marks. The `.bn-suggestion-node` span is `display: contents` so it can't paint a background itself; instead the wrapped diff --git a/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap b/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap index 3e08b40a3f..ad8547f567 100644 --- a/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap +++ b/packages/core/src/y/extensions/__snapshots__/versionDiffFlags.test.ts.snap @@ -15,6 +15,14 @@ exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > ] `; +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > move up 1`] = ` +[ + "insert block Next: bob", + "insert Next: bob", + "delete block Next: bob", +] +`; + exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttribution' } > text edit 1`] = ` [ "insert !: bob", @@ -42,6 +50,13 @@ exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRec ] `; +exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRecreatedBlocks' } > move up 1`] = ` +[ + "moved block Next: bob", + "moved from block Next: bob", +] +`; + exports[`experimental diffs { versionDiffFixes: 'implicitDeleteAttributionAndRecreatedBlocks' } > text edit 1`] = ` [ "insert !: bob", @@ -69,6 +84,14 @@ exports[`experimental diffs {} > move into a concurrently deleted block 1`] = ` ] `; +exports[`experimental diffs {} > move up 1`] = ` +[ + "insert block Next: bob", + "insert Next: bob", + "delete block Next: bob", +] +`; + exports[`experimental diffs {} > text edit 1`] = ` [ "insert !: bob", diff --git a/packages/core/src/y/extensions/snapshotPreview.ts b/packages/core/src/y/extensions/snapshotPreview.ts index 7187d49b27..f020c4502d 100644 --- a/packages/core/src/y/extensions/snapshotPreview.ts +++ b/packages/core/src/y/extensions/snapshotPreview.ts @@ -456,10 +456,24 @@ function copiedBlocks( holdsText: (type: string | undefined) => boolean, ): Array<{ original: Y.Node; copy: Y.Node; typeChanged: boolean }> { const { inserted, deleted } = changesSince(doc, baseline); - const originals = new Map(); + // A block copied twice since `baseline` (e.g. indented, then outdented) + // leaves an intermediate copy, deleted too. Pair with the block that was in + // `baseline`: the diff doesn't show intermediate copies. Without one, which + // copy came first is unknown, so several copies aren't paired. + const candidates = new Map(); for (const item of itemsIn(doc, deleted)) { if (isBlock(item)) { - originals.set(blockId(item.content.type), item.content.type); + const id = blockId(item.content.type); + candidates.set(id, [...(candidates.get(id) ?? []), item.content.type]); + } + } + const originals = new Map(); + for (const [id, blocks] of candidates) { + const original = + blocks.find((block) => inBaseline(baseline, block._item!.id)) ?? + (blocks.length === 1 ? blocks[0] : undefined); + if (original) { + originals.set(id, original); } } const copies = itemsIn(doc, inserted).flatMap((item) => diff --git a/packages/core/src/y/extensions/versionDiffAttribution.test.ts b/packages/core/src/y/extensions/versionDiffAttribution.test.ts index 41053c9de5..a4b3506785 100644 --- a/packages/core/src/y/extensions/versionDiffAttribution.test.ts +++ b/packages/core/src/y/extensions/versionDiffAttribution.test.ts @@ -574,8 +574,7 @@ describe("version diff of a type change", () => { ]); }); - // To be fixed by #3172. - it.fails("strikes a moved block's children through with it at its old place", () => { + it("strikes a moved block's children through with it at its old place", () => { const base = baseDocument([ { id: "first", type: "paragraph", content: "First" }, { @@ -637,8 +636,7 @@ describe("version diff of a type change", () => { ]); }); - // To be fixed by #3172. - it.fails.each([ + it.each([ ["the same user", "bob"], ["a different user", "carol"], ])( @@ -666,8 +664,7 @@ describe("version diff of a type change", () => { }, ); - // To be fixed by #3172. - it.fails("credits two type changes to the last one", () => { + it("credits two type changes to the last one", () => { const base = blocks(); const server = history(base); server.apply(editOf(base, 2, toHeading), "bob"); diff --git a/packages/core/src/y/extensions/versionDiffFlags.test.ts b/packages/core/src/y/extensions/versionDiffFlags.test.ts index 04ee9189e8..28809f312c 100644 --- a/packages/core/src/y/extensions/versionDiffFlags.test.ts +++ b/packages/core/src/y/extensions/versionDiffFlags.test.ts @@ -109,6 +109,9 @@ function diff( const viewDoc = new Y.Doc(); Y.applyUpdateV2(viewDoc, after); const editor = editorOn(viewDoc, experimental); + const stored = Y.encodeStateAsUpdateV2(viewDoc); + let writes = 0; + viewDoc.on("update", () => writes++); const view = createYVersionView(editor, viewDoc.get("doc")).open(); view.show({ content: after, @@ -129,8 +132,12 @@ function diff( // Block-level: inserted, deleted and moved blocks, and formatting changes. if (node.type.name === "blockContainer") { for (const mark of node.marks) { - const kind = mark.attrs["moved"] ? "moved" : mark.type.name.slice(13); - if (["insert", "delete", "moved"].includes(kind)) { + const kind = !mark.attrs["moved"] + ? mark.type.name.slice(13) + : mark.type.name === "y-attributed-delete" + ? "moved from" + : "moved"; + if (["insert", "delete", "moved", "moved from"].includes(kind)) { out.push( `${kind} block ${node.firstChild!.textContent}: ${(mark.attrs["userIds"] ?? []).join(",")}`, ); @@ -147,6 +154,9 @@ function diff( return true; }); view.close(); + // Showing a diff never writes to the document. + expect(writes).toBe(0); + expect(Y.encodeStateAsUpdateV2(viewDoc)).toEqual(stored); return out; } @@ -163,6 +173,16 @@ const scenarios: Record void]>> = { [2, "bob", (editor) => editor.updateBlock("x", { type: "heading" })], ], indent: [[2, "bob", nest("x")]], + "move up": [ + [ + 2, + "bob", + (editor) => { + editor.setTextCursorPosition("next"); + editor.moveBlocksUp(); + }, + ], + ], "text edit": [ [ 2, From 7fa4b137034f3900c55c258878bb86937aa6a4c9 Mon Sep 17 00:00:00 2001 From: yousefed Date: Fri, 9 Oct 2026 09:04:34 +0200 Subject: [PATCH 3/3] fix(versioning): credit text across a block re-created several times A block re-created more than once between two versions (retyped, then indented; moved twice) is a chain of copies, each deleted by the update that inserts the next. Follow the chain back by those updates' users and times, so text typed into an intermediate copy keeps its writer. Content added to the original after the earlier version is credited to whoever added it, not shown as unchanged. Also: the pairing no longer recomputes reading order or scans all removed items for each copy. --- .../core/src/y/extensions/snapshotPreview.ts | 233 ++++++++++++++---- .../extensions/versionDiffAttribution.test.ts | 3 +- 2 files changed, 186 insertions(+), 50 deletions(-) diff --git a/packages/core/src/y/extensions/snapshotPreview.ts b/packages/core/src/y/extensions/snapshotPreview.ts index f020c4502d..9fdbe65ef6 100644 --- a/packages/core/src/y/extensions/snapshotPreview.ts +++ b/packages/core/src/y/extensions/snapshotPreview.ts @@ -453,13 +453,17 @@ function matchCopy( function copiedBlocks( doc: Y.Doc, baseline: Y.Doc, + attributions: Y.ContentMap, holdsText: (type: string | undefined) => boolean, -): Array<{ original: Y.Node; copy: Y.Node; typeChanged: boolean }> { +): Array<{ + original: Y.Node; + intermediates: Y.Node[]; + copy: Y.Node; + typeChanged: boolean; +}> { const { inserted, deleted } = changesSince(doc, baseline); - // A block copied twice since `baseline` (e.g. indented, then outdented) - // leaves an intermediate copy, deleted too. Pair with the block that was in - // `baseline`: the diff doesn't show intermediate copies. Without one, which - // copy came first is unknown, so several copies aren't paired. + // A block copied several times since `baseline` (e.g. indented, then + // outdented) leaves intermediate copies, deleted too. const candidates = new Map(); for (const item of itemsIn(doc, deleted)) { if (isBlock(item)) { @@ -467,28 +471,30 @@ function copiedBlocks( candidates.set(id, [...(candidates.get(id) ?? []), item.content.type]); } } - const originals = new Map(); - for (const [id, blocks] of candidates) { - const original = - blocks.find((block) => inBaseline(baseline, block._item!.id)) ?? - (blocks.length === 1 ? blocks[0] : undefined); - if (original) { - originals.set(id, original); - } - } const copies = itemsIn(doc, inserted).flatMap((item) => isBlock(item) && !item.deleted ? [item.content.type] : [], ); return copies.flatMap((copy) => { - const original = originals.get(blockId(copy)); + const blocks = candidates.get(blockId(copy)) ?? []; // Concurrent changes can leave several copies: showing each as the // original would hide that the block is now there more than once. - if ( - !original || - copies.filter((other) => blockId(other) === blockId(copy)).length > 1 - ) { + if (copies.filter((other) => blockId(other) === blockId(copy)).length > 1) { + return []; + } + // The copies before this one, oldest first. The block that was in + // `baseline` is the original; without one, the oldest copy is, if the + // order is known. + const chain = copyChain(copy, blocks, attributions); + const inEarlier = blocks.find((block) => + inBaseline(baseline, block._item!.id), + ); + const original = + inEarlier ?? chain[0] ?? (blocks.length === 1 ? blocks[0] : undefined); + if (!original) { return []; } + // Without a known order back to the original, intermediates aren't used. + const intermediates = chain[0] === original ? chain.slice(1) : []; const from = contentOf(original, baseline)?.name; const to = contentOf(copy, baseline)?.name; // Only text blocks change type in place: an image turned into a paragraph @@ -496,10 +502,102 @@ function copiedBlocks( if (from !== to && !(holdsText(from) && holdsText(to))) { return []; } - return [{ original, copy, typeChanged: from !== to }]; + return [{ original, intermediates, copy, typeChanged: from !== to }]; }); } +/** + * The deleted copies `copy` was made from, oldest first. A change that copies + * a block inserts the copy and deletes the block it copies, in one update: so + * the copy before is the one whose deletion has the same users and times as + * this copy's insertion. Stops where that isn't exactly one block. + */ +function copyChain( + copy: Y.Node, + blocks: Y.Node[], + attributions: Y.ContentMap, +): Y.Node[] { + const chain: Y.Node[] = []; + const left = new Set(blocks); + for (let current = copy; ;) { + const change = changeOf(current._item!, attributions.inserts, "insert"); + let previous = [...left].filter( + (block) => + change !== undefined && + changeOf(block._item!, attributions.deletes, "delete") === change, + ); + // One update can copy a block more than once (e.g. retype, then indent): + // of its deleted copies, the newer one was also made by that update. + if (previous.length > 1) { + previous = previous.filter( + (block) => + changeOf(block._item!, attributions.inserts, "insert") === change, + ); + } + if (previous.length !== 1) { + return chain; + } + chain.unshift(previous[0]); + left.delete(previous[0]); + current = previous[0]; + } +} + +/** + * Pair the copy's content that the original doesn't have with the + * intermediate copy it was typed into: the oldest one holding it. + */ +function matchTypedInCopies( + intermediates: Y.Node[], + copy: Y.Node, + baseline: Y.Doc, + pairs: Array<[Y.ID, Y.ID]>, +) { + const copyContent = contentOf(copy, baseline); + if (!copyContent) { + return; + } + const paired = Y.createIdSet(); + for (const [, b] of pairs) { + paired.add(b.client, b.clock, 1); + } + const unpaired = units(copyContent).filter( + ({ id }) => !paired.has(id.client, id.clock), + ); + for (const intermediate of intermediates) { + const content = contentOf(intermediate, baseline); + if (!content) { + continue; + } + const found: Array<[Y.ID, Y.ID]> = []; + matchUnits(units(content), unpaired, found); + for (const [a, b] of found) { + if (!paired.has(b.client, b.clock)) { + pairs.push([a, b]); + paired.add(b.client, b.clock, 1); + } + } + } +} + +/** The users and times of an item's insertion or deletion, as a key. */ +function changeOf( + item: Y.Item, + map: Y.IdMap, + kind: "insert" | "delete", +): string | undefined { + const attrs = map.slice(item.id.client, item.id.clock, 1)[0]?.attrs ?? []; + function values(name: string) { + return attrs + .filter((attr) => attr.name === name) + .map((attr) => String(attr.val)) + .sort() + .join(","); + } + const users = values(kind); + return users ? `${users}@${values(`${kind}At`)}` : undefined; +} + /** Whether some unit of `item` is in `lost` but not in `kept`. */ function losesUnit(item: Y.Item, lost: Y.IdSet, kept: Y.IdSet): boolean { for (let i = 0; i < item.length; i++) { @@ -680,12 +778,14 @@ function showCopiesOnce( const matches = copiedBlocks( doc, baseline, + Y.createContentMap(renderer.inserts, renderer.deletes), (type) => type !== undefined && editor.schema.blockSpecs[type]?.config.content === "inline", - ).map(({ original, copy, typeChanged }) => { + ).map(({ original, intermediates, copy, typeChanged }) => { const pairs: Array<[Y.ID, Y.ID]> = []; matchCopy(original, copy, baseline, pairs, typeChanged); + matchTypedInCopies(intermediates, copy, baseline, pairs); return { original, copy, typeChanged, pairs }; }); @@ -696,6 +796,13 @@ function showCopiesOnce( let shown = matches.filter((match) => inBaseline(baseline, match.original._item!.id), ); + const removedFrom = new Map(); + const originals = new Set(shown.map(({ original }) => original)); + for (const item of removedContent) { + for (const block of within(item, originals)) { + removedFrom.set(block, [...(removedFrom.get(block) ?? []), item]); + } + } for (let changed = true; changed;) { const kept = Y.createIdSet(); for (const { pairs } of shown) { @@ -705,16 +812,15 @@ function showCopiesOnce( } const next = shown.filter( ({ original }) => - !removedContent.some( - (item) => - (item === original._item || Y.isParentOf(original, item)) && - // Per unit: a deleted item can span content deleted before. - losesUnit(item, removed, kept), - ), + // Per unit: a deleted item can span content deleted before. + !removedFrom + .get(original) + ?.some((item) => losesUnit(item, removed, kept)), ); changed = next.length !== shown.length; shown = next; } + const shownMatches = new Set(shown); // A block that moved elsewhere also shows its original, struck through, at // the old place. One indented or outdented keeps its place in reading @@ -722,18 +828,23 @@ function showCopiesOnce( // struck through with it. const before = readingOrder(earlier.fragment); const after = readingOrder(snapshot.fragment); + const above = blocksAbove(before, new Set(after)); + const aboveNow = blocksAbove(after, new Set(before)); function isMove({ original, copy, typeChanged }: (typeof matches)[number]) { return !typeChanged && !inPlace(original, copy); } - const reorderedOriginals = shown - .filter( - (match) => isMove(match) && reordered(blockId(match.copy), before, after), - ) - .map(({ original }) => original); + // Another block above it than before: not only indented or outdented. + const reorderedOriginals = new Set( + shown + .filter( + (match) => + isMove(match) && + above.get(blockId(match.copy)) !== aboveNow.get(blockId(match.copy)), + ) + .map(({ original }) => original), + ); function struck(original: Y.Node) { - return reorderedOriginals.some( - (other) => other === original || Y.isParentOf(other, original._item!), - ); + return within(original._item!, reorderedOriginals).length > 0; } const unchanged = Y.createIdSet(); @@ -745,13 +856,19 @@ function showCopiesOnce( credits.push(...pairs); continue; } - if (!shown.includes(match)) { + if (!shownMatches.has(match)) { continue; } // A block that moved stays marked, as a move. One copied in place (its // children changed) shows unchanged. const move = isMove(match); for (const [a, b] of pairs) { + // Content added to the original after the earlier version isn't + // unchanged: the copy is credited to whoever added it. + if (!inBaseline(baseline, a)) { + credits.push([a, b]); + continue; + } if (!(move && struck(original))) { unchanged.add(a.client, a.clock, 1); } @@ -763,14 +880,28 @@ function showCopiesOnce( moved.push(copy._item!.id); } } - const movedFrom = itemsIn(doc, removed).filter((item) => - reorderedOriginals.some( - (original) => item === original._item || Y.isParentOf(original, item), - ), + const movedFrom = itemsIn(doc, removed).filter( + (item) => within(item, reorderedOriginals).length > 0, ); renderer.adjust(unchanged, credits, moved, movedFrom); } +/** The blocks among `blocks` that are `item`'s node or hold it. */ +function within(item: Y.Item, blocks: Set): Y.Node[] { + const found: Y.Node[] = []; + let node: Y.Node | null = + item.content instanceof Y.ContentType + ? item.content.type + : (item.parent as Y.Node); + while (node) { + if (blocks.has(node)) { + found.push(node); + } + node = (node._item?.parent as Y.Node | undefined) ?? null; + } + return found; +} + /** The ids of the blocks under `node`, top to bottom, nesting ignored. */ function readingOrder(node: Y.Node, out: unknown[] = []): unknown[] { for (let item = node._start; item !== null; item = item.right) { @@ -785,16 +916,22 @@ function readingOrder(node: Y.Node, out: unknown[] = []): unknown[] { } /** - * Whether the block with this id has another block above it than before, - * counting only blocks in both versions. An indent or outdent keeps it. + * For each block id in `order`, the block above it, counting only blocks in + * `other` too. An indent or outdent keeps it. */ -function reordered(id: unknown, before: unknown[], after: unknown[]): boolean { - function previous(order: unknown[], other: unknown[]) { - const inOther = new Set(other); - const common = order.filter((block) => inOther.has(block)); - return common[common.indexOf(id) - 1]; +function blocksAbove( + order: unknown[], + other: Set, +): Map { + const above = new Map(); + let previous: unknown = undefined; + for (const id of order) { + if (other.has(id)) { + above.set(id, previous); + previous = id; + } } - return previous(before, after) !== previous(after, before); + return above; } /** diff --git a/packages/core/src/y/extensions/versionDiffAttribution.test.ts b/packages/core/src/y/extensions/versionDiffAttribution.test.ts index a4b3506785..4cb5ca73d9 100644 --- a/packages/core/src/y/extensions/versionDiffAttribution.test.ts +++ b/packages/core/src/y/extensions/versionDiffAttribution.test.ts @@ -772,8 +772,7 @@ describe("version diff of a document several users wrote", () => { return wrong; } - // To be fixed by #3172. - it.fails("credits every word to its writer, between any two versions", () => { + it("credits every word to its writer, between any two versions", () => { const base = baseDocument([ { id: "start", type: "paragraph", content: "" }, ]);