Fix widgets background when loading theme styles #32683
Conversation
tellthemachines
requested review from
adamziel,
draganescu,
kevin940726,
noisysocks and
talldan
as code owners
|
Size Change: +1.57 kB (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
Nice work! This looks totally feasible. I checked out this PR alongside WordPress/wordpress-develop#1369 and here's how things looked using Twenty Twenty-one:
Some notes:
|
noisysocks
added
the
[Feature] Widgets Screen
noisysocks
added
the
Backport to WP 6.7 Beta/RC
I don't think we can, unless I'm missing something. Themes add their background colours to the body, which in the editor gets translated as
Done! The culprit was in the Legacy Widget block boilerplate.
On by default, like in the post editor? |
Can we move
I'm not sure. Stepping back, I think there's two possible scenarios for what we can do in 5.8 and we should do some testing and pick the scenario that has the best UX:
|
Thanks! Not sure if moving it will have any ill effect, but adding the same class to each widget area works. It should be fixed now, but my local env is playing up and I can't make sure. I'm inclined to shipping theme styles on by default as the best UX solution, based on the issues raised about it.
This would affect the post editor too, right? Would be good to have some design input on it. In any case, if theme styles are on by default it won't be such an issue. |
Correct. I opened a separate issue for it: #32718 |
tellthemachines
force-pushed
the
fix/widgets-background
branch
from
6225b19
to
4f4a1d2
Compare
@tellthemachines could you provide further setup instructions here? I don't believe I'm set up to test this as described 😅 A quick video/screenshots would also be super helpful if possible!! Would be good to see: widget areas full/empty, open/closed, a widget resting/selected, some common flows like adding and selecting a legacy widget from the dropdown.
Related: #32569 (comment) and #32711 |
|
Here's a video: Kapture.2021-06-17.at.11.33.40.mp4Overall I think it looks really good! My testing notes:
|
This is fixed now!
I removed the hardcoded background colour from the Legacy Widget block, but I think that change needs to be backported to core before we can test it. I tried changing it manually in my local env and it worked ok.
Yeah, the truncation in Twenty Twenty One is more obvious than in other themes (we can fix it with an override for the theme styles) but I can't repro the serif font locally 😕 |
|
This is how it looks for me now. The title of each widget area has some bleeding issues.
|
If you mean the space around the heading, I've just pushed a fix. |
|
Hm. I still see some padding issues with the area header, now in Twenty Twenty.
I think that we're fighting a losing battle against CSS bleed issues in the Widget Area block. How about we remove the top level I've pushed up d2fd4f4 which does exactly that. It seems to work but admittedly I haven't tested it very thoroughly. Let me know what you think and feel free to remove the commit if it's no good. |
|
Nice, thanks for the video @noisysocks ✌️
This is probably the main thing that I would want to be sure we can get resolved. Plus some of the issues @noisysocks mentioned with the extra spacing in the header area, but it sounds like those might be fixed with the latest commit. Another thing that caught my eye was that it looks like the width of the center column changes in width when switching the "Use theme styles" preference on and off (looks wider when the styling is off). Thanks for working on this @tellthemachines! |
|
@critterverse the Legacy Widget block preview is fixed, but this PR will have to be backported to Core before we can see the changes take effect.
Is this the whole widget areas, or just the blocks inside them? I notice some padding diffs on some of the themes, but that's due to theme styling (doesn't happen on all of them). If it was the widget areas themselves though, I can't see that with the latest changes so it should be fixed. |
tellthemachines
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
youknowriad
removed
the
Backport to WP 6.7 Beta/RC
* Fix widgets background when loading theme styles * Adjust for dark background colors. * Fix heading hover styles * Fix theme styles in legacy widget preview * Remove theme colour from editor background * Remove border styling as not needed anymore * Add toggle and fix background styles * Fix padding issue * Add back interface background color * Fix legacy widget select padding * Fix theme style bleed * Fix space around widget area heading * Move .editor-styles-wrapper to the widget area * Create temp canvas only if no canvas exists; inserter fix * Add theme styles for older versions of WP Co-authored-by: Robert Anderson <robert@noisysocks.com>
* Fix widgets background when loading theme styles * Adjust for dark background colors. * Fix heading hover styles * Fix theme styles in legacy widget preview * Remove theme colour from editor background * Remove border styling as not needed anymore * Add toggle and fix background styles * Fix padding issue * Add back interface background color * Fix legacy widget select padding * Fix theme style bleed * Fix space around widget area heading * Move .editor-styles-wrapper to the widget area * Create temp canvas only if no canvas exists; inserter fix * Add theme styles for older versions of WP Co-authored-by: Robert Anderson <robert@noisysocks.com>
Description
WordPress/wordpress-develop#1369 fixes #26163. This PR updates the editor styles so whatever background color the theme sets is reflected in the editor, and changes the widget area border color so it's visible on a light background unless a dark background is set.
How has this been tested?
To test, build local environment with WordPress/wordpress-develop#1369, then apply these changes. Check that with Twenty Twenty One, the editor and the widget area backgrounds match the theme styles, and widget area borders are visible whether the background color is light or dark.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).