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

UIElement children #3971

Closed
pjasiun opened this issue Feb 1, 2017 · 1 comment · Fixed by ckeditor/ckeditor5-engine#967
Closed

UIElement children #3971

pjasiun opened this issue Feb 1, 2017 · 1 comment · Fixed by ckeditor/ckeditor5-engine#967
Labels
package:engine type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@pjasiun
Copy link

pjasiun commented Feb 1, 2017

tl;dr: UIElement children should not be stored in the view.

So we decided to introduce UIElement (https://github.com/ckeditor/ckeditor5-engine/issues/788) and we decided to disable adding children to it in the very first version to keep things simple at the beginning. Now the question is how children of the UI element should be handled.

The first option seems to be natural: UIElement can have children in the view like any other element. The profit is that they will be handled like any other children. The problem is that they will be handled like any other childer ;) For instance TreeWalker will enter there and the developer needs to ensure not to thread text in the UIElement like a text in the content. Also, these children should be never moved out of this UIElement, should not accept selection and have no model representation. More I think about them more I feel that view does not care about the UIElement children.

So now the second option appear: what if the UIElement is always empty in the view, but have the whole structure hidden. Developer mark it only to be re-rendered, if needed. The renderer asks the UIElement for its children and inserts then into the DOM element. Thanks to it the element will be always simple in the view but may have a very complex internal structure in the DOM. It can also handle events (click, drag) internally what will be also transparent for the view, observers, and converters.

I think that the second approach will be more flexible: we will be able to implement any UI component and do not care how it will affect the view.

To make the API consistent, every view node could have a render() method which will create DOM element/elements. Now its renderer who create a DOM node based on the view, but moving it out of the renderer is something I was thinking about for a long time.

@pjasiun
Copy link
Author

pjasiun commented May 29, 2017

In the F2F talk, we agreed that the first step should be to add the render() method only to the UIElement. It should be used by the DOMCoverter. At the same time, observers have to be updated to handle events on the UIElement children properly. In particular, it should not be possible to put the selection inside the UIElement (it should be moved before or after it). Click event should be fired on the view.UIElement.

@szymonkups szymonkups self-assigned this Jun 5, 2017
pjasiun referenced this issue in ckeditor/ckeditor5-engine Jun 14, 2017
Feature: UIElement has it's own render method used by DomConverter and can create DOM children. Improved integration with observers and other view elements. Closes #799.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added module:view type:improvement This issue reports a possible enhancement of an existing feature. package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants