[0.8] feat: set the update tag from yjs based on the origin #3608
Conversation
husseinraoouf
requested review from
trueadm,
zurfyx,
fantactuka,
acywatson,
tylerjbainbridge and
thegreatercurve
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hi @husseinraoouf! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
I'd call it It would also require adjustments for syncLexicalUpdateToYjs that checks for tag |
|
@fantactuka Makes sense I will update the pr
Can I ask What is the expected release date for 0.8 |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
… historic and clean up
|
@fantactuka @trueadm I updated the pr with the requested changes, thank you for the review |
Theres's no specific release date for 0.8, but we can do it in the next few days probably. |
|
Hey @acywatson any outlook on merging this one in soon? |
It needs to pass the integrity check first - looks like a prettier error |
| @@ -234,7 +235,7 @@ export function syncLexicalUpdateToYjs( | |||
| // types a character and we get it, we don't want to then insert | |||
| // the same character again. The exception to this heuristic is | |||
| // when we need to handle normalization merge conflicts. | |||
| if (tags.has('collaboration')) { | |||
| if (tags.has('collaboration') || tags.has('historic')) { | |||
There was a problem hiding this comment.
Do we need to do this part in order to accomplish the goal set out in the PR description? This seems like a behavioral change for the core, whereas I thought this PR was just trying to pass through the tags.
There was a problem hiding this comment.
@acywatson I thought i should handle both tags here because now the changes that come from the collaboration plugin could come with either of those two tags but if you want i can revert that
trueadm
changed the title
This PR makes the updates tags for updates coming from yjs change based on origin, so if the changes came from UndoManger the tag is set to
collaboration-historyand if the updates came from other places like websocket the tag is set tocollaboration,this is to be able to know which updates came from the user's interaction and which updates came from other users through yjs