From c9dc7011e7df844ca7aa7ce2053dad65764e08b5 Mon Sep 17 00:00:00 2001 From: Laurens Lavaert Date: Tue, 2 Jul 2024 16:03:27 +0000 Subject: [PATCH 1/3] feat: allow marks to exist on all nodes that support it --- src/lib.js | 18 +++++++++-- src/plugins/sync-plugin.js | 66 +++++++++++++++++++++++++++++++++----- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/src/lib.js b/src/lib.js index 820c13d3..2802c93c 100644 --- a/src/lib.js +++ b/src/lib.js @@ -1,4 +1,4 @@ -import { updateYFragment, createNodeFromYElement } from './plugins/sync-plugin.js' // eslint-disable-line +import { updateYFragment, createNodeFromYElement, MarkPrefix } from './plugins/sync-plugin.js' // eslint-disable-line import { ySyncPluginKey } from './plugins/keys.js' import * as Y from 'yjs' import { EditorView } from 'prosemirror-view' // eslint-disable-line @@ -416,8 +416,20 @@ export function yXmlFragmentToProsemirrorJSON (xmlFragment) { } const attrs = item.getAttributes() - if (Object.keys(attrs).length) { - response.attrs = attrs + + // Add all non-mark attributes to the element + for (const key of Object.keys(attrs).filter((key) => !key.startsWith(MarkPrefix))) { + if (!response.attrs) response.attrs = {} + response.attrs[key] = attrs[key] + } + + // Check whether the attrs contains marks, if so, add them to the response + if (Object.keys(attrs).some((key) => key.startsWith(MarkPrefix))) { + response.marks = Object.keys(attrs) + .filter((key) => key.startsWith(MarkPrefix)) + .map((key) => { + return attrs[key] + }) } const children = item.toArray() diff --git a/src/plugins/sync-plugin.js b/src/plugins/sync-plugin.js index b548aef7..2fc7d183 100644 --- a/src/plugins/sync-plugin.js +++ b/src/plugins/sync-plugin.js @@ -20,6 +20,9 @@ import * as random from 'lib0/random' import * as environment from 'lib0/environment' import * as dom from 'lib0/dom' import * as eventloop from 'lib0/eventloop' +import * as f from 'lib0/function' + +export const MarkPrefix = '_mark_' /** * @param {Y.Item} item @@ -723,7 +726,22 @@ export const createNodeFromYElement = ( : { type: 'added' } } } - const node = schema.node(el.nodeName, attrs, children) + const nodeAttrs = {} + const nodeMarks = [] + + for (const key in attrs) { + if (key.startsWith(MarkPrefix)) { + const markName = key.replace(MarkPrefix, '') + const markValue = attrs[key] + if (isObject(markValue)) { + nodeMarks.push(schema.mark(markName, /** @type {Object} */ (markValue).attrs)) + } + } else { + nodeAttrs[key] = attrs[key] + } + } + + const node = schema.node(el.nodeName, nodeAttrs, children, nodeMarks) mapping.set(el, node) return node } catch (e) { @@ -802,12 +820,16 @@ const createTypeFromTextNodes = (nodes, mapping) => { */ const createTypeFromElementNode = (node, mapping) => { const type = new Y.XmlElement(node.type.name) + const nodeMarksAttr = nodeMarksToAttributes(node.marks) for (const key in node.attrs) { const val = node.attrs[key] if (val !== null && key !== 'ychange') { type.setAttribute(key, val) } } + for (const key in nodeMarksAttr) { + type.setAttribute(key, nodeMarksAttr[key]) + } type.insert( 0, normalizePNodeContent(node).map((n) => @@ -835,7 +857,7 @@ const equalAttrs = (pattrs, yattrs) => { const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null) let eq = keys.length === - Object.keys(yattrs).filter((key) => yattrs[key] !== null).length + Object.keys(yattrs).filter((key) => yattrs[key] !== null && !key.startsWith(MarkPrefix)).length for (let i = 0; i < keys.length && eq; i++) { const key = keys[i] const l = pattrs[key] @@ -846,6 +868,20 @@ const equalAttrs = (pattrs, yattrs) => { return eq } +const equalMarks = (pmarks, yattrs) => { + const keys = Object.keys(yattrs).filter((key) => key.startsWith(MarkPrefix)) + let eq = + keys.length === pmarks.length + const pMarkAttr = nodeMarksToAttributes(pmarks) + for (let i = 0; i < keys.length && eq; i++) { + const key = keys[i] + const l = pMarkAttr[key] + const r = yattrs[key] + eq = key === 'ychange' || f.equalityDeep(l, r) + } + return eq +} + /** * @typedef {Array|PModel.Node>} NormalizedPNodeContent */ @@ -900,7 +936,8 @@ const equalYTypePNode = (ytype, pnode) => { ) { const normalizedContent = normalizePNodeContent(pnode) return ytype._length === normalizedContent.length && - equalAttrs(ytype.getAttributes(), pnode.attrs) && + equalAttrs(pnode.attrs, ytype.getAttributes()) && + equalMarks(pnode.marks, ytype.getAttributes()) && ytype.toArray().every((ychild, i) => equalYTypePNode(ychild, normalizedContent[i]) ) @@ -1017,6 +1054,16 @@ const marksToAttributes = (marks) => { return pattrs } +const nodeMarksToAttributes = (marks) => { + const pattrs = {} + marks.forEach((mark) => { + if (mark.type.name !== 'ychange') { + pattrs[`${MarkPrefix}${mark.type.name}`] = mark.toJSON() + } + }) + return pattrs +} + /** * Update a yDom node by syncing the current content of the prosemirror node. * @@ -1042,10 +1089,13 @@ export const updateYFragment = (y, yDomFragment, pNode, mapping) => { if (yDomFragment instanceof Y.XmlElement) { const yDomAttrs = yDomFragment.getAttributes() const pAttrs = pNode.attrs - for (const key in pAttrs) { - if (pAttrs[key] !== null) { - if (yDomAttrs[key] !== pAttrs[key] && key !== 'ychange') { - yDomFragment.setAttribute(key, pAttrs[key]) + const pNodeMarksAttr = nodeMarksToAttributes(pNode.marks) + const attrs = { ...pAttrs, ...pNodeMarksAttr } + + for (const key in attrs) { + if (attrs[key] !== null) { + if (yDomAttrs[key] !== attrs[key] && key !== 'ychange') { + yDomFragment.setAttribute(key, attrs[key]) } } else { yDomFragment.removeAttribute(key) @@ -1053,7 +1103,7 @@ export const updateYFragment = (y, yDomFragment, pNode, mapping) => { } // remove all keys that are no longer in pAttrs for (const key in yDomAttrs) { - if (pAttrs[key] === undefined) { + if (attrs[key] === undefined) { yDomFragment.removeAttribute(key) } } From fd0bb448403a237298b696c67d10063e38c3a22c Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Fri, 22 Nov 2024 14:24:51 +0100 Subject: [PATCH 2/3] test: add test for marks on nodes --- tests/y-prosemirror.test.js | 65 ++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/tests/y-prosemirror.test.js b/tests/y-prosemirror.test.js index 730818c3..477a5d90 100644 --- a/tests/y-prosemirror.test.js +++ b/tests/y-prosemirror.test.js @@ -23,7 +23,7 @@ import { findWrapping } from 'prosemirror-transform' import { schema as complexSchema } from './complexSchema.js' import * as promise from 'lib0/promise' -const schema = /** @type {any} */ (basicSchema.schema) +const schema = /** @type {import('prosemirror-model').Schema} */ (basicSchema.schema) /** * Verify that update events in plugins are only fired once. @@ -452,6 +452,69 @@ export const testAddToHistoryIgnore = (_tc) => { ) } +/** + * @param {t.TestCase} tc + */ +export const testAddMarkToNode = (_tc) => { + const view = createNewProsemirrorView(new Y.Doc()) + view.dispatch( + view.state.tr.insert( + 0, + /** @type {any} */ (schema.node('image', { src: 'http://example.com', alt: null, title: null }, [], [schema.mark('link', { href: 'https://yjs.dev', title: null })])) + ) + ) + // convert to JSON and back because the ProseMirror schema messes with the constructors + const stateJSON = JSON.parse(JSON.stringify(view.state.doc.toJSON())) + // test if transforming back and forth from yXmlFragment works + const xml = new Y.XmlFragment() + prosemirrorJSONToYXmlFragment(/** @type {any} */ (schema), stateJSON, xml) + const doc = new Y.Doc() + doc.getMap('root').set('firstDoc', xml) + // test if transforming back and forth from Yjs doc works + const backandforth = JSON.parse(JSON.stringify(yXmlFragmentToProsemirrorJSON(xml))) + // Add the missing alt and title attributes, these are defaults for the image mark + backandforth.content[0].content[0].attrs.alt = null + backandforth.content[0].content[0].attrs.title = null + + t.assert(backandforth.content[0].content[0].marks.length === 1, 'node has one mark') + t.compare(stateJSON, backandforth, 'marks on nodes are preserved') +} + +/** + * @param {t.TestCase} tc + */ +export const testRemoveMarkFromNode = (_tc) => { + const view = createNewProsemirrorView(new Y.Doc()) + view.dispatch( + view.state.tr.insert( + 0, + /** @type {any} */ (schema.node('image', { src: 'http://example.com', alt: null, title: null }, [], [schema.mark('link', { href: 'https://yjs.dev', title: null })])) + ) + ) + t.assert(view.state.doc.content.firstChild.content.firstChild.marks.length === 1, 'node has one mark') + + view.dispatch( + view.state.tr.removeNodeMark(1, schema.marks.link) + ) + t.assert(view.state.doc.content.firstChild.content.firstChild.marks.length === 0, 'node has no marks') + // convert to JSON and back because the ProseMirror schema messes with the constructors + const stateJSON = JSON.parse(JSON.stringify(view.state.doc.toJSON())) + // test if transforming back and forth from yXmlFragment works + const xml = new Y.XmlFragment() + prosemirrorJSONToYXmlFragment(/** @type {any} */ (schema), stateJSON, xml) + const doc = new Y.Doc() + doc.getMap('root').set('firstDoc', xml) + + // test if transforming back and forth from Yjs doc works + const backandforth = JSON.parse(JSON.stringify(yXmlFragmentToProsemirrorJSON(xml))) + // Add the missing alt and title attributes, these are defaults for the image mark + backandforth.content[0].content[0].attrs.alt = null + backandforth.content[0].content[0].attrs.title = null + + t.assert(backandforth.content[0].content[0].marks === undefined, 'node has no marks') + t.compare(stateJSON, backandforth) +} + const createNewProsemirrorViewWithSchema = (y, schema, undoManager = false) => { const view = new EditorView(null, { // @ts-ignore From 235d647e8d246f6b16cf82b576a35819d212c767 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Fri, 22 Nov 2024 15:51:21 +0100 Subject: [PATCH 3/3] feat: support overlapping marks #34 based on (#52) --- src/lib.js | 35 +++++++++++++++++++++++++++-------- src/plugins/sync-plugin.js | 35 +++++++++++++++++++++++++++++------ tests/complexSchema.js | 12 ++++++++++++ tests/y-prosemirror.test.js | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 14 deletions(-) diff --git a/src/lib.js b/src/lib.js index 2802c93c..41c18e82 100644 --- a/src/lib.js +++ b/src/lib.js @@ -395,18 +395,37 @@ export function yXmlFragmentToProsemirrorJSON (xmlFragment) { } if (d.attributes) { - text.marks = Object.keys(d.attributes).map((type) => { + const marks = [] + + Object.keys(d.attributes).forEach((type) => { const attrs = d.attributes[type] - const mark = { - type - } + if (Array.isArray(attrs)) { + // multiple marks of same type + attrs.forEach(singleAttrs => { + const mark = { + type + } - if (Object.keys(attrs)) { - mark.attrs = attrs - } + if (Object.keys(singleAttrs)) { + mark.attrs = singleAttrs + } + + marks.push(mark) + }) + } else { + const mark = { + type + } - return mark + if (Object.keys(attrs)) { + mark.attrs = attrs + } + + marks.push(mark) + } }) + + text.marks = marks } return text }) diff --git a/src/plugins/sync-plugin.js b/src/plugins/sync-plugin.js index 2fc7d183..ac471cb3 100644 --- a/src/plugins/sync-plugin.js +++ b/src/plugins/sync-plugin.js @@ -6,7 +6,6 @@ import { createMutex } from 'lib0/mutex' import * as PModel from 'prosemirror-model' import { Plugin, TextSelection } from "prosemirror-state"; // eslint-disable-line import * as math from 'lib0/math' -import * as object from 'lib0/object' import * as set from 'lib0/set' import { simpleDiff } from 'lib0/diff' import * as error from 'lib0/error' @@ -735,6 +734,8 @@ export const createNodeFromYElement = ( const markValue = attrs[key] if (isObject(markValue)) { nodeMarks.push(schema.mark(markName, /** @type {Object} */ (markValue).attrs)) + } else if (Array.isArray(markValue)) { + nodeMarks.push(...markValue.map(attrs => schema.mark(markName, attrs))) } } else { nodeAttrs[key] = attrs[key] @@ -779,7 +780,15 @@ const createTextNodesFromYText = ( const delta = deltas[i] const marks = [] for (const markName in delta.attributes) { - marks.push(schema.mark(markName, delta.attributes[markName])) + if (Array.isArray(delta.attributes[markName])) { + // multiple marks of same type + delta.attributes[markName].forEach(attrs => { + marks.push(schema.mark(markName, attrs)) + }) + } else { + // single mark + marks.push(schema.mark(markName, delta.attributes[markName])) + } } nodes.push(schema.text(delta.insert, marks)) } @@ -918,9 +927,14 @@ const equalYTextPText = (ytext, ptexts) => { return delta.length === ptexts.length && delta.every((d, i) => d.insert === /** @type {any} */ (ptexts[i]).text && - object.keys(d.attributes || {}).length === ptexts[i].marks.length && - ptexts[i].marks.every((mark) => - equalAttrs(d.attributes[mark.type.name] || {}, mark.attrs) + Object.keys(d.attributes || {}).reduce((sum, val) => sum + (Array.isArray(val) ? val.length : 1), 0) === ptexts[i].marks.length && + ptexts[i].marks.every((mark) => { + const yattrs = d.attributes || {} + if (Array.isArray(yattrs)) { + return yattrs.some((yattr) => equalAttrs(mark.attrs, yattr)) + } + return equalAttrs(mark.attrs, yattrs) + } ) ) } @@ -1048,7 +1062,16 @@ const marksToAttributes = (marks) => { const pattrs = {} marks.forEach((mark) => { if (mark.type.name !== 'ychange') { - pattrs[mark.type.name] = mark.attrs + if (pattrs[mark.type.name] && Array.isArray(pattrs[mark.type.name])) { + // already has multiple marks of same type + pattrs[mark.type.name].push(mark.attrs) + } else if (pattrs[mark.type.name]) { + // already has mark of same type, change to array + pattrs[mark.type.name] = [pattrs[mark.type.name], mark.attrs] + } else { + // first mark of this type + pattrs[mark.type.name] = mark.attrs + } } }) return pattrs diff --git a/tests/complexSchema.js b/tests/complexSchema.js index 659175f3..65f06270 100644 --- a/tests/complexSchema.js +++ b/tests/complexSchema.js @@ -157,6 +157,7 @@ export const nodes = { const emDOM = ['em', 0] const strongDOM = ['strong', 0] const codeDOM = ['code', 0] +const commentDOM = ['span', 0] // :: Object [Specs](#model.MarkSpec) for the marks in the schema. export const marks = { @@ -224,6 +225,17 @@ export const marks = { } }, + comment: { + attrs: { + id: { default: null } + }, + exclude: '', // allow multiple "comments" marks to overlap + parseDOM: [{ tag: 'span' }], + toDOM () { + return commentDOM + } + }, + ychange: { attrs: { user: { default: null }, diff --git a/tests/y-prosemirror.test.js b/tests/y-prosemirror.test.js index 477a5d90..101a11d4 100644 --- a/tests/y-prosemirror.test.js +++ b/tests/y-prosemirror.test.js @@ -702,3 +702,37 @@ export const testRepeatGenerateProsemirrorChanges300 = tc => { checkResult(applyRandomTests(tc, pmChanges, 300, createNewProsemirrorView)) } */ + +/** + * @param {t.TestCase} tc + */ +export const testDuplicateMarks = tc => { + const ydoc = new Y.Doc() + const type = ydoc.getXmlFragment('prosemirror') + const view = createNewComplexProsemirrorView(ydoc) + t.assert(type.toString() === '', 'should only sync after first change') + + view.dispatch( + view.state.tr.setNodeMarkup(0, undefined, { + checked: true + }) + ) + + const marks = [complexSchema.mark('comment', { id: 0 }), complexSchema.mark('comment', { id: 1 })] + view.dispatch(view.state.tr.insert(view.state.doc.content.size - 1, /** @type {any} */ complexSchema.text('hello world', marks))) + const stateJSON = JSON.parse(JSON.stringify(view.state.doc.toJSON())) + + // remove the attrs, because ychange is setting a default value + delete stateJSON.content[1].attrs + + // test if transforming back and forth from Yjs doc works + const backandforth = JSON.parse(JSON.stringify(yDocToProsemirrorJSON(prosemirrorJSONToYDoc(/** @type {any} */ (complexSchema), stateJSON)))) + + console.dir(stateJSON, { depth: null }) + console.dir(backandforth, { depth: null }) + + t.compare(stateJSON, backandforth) + + // TODO: create a toString test, this currently fails because YXmlText breaks + // t.compareStrings(type.toString(), '') +}