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

style-editor.css should not overwrite style file of block #12904

Closed
manake opened this issue Dec 15, 2018 · 2 comments
Closed

style-editor.css should not overwrite style file of block #12904

manake opened this issue Dec 15, 2018 · 2 comments
Labels
[Feature] Custom Editor Styles Functionality for adding custom editor styles Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.

Comments

@manake
Copy link

manake commented Dec 15, 2018

style-editor.css should not overwrite style file of block.

style-editor.css:

h2 { /* This is what I write in style-editor.css */
    font-size: 20px;
}

.editor-styles-wrapper h2 { /* This is how it's actually printed (overwrites all styles of blocks!) */
    font-size: 20px;
}

style.css:

.wp-block-something h2 {
    font-size: 30px;
}

The first one takes precedence because WordPress adds some additional class there (should not).

This entire "let's add .editor-styles-wrapper to all styles" idea is bad in my opinion. If you think it should stay (I think it should not) then how are we supposed to style blocks with such interference?

@swissspidy swissspidy added the [Feature] Custom Editor Styles Functionality for adding custom editor styles label Dec 15, 2018
@designsimply designsimply added [Type] Enhancement A suggestion for improvement. Needs Technical Feedback Needs testing from a developer perspective. labels Jan 9, 2019
@designsimply
Copy link
Member

Thank you for including a code example in your report!

I noticed the same thing is being discussed at #10067 (comment) and I also spotted what sounds to me like a potentially cool solution at #9008 (comment):

This surfaces some issues

sometimes these theme styles do things like body { rule } or body selector { rule }. Since I’m just adding a namespace, these styles won’t be applied.
the stylesheets are converted on the fly, which mean the relative links (images,...) in the styles won’t work at the moment.
I believe both of these issues can be fixed using:

1- More smart injecting mechanism. If we detect that the selector body in the selector, we inject the namespace after that
2- Since the CSS should be read and injected, we need to rewrite the relative urls on the fly to inject the right prefixes on the fly.

A blanket statement like saying the "let's add .editor-styles-wrapper to all styles" idea is bad doesn't take into consideration what it was trying to solve. Please have a look at #12874 (comment) for a bit more detail on that.

@manake I would love if we could consolidate this issue into one that's already open if possible so that we don't end up with multiple variations of open requests about the same problem floating around. However, your issue here is already clear, concise, and more specific compared to #10178 so I'm trying to figure out whether a solution to #10178 would potentially cover this case, that would be a reason in favor of consolidating!, or if it should remain open as a separate issue. Sometimes it's hard to strike the right balance. I'm open to either combining or staying separate for this case. What do you think?

@youknowriad
Copy link
Contributor

This entire "let's add .editor-styles-wrapper to all styles" idea is bad in my opinion. If you think it should stay (I think it should not) then how are we supposed to style blocks with such interference?

I agree with you that it has drawbacks and the example share here is a good example. The problem is we don't have any alternative at the moment. We can't use shadow DOM CSS features as it's not working across browsers, we can't use iframe as it breaks the editor in different ways (accessibility, modals...). This mechanism is the best tradeoff we have at the moment.

Ideally, we should move to shadow DOM in the future when WordPress drops support for old browsers but that's not possible at the moment. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Custom Editor Styles Functionality for adding custom editor styles Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants