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

Apply front-end theme CSS to CKEditor instances #4

Closed
quicksketch opened this issue Jun 22, 2023 · 8 comments · Fixed by #120
Closed

Apply front-end theme CSS to CKEditor instances #4

quicksketch opened this issue Jun 22, 2023 · 8 comments · Fixed by #120

Comments

@quicksketch
Copy link
Member

Now that CKEditor now longer uses an iframe, we'll need to rework front-end CSS and the way it applies to CKEditor instances.

@quicksketch
Copy link
Member Author

Here are some related issues and background research from Drupal:

Overall the solution for Drupal has been to load the CSS file as part of the editor parent page, but include a .ck-content wrapping selector around everything.

Multiple users have been confused that the CKEditor 5 (and 4) module load the front-end theme's CSS, not the admin theme. Maybe we should make it so that if an admin theme provides a CKEditor stylesheet, but the front-end theme does not, it loads the admin theme CSS file?

@indigoxela
Copy link
Member

It's pretty tricky... I don't think, we really can avoid confusion.

@wimleers
Copy link

wimleers commented Dec 1, 2023

Overall the solution for Drupal has been to load the CSS file as part of the editor parent page, but include a .ck-content wrapping selector around everything.

FYI: this was recommended by the CKEditor team.

Multiple users have been confused that the CKEditor 5 (and 4) module load the front-end theme's CSS, not the admin theme.

FASCINATING! 😄 Most people on the Drupal side who complain about any aspect of this, are almost viscerally angry that the preview isn't a perfect "WYSIWYG" 😅

@quicksketch
Copy link
Member Author

I filed a PR at #120 that updates our implementation to add front-end CSS files on pages that contain a CKEditor instance. Note that because the CSS is applied across the entire page, the $format parameter in hook_ckeditor5_css_alter() was dropped.

Test coverage here in contrib is a little tricky. We could add a test theme within this module but that seems a little strange.

To test manually:

  • Make a new file at core/themes/basis/css/ckeditor-styles.css with the following contents:
.ck-content {
  background-color: lightblue !important;
}
  • Edit core/themes/basis/basis.info and add the following:
ckeditor5_stylesheets[] = css/ckeditor5-styles.css
  • Clear caches.
  • Visit /node/add/post and use a CKEditor 5 text format.
  • The CSS file should be present on the page and affect the contents of the editor.
    image

@indigoxela
Copy link
Member

Will play with that soon with some of my contrib themes. 👍

@indigoxela
Copy link
Member

indigoxela commented Dec 14, 2023

As expected, quite a mess.

Even if the setting "Use the administration theme when editing or creating content" is off, so the frontend theme is used for content editing, cke variables get in the way, example:

var(--ck-color-base-background)
var(--ck-color-text)

OK, just a few lines to add to existing CSS to fix that (override the vars).

When "Use the administration theme when editing or creating content" is turned on, so the admin theme is used for content creation, things get really messy.
Not only CKEditor variables have to be considered, but also the admin theme styles need to get overridden. So it's not only to clone an existing CSS file and prepend ".ck-content" to each and every rule, some opinionated CSS rules from Seven (tables! ...) also have to get overridden. 😬 That's quite some work. Expected, though.

Fonts are tricky, too. Worst case, both fonts, for frontend and admin theme have to get loaded.

I didn't even start with dynamic stuff like Color module support, dark mode...

Anyway, API-wise - ckeditor5_stylesheets[] = css/ckeditor5-styles.css works as expected.

Didn't test the alter hook, yet.

@quicksketch
Copy link
Member Author

Yeah, same problems Drupal has been seeing since CKE5 too. The administrative styles all apply to the not-so-much WYSIWYG editor. And yes, the selectors all have to be super-specific. And there's a huge amount of bleed-through.

OTOH, there may be some slight benefit that themes that don't provide ckeditor5_stylesheets[] in their .info file at all get at least decent styling provided by the admin theme. CKE4's lack of any useful styling on blockquotes, monospace, links, etc often created a quite-ugly default editor.

@quicksketch
Copy link
Member Author

I tried to write test coverage for a theme adding a ckeditor5_stylesheets[] line, but Backdrop does not allow a theme to be placed within the modules directory (which makes sense). So we can add test coverage after this is in core using the themes from SimpleTest, but not while this module is in contrib.

With the functionality of ckeditor5_stylesheets[] confirmed, I think we can proceed with this set of changes. The short-comings are inherit to CKE5, not this implementation of the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants