Skip to content
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

Allowed to easily set, change, save and load root attributes #13759

Merged
merged 13 commits into from
Mar 28, 2023
Merged

Conversation

scofalik
Copy link
Contributor

@scofalik scofalik commented Mar 26, 2023

Suggested merge commit message (convention)

Other (engine): RootAttributeOperation is now correctly handled by Differ. Root attribute changes will be returned in Differ#getChangedRoots().

Internal (editor-multi-root): Introduced API to easily save and load root attributes. Introduced EditorConfig#rootsAttributes and MultiRootEditor#getRootsAttributes(). Closes #13395.

Internal (editor-multi-root): Introduced MultiRootEditor#getFullData(). The method returns document data for all roots in multi-root editor.

Internal (editor-multi-root): MultiRootEditor#addRoot() has a new option attributes.

Internal (engine): model.Writer#detachRoot() will now remove attributes from the detached root.


Additional information

Most things as internal because the changes will be in the changelog as "introduced multi-root editor type".

See #13395 (comment) for summary of proposed solution.

@scofalik scofalik requested a review from niegowski March 26, 2023 16:14
@Inviz
Copy link
Contributor

Inviz commented Mar 27, 2023

oh that's great that you are addressing this too!

@Inviz
Copy link
Contributor

Inviz commented Mar 27, 2023

Will this sync attributes set via dom using mutation observer?

@scofalik
Copy link
Contributor Author

This solution is for attributes understood as, hm, roots metadata (additional model root data). You can write converters to convert these roots attributes from model to view and then DOM. We will probably write such guide at some point. But in general, the flow should be:

  • initialize root attributes when initing editor (you can set DOM attributes in DOM/HTML too)
  • custom features allow to change these attributes through a custom UI
  • converters (in custom features) handle these attributes (e.g. change something in DOM in the process)
  • when you save the data, you save the roots attributes as well

Automatic sync through mutation observer in a way that you described (DOM -> model) is against the philosophy of how CKE5 works. Instead, the changes should be done on model and be propagated downwards to DOM. You can introduce a custom UI or any other method to change model roots attributes, but the propagation should always be model -> view -> DOM.

@Inviz
Copy link
Contributor

Inviz commented Mar 27, 2023

You can write converters to convert these roots attributes from model to view and then DOM.

Is it true now? I remember it was not possible before, for one reason or another it didn't really work. I remember filed issue on that, and i stumbled upon that myself too.

Automatic sync through mutation observer in a way that you described (DOM -> model) is against the philosophy of how CKE5 works. Instead, the changes should be done on model and be propagated downwards to DOM. You can introduce a custom UI or any other method to change model roots attributes, but the propagation should always be model -> view -> DOM.

You are right. I think i was confused about some other part that relied on syncing changes from the dom, but perhaps it was selection or something like that. Either way it doesn't matter.

@scofalik
Copy link
Contributor Author

scofalik commented Mar 28, 2023

Is it true now? I remember it was not possible before, for one reason or another it didn't really work. I remember filed issue on that, and i stumbled upon that myself too.

I think it is difficult currently, as root attributes changes were not buffered in Differ. After this PR, it will be a bit easier. But in general, even now, you can set attributes on roots. But it is not convenient to use them afterwards. One official plugin that uses it is HTML comments.

Copy link
Contributor

@niegowski niegowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Only minor comments.

@scofalik scofalik merged commit c121061 into master Mar 28, 2023
@scofalik scofalik deleted the ck/13395-3 branch March 28, 2023 21:18
@huzedong2015
Copy link

addRoot cannot add a new HTML element

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for storing additional data related to roots
4 participants