Move hard coded colors from components to global _colors.css

[adamerose]

This posed a problem for me while I was trying to implement my own dark theme by overriding colors in _colors.css as described by the documentation. I had this light background on unfocussed selected text and had to go through the components css files to find where this was defined and override it myself.

[mlewand]

@oleq could you have a look at it?

[oleq]

@adamerose I'm not sure why you had to override colors in _colors.css when you can do the same in the styles of your application, for instance by adding:

.ck.ck-editor__editable_inline.ck-blurred ::selection {
	background: red;
}

[adamerose]

you can do the same in the styles of your application, for instance by adding...

This seems like a workaround for the issue I described, whereas my PR fixes the underlying issue. I think users should be able make color themes following the theming documentation without needing to manually style individual elements that don't follow the color theme CSS variables.

@adamerose I'm not sure why you had to override colors in _colors.css

That is the process for color theming described in the documentation, it links to that file.

Check out the color sheet for a full list of customizable colors.

---

While I was searching through GitHub issues and code trying to figure out how to properly make a consistent CKEditor5 color theme I found an old comment about this:

do we want to aggregate all color custom properties in _colors.css? Or maybe we should focus on modularity and move all non-generic colors to respective CSS files?

My opinion would be that non-generic colors should not exist, they all should be derived from theme-based CSS variables that users can adjust so it's easy to modify themes. Otherwise you end up having to manually override element styles like you just said so you don't get a light grey highlight color on light grey text and other inconsistencies. I made a more general issue about this topic #9734.

[oleq]

@adamerose Nowhere in the documentation is recommended to override built-in CSS files before compilation. As a matter of fact, quite the contrary it says:

The file containing custom variables will be named custom.css and it will look as below:

We recommend you store custom property overrides in a dedicated CSS file in your project (outside CKEditor 5 building process). 

Overrides stored in this file (be it custom.css) will take precedence over all sorts of custom properties in the CKEditor 5 project, regardless of their original location (ckeditor5-theme-lark/theme/ckeditor5-ui/components/dropdown/splitbutton.css, ckeditor5-theme-lark/theme/ckeditor5-ui/components/editorui/editorui.css, ckeditor5-theme-lark/theme/ckeditor5-ui/globals/_colors.css, etc.). 

It does not really matter if --ck-color-split-button-hover-background is stored in splitbutton.css or _colors.css. In the end, this property will land in the global :root {} scope in DOM, and your custom CSS file will override it.

[adamerose]

@adamerose Nowhere in the documentation is recommended to override built-in CSS files before compilation.

Yeah I didn't mean before compilation, I'm just overriding them by setting my own values for those variables. And I do this in my own CSS file like you said. I used the word "override" because that's what the docs call it. eg. /* Overrides the border radius setting in the theme. */

It does not really matter if --ck-color-split-button-hover-background is stored in splitbutton.css or _colors.css. In the end, this property will land in the global :root {} scope in DOM, and your custom CSS file will override it.

It matters because the user needs to know that variable exists. The docs say "Check out the color sheet for a full list of customizable colors." which indeed seems to contain almost every color variable. The only color variables that aren't in _colors.css were the 3 variables I moved in this PR. It looks like --ck-color-split-button-hover-background and --ck-color-editable-blur-selection are in the docs but not _colors.css, and the variable that broke my theme --ck-color-editable-blur-selection isn't in either. The only way I found it was seeing which variable in Chrome devtools was breaking my theme and doing a ctrl+F for it across the repo

So I guess one solution is to add --ck-color-editable-blur-selection to the docs, but just adding the 3 scattered colors to _colors.css to be with the rest seems better to me

[adamerose]

Bumping this. I still believe all colors in the CKE5 UI should be controlled by variables stored in _colors.css so users can make complete themes using just those variables.

I've just added the source editor feature to my build and am again using Chrome devtools to inspect it and doing manual overrides on textarea background-color because its color doesn't appear to be a variable in _colors.css or anywhere else from what I can see.

It does not really matter if --ck-color-split-button-hover-background is stored in splitbutton.css or _colors.css

I just want to reiterate... yes users can just manually override everything, but if you just keep all color variables in _colors.css users will be able to make themes without having to go through this iteration loop:

1. Set theme colors in their custom css file, using _colors.css as a reference for what theme color names should be set.
2. Run their build and click around the UI manually looking for edge cases that are colored incorrectly (for example, the text highlighting that I mention in the OP) because they aren't styled using the variables in _colors.css.
3. Clone the CKEditor repo and search through the code to find where that element is actually styled, if at all.
4. Add a manual override to the custom CSS to handle that edge case.
5. Repeat 1-4 every time a new feature is turned on.

[CKEditorBot]

There has been no activity on this PR 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 contribution, leave a comment or reaction under this PR.

[CKEditorBot]

We've closed your PR due to inactivity over the last year. While time has passed, the core of your contribution might still be relevant. If you're able, consider reopening a similar PR.