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

Define default table styles #3160

Closed
Reinmar opened this issue Nov 19, 2017 · 15 comments · Fixed by ckeditor/ckeditor5-table#45
Closed

Define default table styles #3160

Reinmar opened this issue Nov 19, 2017 · 15 comments · Fixed by ckeditor/ckeditor5-table#45
Labels
package:table resolution:expired This issue was closed due to lack of feedback. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@Reinmar
Copy link
Member

Reinmar commented Nov 19, 2017

We'll need to define the default style for tables and, once we'll implement the "table style" feature, a couple of useful alternatives.

The https://alistapart.com/article/web-typography-tables article covers many aspects of table's typography and should help us defining the defaults.

@dkonopka
Copy link
Contributor

dkonopka commented Nov 20, 2017

How about that list of styles?

  • default table
  • striped table (changed background of :nth-child(odd) rows)
  • dark table (inverted bg/text color)
  • stretched table (reduced padding of cells)
  • and last but not sure: hovered table (?) (something like bootstrap is doing)

About caption, should we give a possibility to change a position of it? (bottom/top)

Here you will find some fast live preview @ codepen.

@Reinmar
Copy link
Member Author

Reinmar commented Nov 20, 2017

The styles should be connected to use cases not just reflect our design-freedom :). That's why I linked to that article – e.g., it mentions that striped tables are not that good. It also presents various common types of data presented in tables and conditions like tables on low-resolutions.

@Reinmar
Copy link
Member Author

Reinmar commented Nov 20, 2017

cc @fredck

@fredck
Copy link
Contributor

fredck commented Nov 23, 2017

The default style should be simple and elegant. 1px lines would do it, with some decent padding.

I'm just worried about the width of the table. For instance, making it will width can result on tables as ugly as this one:
image

I took the above screenshot from another editor. They mitigate the problem by giving to the user the ability to resize the columns of the table. Kinda bug, but with patience, you can have this:
image

If we'll not allow users to resize columns, then we may be in troubles. It may be the best option to set some min-width to the cells. But then what happens if there are many columns and the sum of their min-width is bigger than the editable?

Other styles, to be defined later. But we don't have to have many. The important is giving the ability for others to defined as many styles they wish.

@dtwist
Copy link

dtwist commented May 27, 2018

FYI, in case it was missed: Here's markup and styles to fit @oleq's default table design from an earlier (lengthy) comment of mine in ckeditor/ckeditor5-table#1:
https://codepen.io/david-twist/pen/wjyGpo?editors=0100

Also, cross-posted to ckeditor/ckeditor5-table#18 as it relates to markup in addition to styling.

@dtwist
Copy link

dtwist commented May 27, 2018

@fredck wrote:

It may be the best option to set some min-width to the cells. But then what happens if there are many columns and the sum of their min-width is bigger than the editable?

Set the min-width on the table itself. I picked 4em in the codepen prototype above, but it could be whatever you want.

@Reinmar
Copy link
Member Author

Reinmar commented May 27, 2018 via email

@dtwist
Copy link

dtwist commented May 27, 2018

Alas, the cells' min-width wins, at least in Chrome, FF and Safari.
https://codepen.io/david-twist/pen/wjyGpo?editors=0100

@dtwist
Copy link

dtwist commented May 27, 2018

If you're concerned that cell padding won't suffice, you might just choose set a min-width of 1em on cells and live with edge cases of tables with many cols in narrow editors. How often would someone create a legitimate table with anything less than a single character in each cell anyhow?

@Reinmar
Copy link
Member Author

Reinmar commented May 28, 2018

How often would someone create a legitimate table with anything less than a single character in each cell anyhow?

My biggest worry is about ensuring that a freshly created table is editable. Some editors create new tables with minimal width and I find it pretty hard myself to place the caret in the right cell, so I can only imagine how hard it is for people with motor disorder or worse sight.

You're right that cell's min-width wins. And table-layout doesn't change that. So we'd indeed have this edge case that a big table (with many cols) might not fit in a narrow content. I wonder if this is something we could live with.

@dtwist
Copy link

dtwist commented May 28, 2018

@Reinmar Agreed on all points, and I do think it's a livable trade-off, so long as you don't make the min-width too big. I would think 1em, with cell padding (which should give you approximately square cells when empty) should make it clickable enough.

The only potential issue I see with a 1em min width is that it might actually make a cell wider than desired in some cases. For example, "Y" and "N" are both narrower than 1em—imagine several Yes/No columns that (for some reason) have no headings. I'd try some experiments with min-widths closer to .6em and see if the UX is still OK at that size.

One other detail to consider is that once you implement column sizing, you'll need to apply a min-width property to individual columns (as well as the width property) if the defined width would be smaller than the built-in min-width.

oleq referenced this issue in ckeditor/ckeditor5-table Jun 12, 2018
oleq referenced this issue in ckeditor/ckeditor5-theme-lark Jun 12, 2018
…ake it possible to style tables as a content (see ckeditor/ckeditor5-table#2).
dkonopka referenced this issue in ckeditor/ckeditor5-theme-lark Jun 13, 2018
Internal: Moved table editing styles from ckeditor5-table. Decreased the specificity of the widget's nested editable styles to make it possible to style tables as a content (see ckeditor/ckeditor5-table#2). Partially fixes ckeditor/ckeditor5-table#29.
dkonopka referenced this issue in ckeditor/ckeditor5-table Jun 13, 2018
Feature: The MVP of table content styles. Moved styles related to the editing to ckeditor5-theme-lark (see #2).
@oleq oleq reopened this Jun 13, 2018
@oleq
Copy link
Member

oleq commented Jun 13, 2018

What has been merged in ckeditor/ckeditor5-table#45 is an MVP so the discussion is still open.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-table Oct 9, 2019
@mlewand mlewand added this to the backlog milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". package:table labels Oct 9, 2019
@pomek pomek removed this from the backlog milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added resolution:expired This issue was closed due to lack of feedback. and removed status:stale labels Jan 15, 2024
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table resolution:expired This issue was closed due to lack of feedback. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants