Conversation
|
Size Change: +8 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
ellatrix
commented
Dec 22, 2022
| } | ||
|
|
||
| .block-editor-iframe__body { | ||
| background-color: $white; |
Member
Author
There was a problem hiding this comment.
@ntsekouras This isn't a good solution because now this rule has a higher specificity than a theme just using the body selector. Also, if a theme sets it on html, it won't be visible. We need to set the white background on the iframe element so that it's not transparent. I fixed that below.
Contributor
There was a problem hiding this comment.
In this brunch this issue #46291 doesn't work though since we're scaling the body of the iframe and this is where the background is applied from theme.json. Any ideas to work around that?
Member
Author
There was a problem hiding this comment.
Ah the scaling... I just talked to Riad about that the other day. I think we should moved the scaling outside the iframe. I'll make a separate PR for that then first.
Member
There was a problem hiding this comment.
Would love to see this fix merged! It's breaking a lot of themes in our deployment of Gutenberg, so we've had to remove this line of CSS from our deployment. It seems like load order of CSS (for example, if CSS is concatenated) can break this.
Member
There was a problem hiding this comment.
Ah nice, @fullofcaffeine found that this PR fixes the issue: #46732
b8eca00 to
683fd3e
Compare
|
Flaky tests detected in ee532e8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6146965021
|
683fd3e to
cd43f33
Compare
Contributor
|
@ellatrix is this a regression? We're already in RC so should only backport fixes for actual regressions from 6.2. |
Member
Author
|
@tellthemachines That's arguable. This is the first release where all editors are iframed by default with the post editor included, so it's part of that feature. Scoping styles seems to cause some problems with escaped CSS property names, but this PR is way less important than Allow styles to be changed dynamically through editor settings. Worst case, people need to avoid escaped property names for now. That said, it would be nice if everything worked normally. |
Contributor
|
The code change in specificity looks good to me at a glance, so I'd just in general encourage good testing. |
Member
Author
|
Let's add this early in the next release cycle so we have a whole cycle to test. |
Contributor
|
@ellatrix for 6.4 you mean? |
Member
Author
|
Yes |
53455bc to
90702f8
Compare
90702f8 to
ee532e8
Compare
ntsekouras
approved these changes
Sep 11, 2023
Contributor
ntsekouras
left a comment
There was a problem hiding this comment.
Looks good, thank you!
youknowriad
reviewed
Sep 14, 2023
| // We use :where to keep specificity minimal. | ||
| // https://css-tricks.com/almanac/selectors/w/where/ | ||
| html :where(.editor-styles-wrapper) { | ||
| :where(.editor-styles-wrapper) { |
Contributor
There was a problem hiding this comment.
Don't you think the changes in this "reset" file might impact non-framed editors?
Contributor
There was a problem hiding this comment.
Changing this line has created a regression within the iframe when using some classic themes.
#57176
Member
|
@ellatrix @ntsekouras It seems this PR created some issues with some backward compatible styles, see conversation at #56293 |
This was referenced Nov 28, 2023
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
What?
This is unnecessary when the content is iframed so we can skip all this work for better performance.
Fixes #52767 (comment) (CSS parser stumbling over escaped characters)
Why?
Right! Why scope if we don't have to? :)
How?
Iframes!
Testing Instructions
Check content styles are not prefix and still work in iframed editors.
Testing Instructions for Keyboard
Screenshots or screencast