-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply same styles to block previews on inserter and Global Styles #63177
Conversation
Size Change: -5.83 kB (-0.33%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
@jasmussen @jameskoster this one is mostly there, I need to keep testing more blocks and settle on a solution for the grid block (and add screenshots of the testing to the PR description) but if you have any insights that would change the direction of this PR they would be welcome |
This one looks great to me, substantially better than trunk in virtually all cases. There's some animation happening that's also in trunk, but for some reason it stood out to me more here: No action item necessarily, except perhaps turning off any transitions if you find them while you're here. The main point is the grid, which definitely looks more like a grid in trunk: You mention this:
Is there any reason why blocks inside the preview should not be full-wide? If no, then it seems reasonable to apply a blanket 100% width to all blocks inside the previews. Can you explain in more detail the extra rule? |
Thanks for the input @jasmussen ! The main reason I see is cases like the button block, where if we apply the blanket width: 100% we would end up with the button aligned left. Same with blocks like site logo or anything that has a short text (login/out, site title, etc) |
Gotcha. It seems what you've found here is something you repro by putting a grid inside a row block: Which suggests two paths foward, unless I'm missing nuance:
I don't know that I like either of those paths forward. Perhaps 2 if possible. But at the same time, flex is the main missing ingredient to make all these previews better, yes? How do we generally feel about adding bespoke CSS for individual previews? Is this something plugin blocks can do? If yes, seems fine to do here. |
94c9867
to
170bf13
Compare
@jasmussen I found out that the flexbox issue was causing problems with the separator block too (it would resolve to width 0 and be invisible). I changed tactics and did the following:
I think this solves the breakage and the tradeoffs are minimal (some blocks are left aligned but still look ok). It also doesn't require any actions from third party blocks. The problem you pointed out that happens when combining flexbox and grid I couldn't figure out an easy way to solve, since that is how CSS interacts in the wild, and seemed a bit out of scope here but still important to have in mind |
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. |
Flaky tests detected in f7844fe. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9888899355
|
Seems fine with me. I think this one could use a broader code review than I can provide, though. |
I'll go fish for technical reviews |
Co-authored-by: Ben Dwyer <ben@scruffian.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great. Well done 👏
packages/edit-site/src/components/global-styles/block-preview-panel.js
Outdated
Show resolved
Hide resolved
…rdPress#63177) * apply same styles to both previews * reduced default viewport width * use sass units instead of magic numbers * revert viewport width change * fix height issue on inserter preview * calculate min height after scaling * change viewportWidth variable to add proper default * fix width issue with flexbox * centered buttons * centered blocks * remove unnecessary css rule * Update packages/block-editor/src/components/inserter/preview-panel.js Co-authored-by: Ben Dwyer <ben@scruffian.com> * Update packages/edit-site/src/components/global-styles/block-preview-panel.js * commented the CSS --------- Co-authored-by: Ben Dwyer <ben@scruffian.com>
What?
This tries to apply the same styles to the Block Preview that lives in Global Styles to the one in the inserter, and tries to fix some minor visual issues for both in the meantime.
Why?
The previews for block look very different in the GS sidebar and the inserter. Different CSS is applied to both and GS wasn't even scaling with the viewport width selected on the blocks settings. This aims to align the two together for a more consistent experience.
How?
This tries to bring the changes from #46727 to the inserter previews and polish any issues that arise for certain blocks because of it.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Before:
After: