Reduce clearance around the Frame in the site editor #57023
Merged
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
jameskoster
added
[Type] Enhancement
|
Size Change: -9 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in b4c6ad2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7246704927
|
richtabor
reviewed
Dec 15, 2023
packages/base-styles/_variables.scss
Outdated
| $modal-width-large: 840px; | ||
| $spinner-size: 16px; | ||
| $canvas-padding: $grid-unit-30; | ||
| $canvas-padding: $grid-unit-10; |
Member
There was a problem hiding this comment.
Feels a bit too tight. Perhaps we can line it up with the site icon? $grid-unit-20 is close—perhaps a happy medium?
Current:
With 20:
Contributor
Author
There was a problem hiding this comment.
If we want to align with the site icon then grid-unit-15 is even closer:
I'd be happy with either though.
Contributor
There was a problem hiding this comment.
+1 to Rich's suggestion :)
Nice attention to detail, thanks for the PR, Jay!
Contributor
Author
There was a problem hiding this comment.
I updated to that suggestion 👍
Contributor
There was a problem hiding this comment.
Good step forward. Before:
After:
Happy to green chek this one. However, this is not lining up with the site logo:
The site logo is using a CSS transform to scale a 64px tall container to 0.5, making the site icon effectively 32x32 in its resting state. I wonder if we should tweak those maths or not, I think you separately suggested a 64px header may be a good endstate to reach for our post and site editors, so maybe we keep that.
The main issue here is that we're actually cropping that 64x64 site logo container into 60x60 to match the existing post/site editor header heights. This is part of what's causing things to not line up. And we could either change that clearance you just updated to 14px, or we could change the transform to an uneven scale(0.4375) to make the site logo be 28x28px.
In either case, that would make things line up. So I guess the question is, we need to find another 4px. Where do we find them? Do we update metrics to account for 60px top toolbars? Or do we update top toolbars to be 64px?
Contributor
Author
There was a problem hiding this comment.
Since this PR addresses an issue in data views, and because the site icon doesn't align correctly on trunk either, I would prefer to tackle that problem in a follow up.
As you noted there are potentially a few things to try there, and we also need to factor in alignment between site icon + site title + search button, plus site icon + menu items below.
jasmussen
approved these changes
Dec 19, 2023
bph
added
[Feature] Site Editor
artemiomorales
pushed a commit
that referenced
this pull request
Jan 4, 2024
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?
Reduce clearance around the Frame in the site editor from 24px to 8px.
Why?
On screens like data views the current clearance feels unnecessarily large, particularly in the list layout.