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

Support for styling table & table cells #3287

Closed
Reinmar opened this issue Aug 27, 2019 · 10 comments
Closed

Support for styling table & table cells #3287

Reinmar opened this issue Aug 27, 2019 · 10 comments
Assignees
Labels
Epic package:table support:2 An issue reported by a commercially licensed client. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@Reinmar
Copy link
Member

Reinmar commented Aug 27, 2019

We can add support for styling table cells.

In this case it doesn't seem reasonable to approach the problem with our usual "closed set of styles" (like image "full" and "aside" styles). Instead, we need a full flavored option.

One way to imagine it would be a button "cell properties" in a table toolbar. It would open a bigger panel where you could choose border style, border width, cell size, alignment (v and h), background color, border color.


If you'd like to see this feature implemented, please react with 👍 to this ticket.

@Reinmar
Copy link
Member Author

Reinmar commented Sep 17, 2019

Motivation

There are two distinctive requirements:

  • Support as much pasted content as possible.
    • This should unlock support for pasting nicely styled tables from Word, Excel, Google Docs and other websites.
    • This should enable more features of CKEditor 4 to migrate to CKEditor 5.
    • Note: enabling styling options in the table plugin will not automatically enable pasting from Word, Excel or Google Docs. Some things may start working but there's a big chance that we'll have to spend some additional significant time on those apps. Especially on Excel where styles are kept in a separate stylesheet.
  • Allow the users of the editor to apply simple table styles.
    • Here, the goal isn't to allow doing everything. Some styling options such as styling each border separately don't have to be provided in the MVP.

MVP

Styles conversion (step 1)

  • As generic as possible.
  • Inline styles normalization – e.g. converting border: solid 1px red to 3 separate values in the model.

