fix: unsupported node mark #135
Conversation
src/plugins/sync-plugin.js
Outdated
| const prefix = 'yelement_mark_' | ||
| marks.forEach((mark) => { | ||
| if (mark.type.name !== 'ychange') { | ||
| pattrs[`${prefix}${mark.type.name}`] = JSON.stringify(mark.attrs || {}) |
There was a problem hiding this comment.
Is there a .toJSON method on mark.attrs or is it actually already a JSON object?
| } | ||
| } else { | ||
| yDomFragment.removeAttribute(key) | ||
| } | ||
| } | ||
| // remove all keys that are no longer in pAttrs | ||
| for (const key in yDomAttrs) { | ||
| if (pAttrs[key] === undefined) { | ||
| if (attrs[key] === undefined) { |
There was a problem hiding this comment.
Here, we clean up unused attributes. When are unused marks removed?
src/plugins/sync-plugin.js
Outdated
| @@ -780,7 +796,7 @@ const createTypeFromTextOrElementNode = (node, mapping) => | |||
| const isObject = (val) => typeof val === 'object' && val !== null | |||
|
|
|||
| const equalAttrs = (pattrs, yattrs) => { | |||
| const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null) | |||
| const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null && !key.includes('yelement_mark_')) | |||
src/plugins/sync-plugin.js
Outdated
| const key = keys[i]; | ||
| const l = JSON.stringify(pMarkAttr[key]) | ||
| const r = yattrs[key] | ||
| eq = key === 'ychange' || l === r ; |
There was a problem hiding this comment.
Once l and r are objects. You could use f.equalityDeep to compare them. Where import * as f from 'lib0/function'
src/plugins/sync-plugin.js
Outdated
| for (const key in attrs) { | ||
| if (key.startsWith('yelement_mark_')) { | ||
| const markName = key.replace('yelement_mark_',''); | ||
| let markValue = attrs[key]; |
src/plugins/sync-plugin.js
Outdated
| @@ -780,7 +797,7 @@ const createTypeFromTextOrElementNode = (node, mapping) => | |||
| const isObject = (val) => typeof val === 'object' && val !== null | |||
|
|
|||
| const equalAttrs = (pattrs, yattrs) => { | |||
| const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null) | |||
| const keys = Object.keys(pattrs).filter((key) => pattrs[key] !== null && !key.startsWith('yelement_mark_')) | |||
There was a problem hiding this comment.
Why are you filtering yelement_mark_ here in the attributes that you get from the prosemirror node? If there is a reason, why are you not filtering in the next line when you are comparing?
src/plugins/sync-plugin.js
Outdated
| @@ -965,6 +1000,17 @@ const marksToAttributes = (marks) => { | |||
| return pattrs | |||
| } | |||
|
|
|||
| const nodeMarksToAttributes = (marks) => { | |||
| const pattrs = {} | |||
| const prefix = 'yelement_mark_' | |||
There was a problem hiding this comment.
Let's store this prefix globally somewhere as it is reused throughout the file. Also I suggest we use a shorter name that starts with an _. E.g. _mark_.
|
I meet similar sitituation in my custom prosemirror's schema.I'm not sure about when to use mark / attrs for a node. But in file y-prosemirror/src/lib.js， If marks are designed just for text node, and atts for non-text nodes, y-prosemirror will should not support marks for non-text nodes, as that will be anti prosemirror's design. It's a pleasure for any suggestion or guidance on how to use marks/attrs correctly, fellows the design. |
I have also need to handle this case In my understanding，marks act as a way to add attrs to prosemirror document , not only text node. In tiptap and Confluence Editor , also use mark to effect non-text node |
|
Looking good, very useful feature for format compatibility & avoiding data loss. @XiaodongTong - any plans to fix the final requested change and add tests? |
|
@dmonad are there any plans to merge this? I also face the very same issue and wanted to raise a PR. Then I noticed this one here. I can also complete this work if you like |
|
What's actuality pending here? I'd like to contribute to get this thing through |
fix #47
Using『node mark』 as an attribute on YElement，Each mark corresponds to an YElement attribute.
Runable in my project.
Hope to merge