Remove white background on Site Editor 'Frame' #48970
Conversation
jameskoster
added
the
[Feature] Site Editor
|
Size Change: -11 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Subtle! I love it.
I tested this also in the post editor in template editing, and with metaboxes. I feel like indeed there was a reason to have these colors, but I can't find any issues.
As such, giving this tentative approval. But let's hold off on merging until Riad has a chance to chime in, and we can keep it in trunk vs. 6.2 for now, just to be sure.
|
Did we test with a theme without background? (Like empty theme)? How does it impact it? |
|
Oh that's probably it. Without this, the frame loads with a dark background behind the spinner.
Once the theme loads, even if it doesn't have a background color, ultimately it falls back to this:
So I guess the question is whether that dark loading state is preferrable to the white haze 🤔 |
|
It should be possible to detect if and what the background color is, and then use a light or dark background, but that is probably overkill. |
|
Maybe the problem isn't that the spinner is on a dark background, in fact that may be preferrable. I think the problem is that there's a brief glimpse of a drop shadow surrounding a frame with no content. I'm not sure how or if that can be fixed — the shadow would ideally be attached to the element that has the background color, so when it's loaded, shadow and background both appear at the same time. But of course that's the body element inside the iframe. It may be best to keep the white background for now, and look at other ways to hide the white haze. |
It's an assumption, but I'd expect the number of sites with a dark background to outnumber those with no background at all. The white fuzziness is quite grating on the eye. It's more obvious since we added the hover animation, but it also appears during the transition between browse/edit, and whenever the canvas is zoomed out (e.g. when choosing a style variation). It would be really nice to fix this.
This seems like a good workaround to me, if possible. The background can be white as a default to fix the 'floating shadow' issue. But when the site loads, can we retrieve the background and apply it here as an override? |
|
Perhaps with 6.2 out the door we can consider merging this? It's easy to revert if anything unforseen comes up. The spinner can be addressed as part of #35503. What do y'all think? |
|
I think we should merge it, yes. But if @youknowriad has a chance to check first, I'd appreciate that. I can't recall where I saw this, but I think he mentioned that there was a performance issue with either having, or omitting, a backgroud on either of these elements. I can't recall the details, but it might be important for this one. The remaining issue is the loading state that shows a drop shadow existing in mid-air:
But it hits me that this issue will be fixed once we get a proper loading state for the entire site editor, which is something we should get to soon anyway. |
|
Personally I'm hesitant. It's not clear to me which one is more common. no background or colored background but I'm not strongly opposed. |
@youknowriad would it be feasible to check if a background color is provided, and only use the white as a fallback when one is not found? |
|
@jameskoster I'm not sure, I think theme.json is not loaded initially, so by the time we'll have the info, the loader is probably already shown. |
|
So, one "hack" we could do, is crop a little bit. This "fixes" the white halo for me: I imagine we can do with less than the 1px if we use various transform tricks here and there. As I write this, I'm realizing a thing that I discovered inspecting this, which is, it doesn't matter if there's a white background, because a child of that already has a #2f2f2f gray background. So we're actually looking at three backgrounds here:
Here's the debugging session:
I think that dark gray is the "canvas" color from focus mode, so it's probably appropriate to keep on that element. But I feel like it does beg the question: if we have three layers, white at the bottom, dark gray between, and then the theme color on the top, doesn't that make the white fallback color more of a theoretical benefit, than a practical one? |
Hm, in that case when would you actually see the white background? 🤔 |
That's what I'm saying. I don't think you can get to such a state at the moment, so we might as well remove it. |
|
Let's merge this. It removes an annoying visual artefact when the site has a dark background and is extremely easy to revert. |
|
I think this PR broke the "manage all templates" page. |
|
I opened a follow-up in #50374 |
What?
Removes the white background applied to
.edit-site-visual-editor__editor-canvasand.edit-site-layout__canvas>div.Why?
When the site has a non-white background, the white frame background can be seen flashing in and out during the on-hover animation. It's particularly noticeable when the background is dark:
before.mp4
Testing Instructions
after.mp4
cc @youknowriad. In the past I think you've expressed that these backgrounds are required, but I couldn't find any issues.