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

Implement TableUtils#getTable() #3233

Closed
Reinmar opened this issue Sep 20, 2018 · 4 comments · Fixed by #7598
Closed

Implement TableUtils#getTable() #3233

Reinmar opened this issue Sep 20, 2018 · 4 comments · Fixed by #7598
Assignees
@niegowski
Labels
package:table status:discussion type:debt This issue describes a technical debt. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:refactor This issue requires or describes a code refactoring.
Milestone
iteration 34 (v21...

Comments

@Reinmar
Copy link
Member

Reinmar commented Sep 20, 2018

We can simplify:

const table = editor.model.document.selection.getFirstPosition().getAncestors().find( item => item.name == 'table' );

To:

const table = tableUtils.getTable( editor.model.document.selection );

I was surprised to not find this function because I thought I saw it somewhere.
@jodator
Copy link
Contributor

jodator commented Sep 20, 2018 •
edited
Loading

@Reinmar:

https://github.com/ckeditor/ckeditor5-table/blob/d9f2790eec5d6b0aba6049216fb3694a3417c69a/src/commands/utils.js#L17-L27
?

used as

findAncestor( 'table', position );
findAncestor( 'tableCell', position );

@Reinmar
Copy link
Member Author

Reinmar commented Sep 20, 2018

So it's a quite generic function. Maybe we could add it to Position?

@jodator jodator self-assigned this Sep 21, 2018
@jodator jodator removed their assignment May 22, 2019
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-table Oct 9, 2019
@mlewand mlewand added this to the nice-to-have milestone Oct 9, 2019
@mlewand mlewand added status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:table labels Oct 9, 2019
@jodator
Copy link
Contributor

jodator commented May 26, 2020

We already have similar methods for View position/element. The findAncestor() is used extensively in tables.

@jodator jodator added squad:red type:debt This issue describes a technical debt. type:refactor This issue requires or describes a code refactoring. labels May 26, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Jun 23, 2020

Conclusion: Add findAncestor() to a model position and remove all the findAncestor() helper function use from cke5-table.

@Reinmar Reinmar modified the milestones: nice-to-have, iteration 34 Jun 23, 2020
@niegowski niegowski self-assigned this Jul 9, 2020
@niegowski niegowski mentioned this issue Jul 9, 2020
Merged
jodator added a commit that referenced this issue Jul 13, 2020
@jodator
Merge pull request #7598 from ckeditor/i/3233
a349af5 
Feature (engine): Added model `Position#findAncestor()` and `Element#findAncestor()` methods. Closes #3233.

Internal (table): Removed `findAncestor` helper.

MINOR BREAKING CHANGE (table): The `findAncestor()` utility function was removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@niegowski niegowski

Labels
package:table status:discussion type:debt This issue describes a technical debt. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:refactor This issue requires or describes a code refactoring.
Projects
None yet
Milestone
iteration 34 (v21.0.0)
Development

Successfully merging a pull request may close this issue.

#3233: Added model Position#findAncestor() and Element#findAncestor() methods ckeditor/ckeditor5
4 participants
@Reinmar @jodator @niegowski @mlewand