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

UI destruction needs to be repeatable #5372

Closed
Reinmar opened this issue Jun 12, 2017 · 1 comment · Fixed by ckeditor/ckeditor5-ui#254
Closed

UI destruction needs to be repeatable #5372

Reinmar opened this issue Jun 12, 2017 · 1 comment · Fixed by ckeditor/ckeditor5-ui#254
Assignees
Labels
package:ui type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Jun 12, 2017

As explained in #114 (comment) and in the following comments.

Theoretically, it's simple – if you initialise some components in A, B, C order, then they need to be destroyed in C, B, A order.

So, e.g. if the editor first loads plugins and then inits the UI, the UI needs to be destroyed before the plugins are destroyed.

However, what if one of the plugins created some UI and added it to editor UI's collection? On editor destroy, first the UI will be destroyed (which is recursive and will destroy that plugin's component too) and then the plugins will be destroyed and one of these plugins may want to somehow destroy the component which it created (it might've even initialised it, but usually plugins don't do that).

We've got a collision of responsibilities here. editor.ui.destroy() is recursive so it destroys things which the editor creator didn't create. And then plugins come and try to destroy those things again.

The solution is explained in #114 (comment).

@Reinmar
Copy link
Member Author

Reinmar commented Jun 12, 2017

Actually, this issue includes two things:

  1. Making the UI framework safer to destroy.
  2. Reviewing the plugins which it brings because e.g. ContextualBalloon does too much in its destroy() method.

@oleq oleq self-assigned this Jun 19, 2017
Reinmar referenced this issue in ckeditor/ckeditor5-ui Jun 20, 2017
Other: Made the UI destruction a fail–safe, repeatable process. Closes #248.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-ui Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. package:ui labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:ui type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants