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

TreeView to TreeModel conversion controller #3544

Closed
pjasiun opened this issue Feb 4, 2016 · 5 comments
Closed

TreeView to TreeModel conversion controller #3544

pjasiun opened this issue Feb 4, 2016 · 5 comments
Assignees
Milestone

Comments

@pjasiun
Copy link

pjasiun commented Feb 4, 2016

  • The converter will be able to convert whole document or part of it, including the document fragment (multiple siblings without a parent, Throw an error when user tries to use inappropriate editor over textarea ckeditor5-core#173).
  • It will be configurable, by callbacks fired for every converted item (elements, texts).
  • Callbacks can be assigned to the specific elements, because of the performance.
  • It will be possible to register generic callback, called every time.
  • Callbacks will have priorities.
  • User will be able to set the position for the next converter callback (mess the document, and said where the conversion cursor should be put), by default the cursor will be moved to the next position.
  • Each step will have a list of items to consume (classes, attributes, styles, name, text), and methods:
    • consume - will remove item from that list,
    • isConsumed - check if the item has been consumed.
  • The list of items to consume will shrink, not grow, because we would end up with the whole list next to each item at the end of the conversion. We could initialise the list when it is used for the first time on each item.
  • There will be tool to create basic converter (e.g. attribute converter).
@pjasiun pjasiun changed the title TreeView to TreeModel converter TreeView to TreeModel conversion controller Feb 18, 2016
@pjasiun
Copy link
Author

pjasiun commented Feb 18, 2016

Note that TreeView to TreeModel conversion will be called on:

  • initialisation,
  • paste/drop,
  • setData.

In all cases it is data insertion. In contrast to model -> view conversion it does not need to be able to handle move or deletion. If one select text and pressed "delete" proper controller will map selected range and delete content on the model. No data conversion is needed.

@pjasiun pjasiun self-assigned this Feb 18, 2016
@pjasiun
Copy link
Author

pjasiun commented Feb 18, 2016

One of the problems TreeView to TreeModel controller need to handle is incorrect data. It need to do some clean-up based on the registered controllers. This controller work is similar to the ACF in the CKEditor 4.

For example if the <h1> element is converting:

  • header converter should handle it, if exists,
  • if headed converter does not exist, the paragraph controller should handle it and transform it into a paragraph,
  • if the paragraph converted is not able to handle it, maybe a <br> need to be inserted,
  • at the end of the chain, there may be an universal controller, equivalent of the allowedContent: true option, with will transform any HTML tag into a model element.

@pjasiun
Copy link
Author

pjasiun commented Feb 18, 2016

During the F2F discussion with @scofalik we realised a problem with consuming mechanism.

Consider such example.

Lets have such view to convert into a model:
<p><b><em>foo</em>bar</b></p>

The cursor is at the begging and convert a <p>:

|<p><b><em>foo</em>bar</b></p> -> p

The cursor move and do nothing, because it can not convert <b> without text. That is fine.

<p>|<b><em>foo</em>bar</b></p> -> p
<p><b>|<em>foo</em>bar</b></p> -> p

The cursor meet text, transform it:

<p><b><em>|foo</em>bar</b></p> -> p: foo {}

The cursor stay on text a little longer and check the parent chain, if it finds there a <b> it adds attribute:

<p><b><em>|foo</em>bar</b></p> -> p: foo {bold: true}

The same for <em>:

<p><b><em>|foo</em>bar</b></p> -> p: foo {bold: true, em: true}

Then it moves to the next position and convert bar text:

<p><b><em>foo</em>|bar</b></p> -> p: foo {bold: true, em: true}, bar

And here we have a problem. it should check the list of parents and consume <b> adding bold: true attribute, but the <b> is already consumed.

We need some way to mark <b> only partially consumed.

@scofalik
Copy link
Contributor

I think that it's fine that <b> is not on consumables list. So TreeController (ConversionController?) will first look for consumables and then might look to the parents. Why is this logical?

When we look for consumables, we look for things like, i.e., class="bold" or data-myAttr="myVal" or even <p> to convert it to p element in TreeModel. This is fine. We can add <p> only once to the TreeModel and class="bold" is only on one item so it should be converted only once also.

When we look to the parent, the parent element is there for every child, so it's like every child has this special "thing" to consume. From conversion point of view, you could think that <b>foo<em>bar</em>a<span>b</span>c</b> is same as <b>foo</b><b><em>bar</em></b><b>a</b><b>span>b</span></b><b>c</b> and it should be treated like this.

The problem appears when two converters would like to convert parent element tag. We will either have to:

  • create a list of parents names for every converted node (this is not that inefficient actually as you might do it once for first visited element and then add/remove things to the list) and put it into consumables (the list will be refreshed for every node, which makes sense as other consumables are also evaluated for each converted node), or
  • make a separate "conversion" loop for parents (first we call all converters for this node and then we loop through parents of this node and try to apply parent-converters).

@pjasiun
Copy link
Author

pjasiun commented Feb 19, 2016

In short,we need a way to consume part of the element related with certain node. But at the same time it need to be possible to consume whole element, because during view - model conversion we are not able to distinguish between <b> type and <p> type.

@pjasiun pjasiun closed this as completed Apr 18, 2016
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the v0.1.0 milestone Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants