-
-
Notifications
You must be signed in to change notification settings - Fork 569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix retains for embedded types in YTextEvent #530
Conversation
e4ede11
to
63aafba
Compare
080407b
to
e1fd2de
Compare
Thank you! |
Could you clarify what you mean? The deltas from both before and after the PR are valid Quill deltas. The issue however is that retains associated with embedded types (e.g. an XmlText inside an XmlText) can contain unrelated attribute updates that are actually associated with surrounding text. This could be an erroneous null attribute like the example in the linked ticket or an erroneous non-null attribute value. An example:
where the retain is applying bold=false to the XmlText even though it's not bold and neither are its contents. There is one issue with this PR as is: it can create more retains than are strictly necessary. For example [retain 3, retain 4, retain 1], instead of just retain 8. That's still a valid Quill delta but not the most concise way to represent things. |
b47a6d2
to
bbbe4da
Compare
}) | ||
text0.delete(3, 3); | ||
testConnector.flushAllMessages() | ||
t.compare(deltas, [[{retain: 3}, { delete: 3}]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before the fix this is [[{"retain":3},{"delete":3},{"retain":4,"attributes":{"bold":false}}]]
}) | ||
text0.delete(0, 3); | ||
testConnector.flushAllMessages() | ||
t.compare(deltas, [[{ delete: 3}]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before the fix this is [[{"delete":3},{"retain":1,"attributes":{"bold":null}}]]
Don't include attributes in retains for embedded types since they're not subject to formatting
This fixes #529
Huly®: YJS-578