UI (step 2)

  • Add:
    • "Cell properties" to the cell options dropdown (note: currently we have the "merge cell" option which we planned to rename to "cell options" at this stage).
    • "Table properties" to the table toolbar.
  • Opening the properties panel closes the standard table toolbar. The panel sticks to the table. Just like in the link feature.
  • Make the properties panel responsive (note: depending on its layout it may not require much work – if it'll be narrow by default it should just work well).
  • Keep the panel simple in the MVP – single column.

Styling options to include

Cell styling (priority: highest)

Styles applied to <td>:

  • background color (MVP)
  • border (MVP)
    • color
      • the same dropdown as for font color
    • width
      • one input (value + unit)
      • if the user typed a number, let's add 'px' automatically
    • style
      • dropdown with a preview
  • width/height
    • one input (value + unit)
    • if the user typed a number, let's add 'px' automatically
  • text alignment (vertical and horizontal)
  • cell padding
    • one input (value + unit)
    • if the user typed a number, let's add 'px' automatically

Table styling (priority: high)

Styles applied to <table> itself:

  • background color
  • width/height (may need to be downcasted to <figure>)
  • summary
  • alignment (may need to be downcasted to <figure>)

Future

  • Support for pasting from Excel.
  • Column/row styles
    • "Col/row properties" to col/row options dropdown
    • Same options as for table cells
    • For each cell in col/row apply these styles
    • We will be able to remove this once we'll have col/row selections
  • Predefined table styles (similar to image styles).
  • Integration between table cell alignment and block alignment inside.
  • Better complex inline style handling.
  • Better UI for setting borders.
  • Multi-cell selection. This will allow us to get rid of the col/row properties options.
  • Validation in the inputs.

Other considerations

Style normalization

  • border vs border-left vs border-left-style

Model:

  • table[border] = { left: {style,width,color}, top, ... } (object)

Upcasting:

  • <table style="border: solid; border-left-width:23px; border-top:solid 2px blue; border-width: 10px; border-style: solid;">
  • extractBorderStyles( tableViewElement ){ left, top, ... }

Downcasting:

  • Always output all 4 borders separately: border-left, border-top, border-bottom, border-right
  • In the future: add checking if all 4 borders are identical and then output border, or if all 4 colors are identical and then output border-color.

Enabling/disabling styling options

  • Config-based?
    • Seems to be a simpler option (less bloat).
    • Plugins to have:
      • TableCellProperties
      • TableColumnRowProperties
      • TableProperties
    • In the future: ways to reconfigure available options: config.table.supportedProperties = [ ]; (conversion and UI)
  • Plugin-based
    • Too many plugins 🤪
    • (border, size, background, ...) x (editing, UI, glue) x (cell, col&row, table)

Extending the list of options

Ideally, it should be possible to write your own plugin which will add its option to e.g. cell properties panel. It's not necessarily MVP material, but if it will turn out that this is easily doable, it'll be perfect.

Could be done like this:

editor.plugins.get( 'TableCellPropertiesUI' ).addProperty( {
    ui: sth, // some way to specify how the input will be displayed,
    onSave: sth, // a callback doing something on save,
    order: 100 // place on the list,
} );

This is oversimplified, because I don't know what's the best option to load the data into the UI and save it back. Especially in col/row cases when you have to read data from multiple items.

Perhaps, the simplest option is callback based.

const input = LabeledInputView( { label: 'Padding' } );
      
editor.plugins.get( 'TableCellPropertiesUI' ).addProperty( {
    ui: ( cell ) => {
        input.value = cell.getAttribute( 'padding' );
  
        return input;
    },
    onSave: () => {
        editor.execute( 'tableCellPadding', { value: cleanupValue( input.value ) } );
    }, 
    order: 100
} );

@Reinmar Reinmar assigned jodator and dkonopka and unassigned jodator Sep 17, 2019
@Reinmar
Copy link
Member Author

Reinmar commented Sep 17, 2019

BTW, I'm totally unsure what to do with commands here. Separate commands for all those options? There will be many of them, but perhaps there's no other way.

@Reinmar
Copy link
Member Author

Reinmar commented Sep 17, 2019

First steps:

  • UI mockups
  • conversion part of this feature

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-table Oct 9, 2019
@mlewand mlewand added this to the next milestone Oct 9, 2019
@mlewand mlewand added status:confirmed 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 Oct 21, 2019

Just to confirm - the table/tableCell/tableRow properties should be prefixed with model element name?

  • Model names for attributes on tableCell:
    • tableCellBorderWidth, tableCellBorderColor, tableCellBorderStyle,
    • tableCellBackgroundColor
    • tableCellPadding
    • tableCellVerticalAlignment
      column
    • tableColumnWidth (also should be on every cell or just in the first in column - might be hard if column is spanned on top)
  • Model for tableRow
    • tableRowHeight
  • Model for table
    • tableBorderWidth, tableBorderColor, tableBorderStyle
    • tableBackgroundColor
    • tableWidth
    • tableHeight

@Reinmar
Copy link
Member Author

Reinmar commented Oct 22, 2019

Model for tableRow

  • tableRowHeigh

I think this is still a table cell's setting. As the spec says:

For each cell in col/row apply these styles


BTW, I did you skip the alignment (of a table, but also a horizontal alignment of a table cell) on purpose when listing those attributes?

@jodator
Copy link
Contributor

jodator commented Oct 22, 2019

I've might just miss them. The other thing is that at least MS Word places the alignment on paragraphs so this works out of the box and I wonder what we should do there.

@jodator
Copy link
Contributor

jodator commented Oct 22, 2019

OK I see that prefixing those attributes doesn't make any sense - we have width on Image element already and table has headingRows not tableHadingRows.

@jodator
Copy link
Contributor

jodator commented Oct 23, 2019

OK guys, to sum up current work status I've concluded this list to track progress to finish MVP in a step 1 form - retention of as much pasted content as possible without adding table styles support in Paste From Office. I've extracted this from @Reinmar's comment with only relevant parts.

  1. Current ticket (MVP step 1) - current state
    • Check pasting content vs current model → (generate more follow ups if needed)
    • Check CKEditor 4 migration
    • Generic styles conversion with styles normalization - review idea/implementation
      • Normalizing different shorthand values: border vs border-left vs border-left-style to single normalized object (border - table[border] = { left: {style,width,color}, top, ... } )
      • Upcasting (parsing styles) - called "normalization" done on view/Element level - existing methods)
      • Reading normalized styles properties (done on view/Element level - new method) using "extracting" process to read from complex structures (border for now). You can read border-color object from border object.
      • Downcasting (done the same way as upcasting - model is setting style properties on a view/Element - different features might set similar properties and the output will always be normalized (string value) in a repeatable and predictable way in a process called "reducing" (or "inlining").
      • Review ViewConsumable support for handling style shorthands (also make it pluggable as below)
      • 50/50 In the future: add checking if all 4 borders are identical and then output border, or if all 4 colors are identical and then output border-color. Note: already done for padding and margin should be fairly easy to add it for border though.
      • Make it pluggable - right now it is a singleton inside a module :/ but is done as separate "plugins" for border, background , margin and padding
      • Check above against current features that might use style property shorthand (tests should be green 100% across all codebase)
        • Indent - yay 1 converter less
        • Font - also works
    • Plugins to have:
      • TableCellProperties
      • TableColumnRowProperties handles table cell height / width
      • TableProperties
    • Styling (one field value + unit, adding unit automatically)
      • table cell
        • background color (as backgroundColor)
        • border (color, style, width) as borderColor, borderSyle, borderWidth
        • width/height as width, height
        • vertical text alignment as verticalAlignment
        • horizontal text alignment as text-align
        • padding as padding
      • table
        • background color as backgroundColor
        • width/height (may need to be downcasted to <figure>) as width, height
        • summary - I'd go to a follow-up with this (deprecation warning from MDN)
        • alignment (may need to be downcasted to <figure>) - controviersial - do not use align attribute but margin-right=0;margin-left:auto as described on mdn. If so what with margin-right:23px;margin-left:auto - should this work with BlockIndent? (it could by consuming only auto values)
  2. Create follow-ups

ps.: this comment might be updated with more steps but ATM I think that's all what's needed for MVP step 1.

@sbilgil
Copy link

sbilgil commented Jan 22, 2020

when this can be on prod?

@jodator jodator changed the title Support for styling cells Support for styling table & table cells Jan 29, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Feb 4, 2020

when this can be on prod?

In two weeks 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic package:table support:2 An issue reported by a commercially licensed client. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests

7 participants