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.

Sign up for GitHub

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

Jump to bottom

Introduce Range#getContainedElement() #6364

Closed
Reinmar opened this issue Mar 2, 2020 · 1 comment · Fixed by ckeditor/ckeditor5-engine#1829
Closed

Introduce Range#getContainedElement() #6364

Reinmar opened this issue Mar 2, 2020 · 1 comment · Fixed by ckeditor/ckeditor5-engine#1829
Assignees
@oleq
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone
iteration 30 (v18...

Comments

@Reinmar
Copy link
Member

Reinmar commented Mar 2, 2020 •
edited
Loading

📝 Provide a description of the new feature

PRs like https://github.com/ckeditor/ckeditor5-table/pull/258/files and lots of other code that I've seen recently shows that we need such a method.

We need it in the model and in the view.

The second part of this issue is to clean up existing code, especially in ckeditor5-table. I think that it may be hard to scan our entire code base for those things, but looking got phrases like "get\w+element" may help. But this is not a priority to change all that now.

Strongly connected with: #6358.

If you'd like to see this feature implemented, add a 👍 reaction to this post.
@Reinmar Reinmar added the type:feature This issue reports a feature request (an idea for a new functionality or a missing option). label Mar 2, 2020
@Reinmar Reinmar mentioned this issue Mar 2, 2020
Closed
@Reinmar Reinmar added this to the iteration 30 milestone Mar 2, 2020
@Reinmar Reinmar added Epic and removed Epic labels Mar 2, 2020
This was referenced Mar 2, 2020
Merged
Closed
@Reinmar Reinmar assigned oleq and unassigned mlewand Mar 6, 2020
This was referenced Mar 6, 2020
Merged
Merged
@oleq
Copy link
Member

oleq commented Mar 6, 2020

I reviewed the codebase and I saw no use cases for such a method View Range. This kind of check is used only in (View)Selection#getSelectedElement().

@oleq oleq mentioned this issue Mar 9, 2020
Merged
Reinmar added a commit to ckeditor/ckeditor5-engine that referenced this issue Mar 9, 2020
@Reinmar
Merge pull request #1829 from ckeditor/i/6364
8fb1efa 
Feature: Implemented the model and view `Range#getContainedElement()` methods. Closes ckeditor/ckeditor5#6364.
mlewand added a commit to ckeditor/ckeditor5-table that referenced this issue Mar 10, 2020
@mlewand
Merge pull request #264 from ckeditor/i/6364
9ab3750 
Internal: Replaced the `getRangeContainedElement()` helper with `Range#getContainedElement()`. See [ckeditor/ckeditor5#6364](ckeditor/ckeditor5#6364).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oleq oleq

Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Milestone
iteration 30 (v18.0.0)
Development

Successfully merging a pull request may close this issue.

i/6364: Implemented the Range#getContainedElement() method. ckeditor/ckeditor5-engine
3 participants
@Reinmar @oleq @mlewand