Move the default heading styles from editor styles to theme.scss #29588
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
[Type] Enhancement
|
Size Change: -143 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
Contributor
|
Love this. I'd love @kjellr thoughts on it, but I think it's time.
|
Contributor
Author
|
I changed my mind :) and found a better fix IMO. This fix uses the "revert" value to override the wp-admin styles for headings and "revert" to the browser defaults for these styles unless a theme provides its own editor styles which are going to be higher specificity. I think this is the ultimate fix :P I'm tempted to use that for all the other things in the editor-styles.scss file (I'll do separately if this works well). Revert is supported in 87% of the browsers and probably something like 95% of the browsers we actually support. I think it's fine because what will happen if the browser doesn't support it is just that the editor won't look exactly like the frontend by default, so I think it's a good progressive enhancements strategy. I'd love some testing in various themes for this PR. |
Contributor
|
Yeah my concern with the previous suggestion is that it wouldn't work if the theme opted out of block styles, because of this issue: #29252 |
Contributor
Author
|
If this proves to be good solution I'll update the list PR to do the same. |
Contributor
|
In "empty theme" the margins look different. Editor: Front end: I suppose this is because somewhere in the context a different base is being set, but I couldn't find where. |
Contributor
Author
I think it might be because of the default block margins applied to all blocks in the editor but I'll confirm. |
Contributor
|
The margins are exactly the same value in ems in the inspector, it's just that the browser is displaying an em at different sizes. |
Contributor
|
Actually it's not the margins at all it's the line-height, which is coming from this line: |
Contributor
|
Changing to |
Contributor
Author
yes, the problem with that thing is that we will not be able to "revert it" because the "body" styles from wp-admin will still apply since it's a parent for headings, so it will be inherited, what we need to do there is to actually set a default line height that is equal to the browser one. |
Contributor
Author
yes, that's a good fix but I'll leave all the "body" things for later (to be edited in one go) |
Contributor
Author
|
@scruffian #29590 this PR builds on top of the current one and applies bigger changes to the wrapper, lists and paragraphs. I mostly wanted to "validate" the approach and probably land it in smaller pieces starting with the current one for headings. |
Contributor
Author
|
Ok let's close in favor of #29590 which is more ambitious but better. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this PR is always the same of #29517 and #29556: Make frontend and backend look similar by default.
The way we achieve that is that we should empty the "editor-styles.scss" because this file applies styles the backend that the them don't necessarily have. So our options are something like:
1- Remove these styles.
2- Apply these styles to both frontend and backend by default
3- Only apply these styles in both frontend and backend if the theme opt-into the opinionated theme styles.
The current implementation in this PR does 3 (moves the default heading styles from editor-styles to theme.scss), This can be impactful for themes with the opt-in true. I'm not sure yet if this is the best approach or we should go with "1" instead. Let's test in some themes and gather opinions.
Edit: Found a better solution explained here #29588 (comment)