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

Editor stylesheet CSS specificity issues #10067

Closed
brettsmason opened this issue Sep 20, 2018 · 28 comments
Closed

Editor stylesheet CSS specificity issues #10067

brettsmason opened this issue Sep 20, 2018 · 28 comments
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 Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@brettsmason
Copy link

Describe the bug
Editor stylesheet CSS specificity issues.

To Reproduce

  1. Add editor stylesheet as per the documentation.
  2. Add a basic heading tag eg h2
  3. An example of the generated style I'm using which works fine:
.editor-block-list__block h2 {
   font-size: 2rem;
   line-height: 1.16666667;
}
  1. The line-height rule is being overruled by a few rules in core with higher specificity:
.wp-block-heading h1.editor-rich-text__tinymce, .wp-block-heading h2.editor-rich-text__tinymce, .wp-block-heading h3.editor-rich-text__tinymce {
   line-height: 1.4;
}

.editor-rich-text__tinymce.mce-content-body {
    line-height: 1.8;
}

Expected behavior
Core styles should have less specificity to allow for an easier time adding a comprehensive custom editor stylesheet.

Could the above rules not be written like:

.wp-block-heading h1, .wp-block-heading h2, .wp-block-heading h3 {
   line-height: 1.4;
}

.editor-rich-text__tinymce {
    line-height: 1.8;
}

This allows the editor stylesheet to correctly trump the core styles.
This is only a brief example, but I'm sure there are others. I'd be happy to list all problematic styles that I find if it helps.

Additional context
Gutenberg 3.8 and WordPress 4.9.8

@designsimply designsimply added the Needs Testing Needs further testing to be confirmed. label Sep 20, 2018
@designsimply
Copy link
Member

Tested and confirmed with the following steps:

  1. Add the following to the theme's functions.php file:
add_theme_support( 'editor-styles' );
add_editor_style( 'editor-style.css' );
  1. Add this to editor-style.css in the theme folder:
h2 {
   color: salmon;
   font-size: 4rem;
   line-height: 3;
}

Result: I expected the line height to be 3 (exaggerated for visibility) and it wasn't.

screen shot 2018-09-20 at 7 52 19 am
Seen at http://alittletestblog.com/wp-admin/post.php?post=15045&action=edit running WordPress 4.9.8 and Gutenberg 3.9.0-rc.2 using Firefox 62.0 on macOS 10.13.6.

@designsimply designsimply added [Feature] Extensibility The ability to extend blocks or the editing experience Customization Issues related to Phase 2: Customization efforts and removed Needs Testing Needs further testing to be confirmed. [Feature] Extensibility The ability to extend blocks or the editing experience labels Sep 20, 2018
@designsimply
Copy link
Member

This issue is great for a specific case and to see if we can improve something by continuing the work started in #9008. There is another issue #9534 which asks for feedback related to styling, and I thought I would point you there in case you would like to contribute to that conversation—which is more about critical thinking on the approach for styling on a more general level.

I'd be happy to list all problematic styles that I find if it helps.

Thanks! Let's ask about this. @youknowriad, would it help to have more examples here?

@youknowriad
Copy link
Contributor

Yes, this is very useful and I expect we polish this over time. Most of these styles can be resolvers by changing the way they are being applied but we need to do these changes with care. A list of specificity issues would be great to have.

@jasmussen might also be interested in this.

@brettsmason
Copy link
Author

The selectors I've noticed so far are:

body.gutenberg-editor-page
.editor-post-title__block .editor-post-title__input
.editor-rich-text__tinymce.mce-content-body
.wp-block-heading h1.editor-rich-text__tinymce, .wp-block-heading h2.editor-rich-text__tinymce, .wp-block-heading h3.editor-rich-text__tinymce

All of the above seem overly specific and hard to overwrite.

On the other hand, trying to import custom form styles into the editor have been a challenge as they can easily overwrite the inserter button styles for example. In this instance, I think the styles should have higher specificity, as I can't think of a situation where these should be trumped.


Possibly not related and let me know if you would like a new issue, but noticed another bug in the editor.
Using this:

body {
    background-color: $color-background;
}

Produces this which styles the blocks background, rather than the editor background like I expected:

.editor-block-list__block {
    background-color: #544882;
}

It could be the case that I'm using the new feature incorrectly.

@m-e-h
Copy link
Member

m-e-h commented Sep 25, 2018

Another unnecessarily specific block is the Code block:

in block-library/edit-blocks.css we have:

.wp-block-code .editor-plain-text {
    font-family: Menlo,Consolas,monaco,monospace;
    font-size: 14px;
    color: #23282d;
}

in editor/style.css we have:

.gutenberg .editor-plain-text {
    border: none;
    box-shadow: none;
    color: inherit;
    font-family: inherit;
    font-size: inherit;
    line-height: inherit;
    margin: 0;
    padding: 0;
    width: 100%;
}

Since .editor-plain-text is already set to inherit color, font-family, and font-size, we could probably just add the styles to .wp-block-code rather than .wp-block-code .editor-plain-text

@mtias mtias added Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Enhancement A suggestion for improvement. labels Oct 7, 2018
@m-e-h
Copy link
Member

m-e-h commented Oct 8, 2018

Fixed in #10358

.editor-block-list__block ul:not(.wp-block-gallery) {
    list-style-type: disc;
}

A little less specific would be:

.editor-block-list__block ul {
    list-style-type: disc;
}

This already exists:

.editor-block-list__block .wp-block-gallery {
    list-style-type: none;
}

@chrisvanpatten
Copy link
Member

@m-e-h That one in particular has been fixed as of this morning in #10358.

@celloexpressions
Copy link

This should be considered a blocker for API freeze and 5.0. Theme authors can't effectively implement block and editor style support until CSS specificity requirements can be finalized. This also impacts bundled theme updates, which will set the precedent for theme best practices.

@nickcernis
Copy link
Contributor

Specificity issues are also blocking work we're doing to make existing themes Gutenberg-friendly.

We currently have to add more specific rules for editor widths in order to override default editor block styling when using add_editor_style( 'editor-style.css' );.

For example, the CSS in the Gutenberg theme support handbook to change editor width looks like this:

/* Main column width */
body.gutenberg-editor-page .editor-post-title__block,
body.gutenberg-editor-page .editor-default-block-appender,
body.gutenberg-editor-page .editor-block-list__block {
    max-width: 720px;
}

/* Width of "wide" blocks */
body.gutenberg-editor-page .editor-block-list__block[data-align="wide"] {
    max-width: 1080px;
}

/* Width of "full-wide" blocks */
body.gutenberg-editor-page .editor-block-list__block[data-align="full"] {
    max-width: none;
}

But this has no effect when using add_editor_style( 'editor-style.css' );, as noted in this closed issue.

We are currently using more specific rules as a workaround (with an extra .wp-admin class):

body.wp-admin.gutenberg-editor-page .editor-block-list__block,
body.wp-admin.gutenberg-editor-page .editor-default-block-appender,
body.wp-admin.gutenberg-editor-page .editor-post-title__block {
	max-width: 732px;
}

@youknowriad
Copy link
Contributor

youknowriad commented Oct 15, 2018

When using add_editor_style( 'editor-style.css' ); this should be enough:

.wp-block {
	width: 732px;
}

@nickcernis
Copy link
Contributor

nickcernis commented Oct 16, 2018

Thanks, @youknowriad. It looks like additional hacks are also required beyond that to handle block alignment and full/wide cases:

https://github.com/WordPress/twentynineteen/blob/de75cbdecdfc95a3309e3420acbaabc4c981e574/style-editor.css#L4-L6

Once a method is finalised it would be good to update the handbook at https://wordpress.org/gutenberg/handbook/extensibility/theme-support/#changing-the-width-of-the-editor, which does not currently reflect the .wp-block width method you outline or the way Twenty Nineteen is handling block width with data-align.

@chrisvanpatten
Copy link
Member

body mapping to .editor-block-list__block is definitely an issue; it basically renders the cascade useless for any properties you set inside body. I ran into this in #10562, and had to put an override in so nested blocks inherited color from the container block.

I think it would make more sense for body to map to .edit-post-visual-editor or .editor-writing-flow, and have .wp-block map to .editor-block-list__block (or something like that).

@youknowriad
Copy link
Contributor

Agreed we should try to refactor the editor styles to apply to the editor's container instead of the block's wrapper.

The downside though is that it can affect the UI bits (toolbars, movers...) so we need to be careful there and check for feasibility first.

@m-e-h
Copy link
Member

m-e-h commented Oct 16, 2018

The UI bits are definitely the issue when scoping beyond the block itself. #10178

But .editor-block-list__block already wraps the UI that would be most problematic. Things like .editor-block-mover and .editor-block-contextual-toolbar. So moving it to the editor's container shouldn't change too much.

But I think globalizing the CSS scope, even more, may be a move in the wrong direction.
I actually created my own PostCSS plugin to narrow the scope more by wrapping my styles in [data-block] or .editor-block-list__block-edit.

@chrisvanpatten
Copy link
Member

chrisvanpatten commented Oct 17, 2018

@m-e-h actually raises a good point, and also we know that the styles are reasonably well scoped because of nested blocks. Otherwise, the styles mapped to editor-block-list__block would cascade down into child block UI, but they currently don't.

I think simply changing body to map to edit-post-visual-editor or similar would likely work more smoothly than we think!

@chrisvanpatten
Copy link
Member

Encountered another case where there are specificity issues:

web_inspector_ dev_tomodomo_co _post_php

The font size classes are being overridden by the core editor style classes.

@m-e-h
Copy link
Member

m-e-h commented Oct 19, 2018

I've run into several cases like that, in the editor, as well @chrisvanpatten .

I've found that p and pretty much anything that uses 16px/1rem should almost always be set to font-size: inherit if they need to be set at all.

@youknowriad
Copy link
Contributor

I'm planning to try this in the days and see if it's viable

I think simply changing body to map to edit-post-visual-editor or similar would likely work more smoothly than we think!

@kjellr
Copy link
Contributor

kjellr commented Oct 23, 2018

Leaving a few notes over here from Twenty Nineteen land... There are a few things we've been trying to customize that've been difficult with the current stylesheet setup:

Since those last 3 bullets are outside the scope of the usual injected CSS file, we've added a second, un-modified CSS file to house those styles. Calling two stylesheets for the editor styles isn't ideal, and we hope to bring those into one, once it's possible to do so.

@youknowriad
Copy link
Contributor

Thanks all for your input. I opened #10956 which changes the wrapper where we apply the editor styles. Hopefully, this solves a lot of these issues. It doesn't solve everything though.

I'd appreciate some help testing the PR and once we get it, let's try to split this massive issue into discrete issues to ease tracking and solving those.

Thanks

@designsimply
Copy link
Member

Re-tested with master at 70c80680b and found that font-size and line-height are still overridden (screenshot) for editor styles added using the steps noted at #10067 (comment). @youknowriad I noticed #10956 was merged (and should be included for my test just now), does that mean this issue should remain open or should it have been solved in #10956 ?

@kjellr
Copy link
Contributor

kjellr commented Feb 13, 2019

.editor-rich-text__tinymce.mce-content-body {
    line-height: 1.8;
}

^ Just noting that this line height issue should be gone now, as we're no longer using the mce-content-body class. #13867 is open to remove that style entirely.

@youknowriad
Copy link
Contributor

Noting that it would be good to maybe close this issue right now and do smaller ones as we made a lot of improvements recently for these styling issues?

@jasmussen
Copy link
Contributor

Yep, I think that would be good. Even though we already have many tickets in the repo, I would rather have 10 separate, small tickets, than one meta ticket.

@samikeijonen
Copy link
Contributor

Can you point all the issues related to this? I'd like to take a closer look.

@jasmussen
Copy link
Contributor

No, that's the point, these issues would need to be created, one ticket for each issue that hasn't been addressed.

@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Apr 10, 2019
@tellthemachines
Copy link
Contributor

Is there anything still left to address on this ticket? I notice that the issues described on the description have been fixed, as well as the ones mentioned on this comment, this comment, this comment and this comment.

We should, in any case, create new tickets for any remaining issues before we close this one. But my question here is if there are any remaining issues, among the ones mentioned on this ticket, to be addressed.

@tellthemachines tellthemachines added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Apr 23, 2020
@jasmussen
Copy link
Contributor

@tellthemachines I would tend to agree, good assessment. It's very easy to reopen this ticket if we close it in error, people can comment and ask it reopened and we will. So I'll go ahead and close it and direct to [Feature] Custom Editor Styles Functionality for adding custom editor styles , which is usually the best source for specific editor specificity issues. Thanks everyone!

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 Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests