ckeditor / ckeditor5-react

Official CKEditor 5 React component.
https://ckeditor.com/ckeditor-5
Other
425 stars 99 forks source link

Fix: Update data of roots with modified content only #535

Closed f1ames closed 1 month ago

f1ames commented 2 months ago

Suggested merge commit message (convention)

Fix: Update data of roots with modified content only. Closes #534.


Additional information

See #534 for more details. TL;DR:

This happens because setData returned from useMultiRootEditor hook, expects all roots to be passed (since it diffs root list enabling to add/remove them) and existing roots are just updated with passed data, even if it's the same. Such update results in each root model data to be wiped out and re-added which is interpreted by revision history as complete data replace (since it works on model operations and does not diff data changes).

This PR slightly modifies the logic to update only the roots which has changed data.


I dug a bit in the context of below comment in the code:

// If any of the roots content has changed, set the editor data. // Unfortunately, we cannot set the editor data just for one root, // so we need to overwrite all roots (nextProps.data is an // object with data for each root).

But didn't find any case or code which would suggest that we can't do it. All tests green, manuals works fine AFAIT too.

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 0b08803a-48ca-427a-99e2-d30e6c5fb868

Details


Totals Coverage Status
Change from base Build 434d9ed7-0b25-40f8-a191-f513ef65280a: 0.0%
Covered Lines: 584
Relevant Lines: 584

💛 - Coveralls