Global Styles UI: Revert some of the padding / markup changes from #73334 #73834
Conversation
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
andrewserong
added
[Type] Bug
andrewserong
changed the title
|
Size Change: -47 B (0%) Total Size: 2.57 MB
ℹ️ View Unchanged
|
Member
LGTM For good measure, I checked the dataviews for pages and media experiments and they looked okay too. |
|
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 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. |
ramonjd
approved these changes
Dec 9, 2025
Contributor
Author
|
Thanks for the quick review @ramonjd! I'll leave this PR open until tomorrow morning, just in case @youknowriad was intending to roll out 24px padding there, or had a different idea in mind for addressing the padding inconsistencies. Of course, do feel free to click "Squash and merge", anyone! |
t-hamano
reviewed
Dec 9, 2025
Comment on lines
12
to
15
|
|
||
| .global-styles-ui-sidebar__navigator-screen.components-navigator-screen { | ||
| padding: $grid-unit-15; | ||
| .components-navigator-screen { | ||
| padding: 0; | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| .global-styles-ui-sidebar__navigator-screen.components-navigator-screen { | |
| padding: $grid-unit-15; | |
| .components-navigator-screen { | |
| padding: 0; | |
| } |
The unintended padding for Navigator.Screen was removed in #73810, so this override is no longer necessary.
Comment on lines
+18
to
+25
|
|
||
| .global-styles-ui-sidebar__navigator-screen { | ||
| padding-top: $grid-unit-15; | ||
| padding-left: $grid-unit-15; | ||
| padding-right: $grid-unit-15; | ||
| padding-bottom: $grid-unit-15; | ||
| outline: none; | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| .global-styles-ui-sidebar__navigator-screen { | |
| padding-top: $grid-unit-15; | |
| padding-left: $grid-unit-15; | |
| padding-right: $grid-unit-15; | |
| padding-bottom: $grid-unit-15; | |
| outline: none; | |
| } |
This adds even more extra padding, and I think it is unnecessary.
| Actual | Expected |
|---|---|
 |
 |
Contributor
Author
There was a problem hiding this comment.
Just checked against WP 6.9 without Gutenberg active and I believe this padding was intentional so that the padding in this panel is effectively 24px (16px on the internals of the global styles UI plus this 12px). I'd prefer to leave the discussion surrounding what the ideal padding is in this panel to other PRs / issues.
Contributor
|
At first sight for me, this PR does too much changes and make the global styles panel less reusable. Now, I can't express it properly in code, let me check to see if I would do any different. |
Contributor
|
Ok, I dove deeper and here is what I tried to do: I tried to remove all padding from "global-styles-ui" because We want 16px when used in the editor sidebar but we want 24px when used the site editor. So I wanted to move the "padding" responsibility to the container, that way the global styles sidebar becomes more reusable and we need less CSS overrides. Now, it seems that in some cases we want borders that span the whole thing edge to edge. So I see two solutions IMO:
Both of these options are acceptable to me, I'll let you decide what to do for now. What I don't want is more and more CSS overrides like this PR is doing. |
Contributor
I think this is the simplest and most ideal.
As far as I know, only block screens require edge-to-edge borders, so if we absolutely need an edge-to-edge model, we may be able to override it at the consumer level. |
Contributor
Author
|
Thanks for the input everyone! Good ideas, there are some limitations I encountered in exploring the suggestions and I've pushed a change that seeks to go in the spirit of the direction of removing CSS overrides, and inching toward configurability, but without actually going for an implementation just yet. Here's what I've done:
The main reasons that I didn't remove padding altogether or go with configurability is that we don't yet have consistency amongst the internal components. Some components are wrapped in a What this means is that I think we effectively want the internals to assume a visual I'm hoping that the current state of this PR provides a happy medium, and offers a step in the right direction to tidying things up. At the very least, I think we're removing more CSS rules than we're adding. I'd like to land this change if it's okay, and we can continue looking at code quality follow-ups in subsequent PRs. My main goal is for things to visually look pretty much as they did prior to #73334. |
Contributor
What if we remove all the padding from the internal components and just make them span the full width like suggested above? I'm not sure I understand what the limitations here are (outside the border not being edge to edge which seems fine according to @jay's instincts in DataViews for instance) |
Contributor
Author
I tried this but it didn't work because we have several areas that use ToolsPanel or panels from the block-editor package that already contain Any objections to us merging this PR for now? I'll be AFK after today for about five days, so I'd rather get a fix in now if we can, so that |
Contributor
|
Would it be possible to give me a day to explore the alternative and we reconvene to see what we do. |
Contributor
Author
Absolutely! Feel free to commit or merge to this PR, or close and open another. I mostly wanted to be responsible with the time I have today before I go AFK, but happy to hand this over to you. If there are still issues with this area when I'm back later on next week, happy to continue helping out 🙂 |
Contributor
|
Ok let's land this, It hurts me to merge this PR :P but I agree that updating the subcomponents is a bigger task than I expected. The main two blockers:
Both of these changes do impact the block sidebars and not just the global styles. I do think both changes would allow us to remove a ton of CSS overrides if done properly but it's impactful and would require care. Anyone feel free to tackle that problem if time allows :) |
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?
Closes #73843
Reverts some of the global styles UI changes introduced in #73334 — fix the unexpected additional padding. It looks like those changes meant that some of the areas in the right-hand sidebar wind up with (effectively)
24pxpadding. I think that's probably a bit beyond the scope of the intention behind #73334, so this PR proposes reverting those changes.Why?
These changes were unrelated to the specific task of updating DataViews and Modal to use
24pxpadding, and caused some padding issues of their own for particular screens, e.g. Edit Palette, Font size presets, Blocks.Kudos @t-hamano for flagging. And FYI @youknowriad.
How?
Revert just those changes that appeared to affect these areas of the global styles sidebar. That is, restore the markup structure of the root, header, and style variations, and the kind of padding for
Card, and revert many of the CSS changes.Update: also consolidate the components a little and create a
ScreenBodycomponent.Testing Instructions
Screenshots or screencast