Block editor: give iframe fallback background color #57330
Merged
Conversation
This file contains 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
ramonjd
added
[Type] Bug
|
Size Change: -21.6 kB (-1%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
t-hamano
reviewed
Dec 22, 2023
There was a problem hiding this comment.
I went back through the commits and it appears that #56904 first caused this problem. At that time, it appears that the white background color has simply been removed.
If the expected results are the same, how about adding a white default background to the useResizeCanvas hook?
Furthermore, I expect that this style can also be deleted.
cc: @youknowriad
Thanks for the suggestion! Using the useResizeCanvas hook also works, and it doesn't involve creating a new style file. I have a slight preference for putting CSS in CSS files, but happy to take advice. Another thing is that the hook is being imported as __experimentalUseResizeCanvas. Will it always be used to show the editor, or would be need a more "stable" location?
Nice spotting 👍🏻 |
|
Flaky tests detected in 2d599a5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7333697873
|
youknowriad
reviewed
Jan 1, 2024
| iframe[name="editor-canvas"] { | ||
| width: 100%; | ||
| height: 100%; | ||
| background-color: $white; |
There was a problem hiding this comment.
Do you know if the issue is present in the site editor as well? I know there were a lot of back and forth about the default background there due to the loading mechanics and whether this change could impact it. cc @jasmussen
There was a problem hiding this comment.
Thanks for checking.
Yes, for empty theme (or any theme with no set background color) the post and site editors will have a default background color of background-color: $gray-900; coming from https://github.com/WordPress/gutenberg/blob/81852a55e700afdaaca7657a2382dd38c0e52b5a/packages/edit-post/src/components/visual-editor/style.scss#L14
There was a problem hiding this comment.
Do you know if the issue is present in the site editor as well?
This issue has also occurred in the Site Editor in the past, but it has been fixed in #46314. After that, the selector was changed in part of #46752.
There is also an issue where the background color flashes white when the Site Editor loads for the first time (#57001). This problem occurs both in trunk and in this PR, so I think it will need to be fixed in a follow-up.
There was a problem hiding this comment.
Oh, thanks for correcting me @t-hamano
I meant to say, "without a fallback like this..." 😄
This PR unifies those previous fixes for the site editor with the post editor so they'll work the same, running from the same CSS rule.
There was a problem hiding this comment.
CC: @jameskoster — you were there for some of these conversations. Do you recall the details?
tellthemachines
approved these changes
Jan 2, 2024
andrewserong
reviewed
Jan 2, 2024
There was a problem hiding this comment.
This is testing nicely for me, too!
While testing with classic themes, I noticed that TwentyNineteen appears to expose the background color of .edit-post-visual-editor since it sets the max-width of .block-editor-writing-flow to 80% which results in the background showing through at the sides:
Not a blocker for this PR, but just thought I'd mention it since it seems to be another case where the .edit-post-visual-editor background color is poking through, but in that case, maybe it's the theme that needs to be fixed up 🤔
|
Thanks a lot for the reviews here folks. I'll leave it open a day or two in case @jasmussen has feedback. Beyond that, happy to follow up or revert if there's a preferred way. |
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.
What?
I think it fixes #56981
Why?
For themes that do not set a background, the dark background color (
background-color: $gray-900;) from.edit-post-visual-editoris visible.If the text color is also dark, it's hard to read.
How?
By setting a default background color to the editor iframe of
#fff(white).Ad mentioned above, currently that default color is
background-color: $gray-900;Any theme that sets a background color on the html or body element or wherever, will always override this.
Testing Instructions
emptytheme#fffiframePropstake precedence. The iframe should be pink and narrow.Example diff to check that iframeProps still work
Testing Instructions for Keyboard
Screenshots or screencast
Constrast in mobile view