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

Table styles output: collapse or to not collapse? #5575

Closed
jodator opened this issue Oct 11, 2019 · 8 comments
Closed

Table styles output: collapse or to not collapse? #5575

jodator opened this issue Oct 11, 2019 · 8 comments
Assignees
Labels
package:table status:discussion type:question This issue asks a question (how to...).

Comments

@jodator
Copy link
Contributor

jodator commented Oct 11, 2019

📝 Provide a description of a problem

We want to introduce styling tables in the #3287 which will include styling table cells' borders. Unfortunately, HTML/CSS does not have the best control over this when you wish to have collapsed borders.

With boder-collapse: collapse:

  • tables are generally prettier
  • you have 1px borders between cells
  • to style visually borders on the cell you have to style border-bottom of a cell above and border-right of a cell on the left
  • if border width is bigger then default (adjacents' cells) it wins

With border-collapse: separate:

  • tables have a retro look
  • minimum width of a border (with cell spacing set to 0) is 2px (1px from adjacent cells' borders)
  • styling visually borders is pretty straightforward: you setting collor only for the selected cell

To visuallise this:

setting |
same width: 1px | blue border: 2px
---|---|---
border-collapse:collapse | Selection_017 | Selection_022
border-collapse:separate | Selection_019 | Selection_020
border-collapse:separate spacing:0 | Selection_018 | Selection_021

The problem is with

  • what should be the default style
  • how to style the content (should we do the trick with coloring adjacent bordres when in collapsed mode?)

Are there any industry standards for sytling table cells' borders? The content paster from other sources would usually have border collapsed and styled borders of adjacent cells and only styled only bottom/right in styled cell (Google docs does both, tyles top,right,bottom,left of the current and bottom/right of adjacent top/left cell).

ps.: disabling border on the table for collapsed borders renders also the left border of first cell in a row:
Selection_023 Selection_024

@jodator jodator added type:question This issue asks a question (how to...). status:discussion labels Oct 11, 2019
@mlewand mlewand added this to the iteration 28 milestone Nov 4, 2019
@Reinmar
Copy link
Member

Reinmar commented Nov 15, 2019

All HTML-oriented editors which support styling table cell borders (this will be just two... CKEditor 4 and TinyMCE) that I checked currently don't handle this well. If you try to change the borders of the middle cell to blue, only this <td>'s style will be changed leading to a broken visual effect.

At the same time, I can't remember people going mad about this. Why? Because by default in CKEditor 4 we have separate borders. Which, I agree, looks heavy and old. And just unnecessary – table is a table, a grid. Not a "set of cells" like the default browsers styles show it. That's no option for us to have separate styles by default.

Therefore, what I'd do:

  1. Go with collapsed borders and this naive border setting behavior.
    • It will work well for people who don't allow styling table cells (it may be most of them).
    • It will work well for people who only paste tables (e.g. from Word) because pasted content is styled correctly – this is – Word or Google Docs set the border-bottom and border-right styles of adjacent cells.
    • We can document that if you plan to enable the TableCellBorder plugin, you should rather consider adding .ck-content table { border-collapse: separate } to your stylesheet due to this issue.
  2. One day, perhaps we can implement smart styling where in the model we'd have what we have now (border is set only for the cell you modified) but the view would be different. After all, it's the DOM's problem so the view is the right place to accommodate a fix. Or... perhaps even the DOMConverter (although, that'd be perhaps too much).

@Reinmar
Copy link
Member

Reinmar commented Nov 15, 2019

I'm moving this to "In progress". If you agree with me @jodator and @oleq, let's close this as done.

@oleq
Copy link
Member

oleq commented Nov 18, 2019

table is a table, a grid. Not a "set of cells" like the default browsers styles show it. That's no option for us to have separate styles by default.

I agree, no one uses tables with separate cell borders (border-collapse:separate) these days.

Go with collapsed borders and this naive border setting behavior.

I'm also for whatever is most compatible with the pasted content from GDocs/MS Word.

@Reinmar
Copy link
Member

Reinmar commented Nov 19, 2019

I'm also for whatever is most compatible with the pasted content from GDocs/MS Word.

That's tricky. GDocs/MSWord do actually set the adjacent cells' borders to have them rendered correctly in HTML. We can allow pasting that, but I don't think we should work on making our UI doing the same thing as it increases the complexity by... a lot :D

@oleq
Copy link
Member

oleq commented Nov 19, 2019

@Reinmar

So we're left with border-collapse:separate; spacing:0 only? Because I'm a bit confused about what we can do to be Word–compatible and have a sane experience when styling tables from scratch.

@Reinmar Reinmar modified the milestones: iteration 28, iteration 29 Nov 25, 2019
@panr
Copy link
Contributor

panr commented Dec 10, 2019

https://codesandbox.io/s/objective-haibt-ibx59

This is my POC of how we can replace borders for box-shadow hacks (just in case we need this in the future).

@jodator
Copy link
Contributor Author

jodator commented Dec 10, 2019

What worries me with anything different from border: styles is that such content will not be transfered to other editors.

@Reinmar
Copy link
Member

Reinmar commented Dec 16, 2019

Final decision: #5575 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table status:discussion type:question This issue asks a question (how to...).
Projects
None yet
Development

No branches or pull requests

5 participants