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

Account for rowspan in table headingColumns #14826

Open
Witoso opened this issue Aug 21, 2023 · 2 comments
Open

Account for rowspan in table headingColumns #14826

Witoso opened this issue Aug 21, 2023 · 2 comments
Labels
package:table squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Witoso
Copy link
Member

Witoso commented Aug 21, 2023

📝 Provide a description of the improvement

Improvement from @bendemboski.

When computing headingColumns during upcasting, the logic wasn't factoring in s with rowspan attributes that "add" a heading column to subsequent rows. See tests and comments in the code for more detail.


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@Witoso Witoso added type:improvement This issue reports a possible enhancement of an existing feature. package:table squad:core Issue to be handled by the Core team. labels Aug 21, 2023
@wimleers
Copy link

wimleers commented Nov 2, 2023

Tracking this on drupal.org too: https://www.drupal.org/project/drupal/issues/3395434

@aldonace-wu aldonace-wu added the support:2 An issue reported by a commercially licensed client. label Sep 18, 2024
@Timmy-Cos
Copy link

I notice this issue seems very similar to the recently solved #17556, and can be reproduced in a similar way:

  1. Open for example https://ckeditor.com/docs/ckeditor5/latest/examples/builds-custom/full-featured-editor.html
  2. Create a 3x3 table
  3. Merge the first cells of the second and third row
  4. Set the first two columns as headers
  5. editor.setData(editor.getData()) in the console will then strip a header column each time it is run

Image

Alternatively, just open that page and run
editor.setData('<figure class="table"><table><tbody><tr><th>th cell</th><th>th cell</th><td>td cell</td></tr><tr><th rowspan="2">rowspan th cells</th><th>th cell</th><td>td cell</td></tr><tr><th>th cell</th><td>td cell</td></tr></tbody></table></figure>')
The resulting table only has one header column instead of two. It appears the issue lies with setData rather than getData in this case.

✔️ Expected result
editor.setData() returns a valid table with all the provided header columns

❌ Actual result
editor.setData() returns a table with missing header columns

Like #17556 I believe this issue is a regression introduced in 37.0.0-alpha.1. @Witoso would it be fair to relabel this as a bug? Would it be possible for the attached PR to get another look in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants