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

Removing complex colspan/rowspan multi-cell selections column/rows should be improved #6406

Closed
mlewand opened this issue Mar 10, 2020 · 3 comments · Fixed by ckeditor/ckeditor5-table#286
Closed

Removing complex colspan/rowspan multi-cell selections column/rows should be improved #6406

mlewand opened this issue Mar 10, 2020 · 3 comments · Fixed by ckeditor/ckeditor5-table#286
Assignees
@jodator
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior. type:regression This issue reports a bug that was not present in the previous releases.
Milestone
iteration 31 (v19...

Comments

@mlewand
Copy link
Contributor

mlewand commented Mar 10, 2020 •
edited
Loading

📝 Provide detailed reproduction steps (if any)

After ckeditor/ckeditor5-table#261 is merged, there are some edge cases not handled well. Example screencasts:

✔️ Expected result

The table structure should be better.

❌ Actual result

Weird artifact cells added to the right side of the table.

📃 Other details

  • Browser: Any
  • OS: Any
  • CKEditor version: latest master

If you'd like to see this fixed sooner, add a 👍 reaction to this post.
@mlewand mlewand added type:bug This issue reports a buggy (incorrect) behavior. package:table labels Mar 10, 2020
@mlewand mlewand mentioned this issue Mar 10, 2020
Merged
@mlewand mlewand added this to the iteration 31 milestone Mar 16, 2020
@mlewand mlewand added the type:regression This issue reports a bug that was not present in the previous releases. label Mar 16, 2020
@mlewand
Copy link
Contributor Author

mlewand commented Mar 18, 2020 •
edited
Loading

After checking this problem we've seen that a proper fix will require a lot of effort from us to get this right.

Because of that we'd like to understand whether it's an important issue for our users, before committing to it, so be sure to add 👍 reaction to the main post to show your support.

@mlewand mlewand closed this as completed Mar 18, 2020
@mlewand mlewand reopened this Mar 18, 2020
@mlewand mlewand modified the milestones: iteration 31, nice-to-have Mar 18, 2020
@mlewand mlewand mentioned this issue Mar 18, 2020
Closed
@jodator jodator mentioned this issue Mar 24, 2020
Closed
@jodator
Copy link
Contributor

jodator commented Mar 24, 2020

Look at my comments here: #6487.

@jodator jodator self-assigned this Mar 24, 2020
@jodator jodator modified the milestones: nice-to-have, iteration 31 Mar 24, 2020
@jodator
Copy link
Contributor

jodator commented Mar 25, 2020

After checking this problem we've seen that a proper fix will require a lot of effort from us to get this right.

Nah, it was small oversight - the table map obtained from TableWalker must be recalculated on each row/column removal. Fix on the way.

@jodator jodator mentioned this issue Mar 25, 2020
Merged
mlewand added a commit to ckeditor/ckeditor5-table that referenced this issue Mar 26, 2020
@mlewand
Merge pull request #286 from ckeditor/i/6406
725a861 
Fix: Improved spanned cells handling for row and column removals. Closes ckeditor/ckeditor5#6406.
This was referenced Apr 17, 2020
Closed
Closed
This was referenced Jul 8, 2020
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jodator jodator

Labels
package:table type:bug This issue reports a buggy (incorrect) behavior. type:regression This issue reports a bug that was not present in the previous releases.
Projects
None yet
Milestone
iteration 31 (v19.0.0)
Development

Successfully merging a pull request may close this issue.

I/6406: Fix logic behind removing multiple columns or rows in a row ckeditor/ckeditor5-table
2 participants
@jodator @mlewand