-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add html
to reset styles for the .editor-styles-wrapper
container
#62350
Add html
to reset styles for the .editor-styles-wrapper
container
#62350
Conversation
Also removes comment about not needing it within an iframe.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @peXed. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
// an explicit white background color, assuming a white editing canvas. | ||
// So to match browser defaults, we provide a white default here as well. | ||
background: #fff; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The admin styles seem to specifically target the body
. Could we move these particular styles out of the here(.editor-styles-wrapper)
block and under body
, maybe with a link to which styles are being reset?
Also when did this break? Where you able to pinpoint the specific commit or release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cc @aaronrobertshaw in case it rings a bell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector needs at least 0-0-1
specificity to override body
, which it had before removing html
in 46752.
I made #61959 to restore that selector exactly as it was for the whole set, and Aaron already suggested to isolate the change to only the first selector of the stylesheet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, we should probably change the selector to:
div:where(.editor-styles-wrapper),
body:where(.editor-styles-wrapper)
So that it overrides the admin styles for both an iframed and non-iframe editor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restoring is what I tried first :) #61959
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, so we only want it to affect the top level styles. I guess I meant, why not take out that section and do it in a separate block? I find that a bit more readable than html &
.
…62350) Also removes comment about not needing it within an iframe. Unlinked contributors: peXed. Co-authored-by: sabernhardt <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: karmatosed <[email protected]>
I just cherry-picked this PR to the wp/6.6-rc-1 branch to get it included in the next release: 9f0380f |
…62350) Also removes comment about not needing it within an iframe. Unlinked contributors: peXed. Co-authored-by: sabernhardt <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: karmatosed <[email protected]>
Fixes: #57176
(similar to option in #61959, but only increasing the specificity for one selector instead of the whole group)
What?
Raise the specificity of only the
:where(.editor-styles-wrapper)
selector inreset.scss
so that the CSS reset overrides styles in wp-admin/css/common.css.Why?
The admin CSS bleed affects the iframed post editor in 'classic' themes that do not specify
font-size
,line-height
and/orbackground-color
for thebody
. In addition to the extra discrepancies between editor and front end, the smaller text is more difficult to read when editing post content, and the background reduces color contrast.How?
html
to the:where(.editor-styles-wrapper)
ruleset (in the compiled stylesheet).Testing Instructions
body
element coming fromcommon.css
(orload-styles.php
ifSCRIPT_DEBUG
isfalse
). With the PR applied, thebackground
,color
,font-family
,font-size
andline-height
properties should have a line through them (themin-width
is not reset, but that seems to be fine).