search

BitPhinix / slate-yjs

Yjs binding for Slate
https://docs.slate-yjs.dev
MIT License
514 stars 73 forks source link

Ignore retain updates for XmlText #392

Closed ezeidman closed 1 year ago

ezeidman commented 1 year ago

Updates applyToSlate to ignore retain attributes that correspond with XmlText nodes. We already ignore insert attributes for XmlText nodes so should do the same for retains for consistency:

https://github.com/BitPhinix/slate-yjs/blob/966b9325ff312058023d017f857fbb6ac9a7a8a4/packages/core/src/utils/convert.ts#L17-L23

XmlText do have attributes but these are different from the attributes on inserts and retains

  1. insert and retain attributes are "formatting" attributes that are set by XmlText#format or XmlText#insert (and apply to the children of the XmlText)
  2. XmlText attributes are set by XmlText#setAttribute and unaffected by formatting (even if XmlText is nested underneath XmlText)

We still use and update the second kind of attribute:

Initially I thought this was a Yjs issue so there is some more context here: https://github.com/yjs/yjs/issues/529. A big part of the problem is that if you have a mixture of text and XmlText underneath a node, formatting attributes that were only ever applied to the text can show up in retain attributes that apply to non-text. This results in non-text nodes in the Slate tree getting nonsensical text attributes.

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: dbe9895692faa92283861f3cd7e0378004f8284c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | -------------------------- | ----- | | @slate-yjs/core | Major | | @slate-yjs/react | Major | | @slate-yjs/example-backend | Patch | | @slate-yjs/example | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

ezeidman commented 1 year ago

@BitPhinix I know you said you're very busy. Would be very grateful if you can review this week :) Thank you!

BitPhinix commented 1 year ago

Thanks for this and sorry for the delay - will create a new version now