Unify color definition type among components.

[tomalec]

📝 Provide a description of the improvement

Currently, the number of components uses a color definition object. Unfortunately, there are two slightly different types, which introduces confusion and cognitive burden for developers, requires more documentation, and makes code complicated and less performance (due to translations needed).

Current types

TableColorConfig

• docs
• source

Array.<String | Object>

const colors = [
    {
        color: 'hsl(0, 0%, 60%)',
        label: 'Grey'
    },
    'hsl(0, 0%, 80%)',
    {
        color: 'hsl(0, 0%, 90%)',
        label: 'Light grey'
    },
    {
        color: 'hsl(0, 0%, 100%)',
        label: 'White',
        hasBorder: true
    },
    '#FF0000'
]

ColorDefinition

• docs
• source

Object

{
    color: 'hsl(0, 0%, 75%)',
    label: 'Light Grey',
    options: {
        hasBorder: true
    }
}

undocumented, unnamed structure - let's call it ColorValue - for the sake of this post.

• source

Object

{
    value: 'hsl(0, 0%, 75%)',
    label: 'Light Grey',
    hasBorder: true
}

As you can see they are similar, but slightly off. I don't see any particular reason why is it so.

Who uses what?

... that was the tricky part to me, but AFAI have found:

	ColorDefinition	TableColorConfig[~]	TableColorConfig	ColorValue	translates?
ColorGridView	as a configuration parameter	even though it's not documented as such, it uses this structure to create, ColorTileView		as an execute event data	ColorDefinition -> TableColorConfig[~] - when setting up ColorTileViews. ColorDefinition -> ColorValue - when firing execute event.
ColorTileView		Not documented as such, but uses TableColorConfig item-like structure as its properties.			-
ColorInputView	as configuration parameter				-
	internally, to setup ColorGridView, to match values from InputTextView
ColorTableView	as a configuration parameter	even though it's not documented as such, it uses this structure to create, ColorTileView and ColorGridView			ColorDefinition -> TableColorConfig[~] - when setting up ColorTileViews and ColorGridViews. ColorDefinition -> ColorValue - when firing execute event.
TableCellPropertiesView , TablePropertiesView	internally, to setup ColorGridView, to match values from InputTextView		as a configuration parameter		TableColorConfig - > Array.<ColorDefinition> - to create ColorInputViews using dedicated util
ColorTableView	as a configuration parameter

An then comes normalization etc.

Problems

1. As already stated, lack of consistency - all the drawback it brings was already stated (DevX, performance, etc.)

   Fixing that would require major breaking change as may affect users configuration.

2. TableColorConfig - defines an Array of Objects of a specific kind without a name. There is nothing specific in the Array itself. I'd rather create a typedef for the object, then use Array.<SpecificObject> and SpecificObject if needed. With the current setup, I lacked a vocabulary, even to describe the original issue.

   I believe we do not need to introduce a breaking change, to solve that, and simplify the docs, by renaming things.

3. .color - property is misleading to me. If the object type we are talking about is to describe a color to be picked, then we do not talk about "color's color", I would rather say "color's value" or "color's code".

   Such change would require major breaking change, as would affect users' configs. Also, it does not bring much value from the functional perspective (for example improving performance, reducing code bloat). However, if we are about to make a heavy change to meet 1., we could consider such clarification as well.

---

Proposal

I propose for all classes to use the following object

ColorConfig

Object

{
    value: 'hsl(0, 0%, 75%)', // we can go with old `color` if it's too opinionated`
    label: 'Light Grey',
    hasBorder: true
}

The classes/functions that requires an Array, could simply use Array.<ColorConfig> or Array.<ColorConfig | String>
I see no reason why hasBorder requires a dedicated options namespace. Using value property would also simplify creation, of ...Tiles and forwarding the color value from various inputs.

//cc @Reinmar

📃 Other details

• Browser: n/a
• OS: n/a
• CKEditor version: n/a
• Installed CKEditor plugins: n/a

---

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

[tomalec]

I found out, that we have, kind-of fourth type.
Something used in normalizeColorOptions util that per description takes and returns ColorDefinition, but in fact is using something TableColorConfig[~]-like, but with additional view property, and with model instead of color.

https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-ui/src/colorgrid/utils.js#L66-L82

{
	model: color.color.replace( / /g, '' ),
	label: color.label || color.color,
	hasBorder: false,
	view: {
		name: 'span',
		styles: {
			color
		}
	}
}

[oleq]

Feels like DUP of #6237.

[CKEditorBot]

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]

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).