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

Block library CSS isn't transformed for more targeted specificity #11200

Open
chrisvanpatten opened this issue Oct 29, 2018 · 9 comments
Open
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Customization Issues related to Phase 2: Customization efforts [Feature] Custom Editor Styles Functionality for adding custom editor styles [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Enhancement A suggestion for improvement.

Comments

@chrisvanpatten
Copy link
Contributor

chrisvanpatten commented Oct 29, 2018

Per @youknowriad's comment in #10067 I'm splitting this issue out as a more discrete, focussed issue. I also noted this in my review of #10956.

Currently we only apply our fancy CSS transformations to styles that are…

  1. enqueued with add_editor_style('path/to/my.css')
  2. and have registered add_theme_support('editor-styles').

This is great but introduces a small problem: all other CSS, such as the block library CSS, is not transformed which can result in those stylesheets having lower specificity.

An example

A great way to see this is with our font size classes. These classes are set in the block library CSS here: https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/style.scss#L113-L133

However these styles are not run through the CSS transformer, which means they're less specific than our default editor stylesheet's p definition once it's been transformed.

That results in this:

screenshot 2018-10-29 at 08 38 17

Note how the p in the editor stylesheet is transformed to .editor-block-list__block p .editor-styles-wrapper p (my screenshot was from 4.1, not master — #10956 changed the wrapper class that's applied) and ends up being more specific than .has-medium-font-size and the font size is not applied.

Small aside: in master we always apply a style attribute on paragraphs, even when a named font size is selected, which is a bug. I've wanted to fix this but we can't really fix it until this specificity issue is resolved, because otherwise our font sizes would stop working 😁

A few quick ideas on how this could be resolved:

  1. Transform the block library stylesheet (and each block's registered stylesheet?) through the same transformation mechanism as editor-styles
  2. Move the font size (and color definitions) into the editor stylesheet that we register by default. This also encourages themes to define colors and font sizes specific to the theme, instead of just inheriting Gutenberg's defaults
  3. Simply make the CSS selectors in the block library CSS more specific. This could be accomplished by changing .has-large-font-size {} to .has-large-font-size.has-large-font-size {} which is a fun little CSS hack that makes the class more specific (without needing to actually declare the class twice in the HTML)
  4. Some other great idea that I haven't thought of :)

Tagging in @jasmussen and @youknowriad for feedback in particular!

@chrisvanpatten chrisvanpatten added Customization Issues related to Phase 2: Customization efforts [Feature] Extensibility The ability to extend blocks or the editing experience labels Oct 29, 2018
@youknowriad
Copy link
Contributor

Note how the p in the editor stylesheet is transformed to .editor-block-list__block p and ends up being more specific than .has-medium-font-size and the font size is not applied.

At the moment, it's transformed to .editor-styles-wrapper p. I think ideally we don't need to define p in the editor styles at all because it should just inherit from .editor-styles-wrapper

I think the only reason we do this for now is because we have a TinyMCE stylesheet (or Core) overriding p?

@chrisvanpatten
Copy link
Contributor Author

@youknowriad I should have noted that my screenshot was from 4.1, not master… sorry about that :) It is indeed .editor-styles-wrapper p now. I'll make a note in the original issue.

@jasmussen
Copy link
Contributor

by changing .has-large-font-size {} to .has-large-font-size.has-large-font-size {} which is a fun little CSS hack that makes the class more specific (without needing to actually declare the class twice in the HTML)

mindblown.webm

Overall though, this is a difficult challenge that I'm not sure how to best solve. Ensuring styles loaded apply to the editing canvas is one challenge, preventing CSS bleed to and from the admin (since we can't use iframes or shadow dom) is another challenge, and the third challenge is that the block editor markup rarely matches that of the front-end due to the need for it to be an actual edit view rather than a render view.

It seems like we will be doing a lot of rewriting. I do appreciate starting slowly — the twenty themes seem to already prove that you can go a long way with what we have. But as we enter phase 2 I imagine that these style rewritings will indeed need to be a bit smarter.

Outside of immediate fixes, here's a mindgame to challenge. You now have magic powers, and you can snap your fingers and make a perfect system for this. What does this system look like?

Sorry if this is slightly off topic, but I find it often helps to have a years down the line plan as well as near future plan, so a road map can be formed.

@chrisvanpatten
Copy link
Contributor Author

@jasmussen Thinking ahead I think the ideal case is that theme styles exist unaltered and untouched, while editor styles are tightly scoped (via shadow DOM, CSS-in-JS, etc). As the "editing" experience blurs with the "viewing" experience, it would mean ultimately aiming for no special editor-only stylesheet.

It's definitely a bit of a confusing challenge. My sense is that the most important thing, however, is consistency. It doesn't really matter which approach we choose as long as it's applied consistently and methodically.

Personally, I was very sceptical about the dynamic style transformation but in practice it has worked super well (major props to @youknowriad for that bit of brilliance). I think allowing any stylesheet to opt into that process — and specifically allowing block stylesheets to opt in — could ultimately even mean we need less editor-specific CSS overall. Assets could be more easily shared between the editor and front-end, and editor styles would only be needed to make compensations for editor UI chrome.

@designsimply designsimply added the [Feature] Custom Editor Styles Functionality for adding custom editor styles label Nov 16, 2018
@m-e-h m-e-h mentioned this issue Dec 17, 2018
4 tasks
@tellthemachines tellthemachines added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Apr 23, 2020
@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Aug 1, 2023
@fabiankaegy
Copy link
Member

I also keep running into this. For example today I was trying to style an input field through the style stylesheet of a custom block. The issue I ran into was that the theme baseline styles already had some rules for the input element which were overwriting any custom styles I was trying to add to the input element in the custom blocks CSS.

The reason for this is that regardless or the loading order the transformed theme stylesheet with .editor-styles-wrapper input has a higher specificity than .wp-block-namespace-example input.

I think it would be a good solution to also run any styles defined from custom blocks through the styles transformer so that every style loaded in the editor that is meant for the content gets the same treatment and therefore the specificity doesn't get impacted.

Another solution maybe would be to change the way the themes styles get transformed to be less specific using :where(.editor-styles-wrapper) [selector] {} to negate the effects the additional scoping has on the specificity.

@youknowriad
Copy link
Contributor

youknowriad commented Sep 14, 2023

Since this PR #46752 we now avoid scoping the styles within iframed editors. I understand that it's not a 100% bug fix. if you're only using api v3 blocks, I think this bug can be considered "fixed"?

@fabiankaegy
Copy link
Member

@youknowriad nice! I was unaware of that change in the iframed editor.

Sadly in that case it is now up to @Yoast to finally update their metabox Yoast/wordpress-seo#20535

Or we need to look at #52724 to use the iFrame when the metabox is turned off 😅

Thank you for that insight :)

Do you think there is still merit in updating the editor styles scoping to a less specific version using the where selector? 🤔

@youknowriad
Copy link
Contributor

Do you think there is still merit in updating the editor styles scoping to a less specific version using the where selector?

To be honest, I'm not sure. I feel like it's just a different specificity, it's still more specific for the same level of rules. So it will fix some things but not all.

@fabiankaegy
Copy link
Member

Neither am I to be completely honest here. From my testing the specificity would be identical though 🤔

selector has a specificity of 0.0.1
.editor-styles-wrapper selector has a specificity of 0.1.1
:where(.editor-styles-wrapper) selector has a specificity of 0.0.1

So I think it would cover a lot of specificity issues. But it would be kind of a breaking change because if you have come to work around the specificity you may have come to expect the issue and a fix would actually break your workaround in a way...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Customization Issues related to Phase 2: Customization efforts [Feature] Custom Editor Styles Functionality for adding custom editor styles [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

7 participants