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

Introduce Range#hasContent to check if there is any content in a Range #4168

Closed
scofalik opened this issue Aug 30, 2017 · 4 comments · Fixed by ckeditor/ckeditor5-engine#1115
Assignees
Labels
package:engine status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@scofalik
Copy link
Contributor

From time to time we have a feature, in which we have to do something if given range has (or does not have) content.

This is tricky.

What is "content"? When do we want to check "content"? When a user perceive that something has or does not have "content"?

For me, the content is anything that holds some data. Grouping elements, styling elements, ui elements, these are not content. Text and data elements like images are content. These are represented by text nodes and empty elements.

Now the tricky question is whether for example list item or (empty) block quote holds any meaningful data. Though we could argue, I think we can assume that they are not, to simplify the concept.

If you agree that data is Text and EmptyElement it is clear that "not-having-content" is something else than "start position touching end position". Position touching is something else and is used for comparing whether two position are very similar.

Therefore I am for introducing Range#hasContent method, that would traverse through the range (using TreeWalker) and check if the range has any content. I propose this name because it is more meaningful than isEmpty, which could be mistaken with isCollapsed.

Now, it is a bit easier to implement this in view than in model. in view we have elements that already has some meaning, like AttributeElement, ContainerElement, UIElement and EmptyElement. In model, though, we have only Element and Text. So it is not possible to tell something that is converted to empty ContainerElement from something that is converted to EmptyElement.

In view the function would check if there is any Text or EmptyElement in the range. For model, we need to enhance the tree to implement this function.

@scofalik scofalik self-assigned this Aug 30, 2017
@Reinmar
Copy link
Member

Reinmar commented Aug 30, 2017

F2F discussion's results:

@method DataController#isEmpty
@param {model/Element|model/Range} [elementOrRange=mainEditable]

The method will need to use schema to check if an element is an object or not. If it's an object, then it's "a content" itself.

@pjasiun
Copy link

pjasiun commented Aug 30, 2017

Also, closing this, we should also close #401 finally.

@scofalik
Copy link
Contributor Author

I had some doubts but if such information is already in Schema then no matter where we will put this algorithm it will always have to have Schema instance passed. So I am fine with keeping this in something that already has Schema.

@scofalik
Copy link
Contributor Author

However, this way we don't have solution for view. It's okay for now as for now we wanted to make such checks only on model. Also, as I stated, it is easier to check on view.

oskarwrobel referenced this issue in ckeditor/ckeditor5-engine Sep 1, 2017
Feature: Introduced `controller.DataController#hasContent`. Closes #1114.
@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:model status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option). 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 status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants