Patterns: Apply layout and alignment to synced patterns in the editor #54416
Conversation
This file contains 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
aaronrobertshaw
added
[Type] Bug
|
Size Change: +199 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Sep 13, 2023
| @@ -59,39 +62,38 @@ export default function ReusableBlockEdit( { | |||
| ref | |||
| ); | |||
|
|
|||
| const alignments = [ 'wide', 'full' ]; | |||
|
|
|||
| useEffect( () => { | |||
There was a problem hiding this comment.
I wonder about something can we avoid setting the "layout" attribute? and use useMemo for both inferredAlignment and inferredLayout. To avoid messing up with re-renderings and undo/redo as well.
There was a problem hiding this comment.
I've been testing this in a bunch of different cases and it's working pretty well for the most part!
The diff between editor and front end can be slighly messed up if the post content has default layout type, but that's to be expected after #54371. (Incidentally testing this caused me to uncover a bug in that PR 😅 )
One other thing I notice is that, because of the alignfull class, root padding aware alignments come into effect if the pattern wrapper is in the post at root level (when post content has constrained layout:
I'm not sure that that's solvable unless by adding another exception to our record-breaking worst line of CSS. I'll have a think about it. Definitely adding alignfull is the most straightforward way of solving the problem here.
aaronrobertshaw
changed the base branch from
fix/synced-pattern-width-issue
to
trunk
aaronrobertshaw
force-pushed
the
fix/synced-pattern-alignment-and-layout
branch
from
bf18e93
to
b53ee7d
Compare
aaronrobertshaw
commented
Sep 14, 2023
| @@ -45,12 +93,20 @@ export default function ReusableBlockEdit( { attributes: { ref } } ) { | |||
| ref | |||
| ); | |||
|
|
|||
| const { alignment, layout } = useInferredLayout( blocks, parentLayout ); | |||
| const layoutClasses = useLayoutClasses( { layout }, name ); | |||
There was a problem hiding this comment.
Generating the layout styles directly allows us to avoid;
- fully adopting layout support for the editor only
- setting layout attributes and needing to mark the changes as not persisted etc.
| } ); | ||
|
|
||
| const innerBlocksProps = useInnerBlocksProps( blockProps, { | ||
| value: blocks, | ||
| layout, |
There was a problem hiding this comment.
Passing the inferred layout here allows the pattern's inner blocks to still apply their alignment and layout classes appropriately.
|
Flaky tests detected in b53ee7df20b2f4948084a05cc6a68b7e6fd90ea9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6181146199
|
aaronrobertshaw
requested review from
spacedmonkey and
ellatrix
as code owners
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/class-wp-theme-json-test.php |
|
The changes to the styles generated by the theme.json PHP class will need backporting. I'll sort out a patch for that tomorrow. |
andrewserong
approved these changes
Sep 14, 2023
There was a problem hiding this comment.
This is looking great to me, nice work honing in on a lightweight solution that improves rendering in the editor, without affecting block markup! This feels like a safe direction to me, and could easily be rolled back if it introduced any issues for anyone. Testing well:
✅ Correct layout is applied in the editor when a child block contains a wide or full wide alignment
✅ Switching the child blocks to no alignment allows the user to still switch back to an alignment again
✅ Updated global styles rules for root padding ensure root padding behaves as expected in the post and site editors without impacting other blocks using those rules (tested with root padding rules on and off)
Once the PHP tests are updated to match the new markup, I think this PR will be in good shape to land! I'm sure you'll get a few more pairs of eyes for testing, but count this as a vote from me 😀
LGTM! ✨
aaronrobertshaw
force-pushed
the
fix/synced-pattern-alignment-and-layout
branch
from
8be7f69
to
93c0c0f
Compare
86 tasks
kevin940726
reviewed
Sep 14, 2023
Mamaduka
reviewed
Sep 14, 2023
tellthemachines
approved these changes
Sep 14, 2023
|
Thank you everyone for all the extended discussions, feedback, tweaks, testing, and reviews 🙇 I think it's time to merge this one. We can explore a long-term solution that also brings parity between the editor and frontend markup separately, including whether that is best done via a v2 |
This was referenced Sep 14, 2023
Closed
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.
Fixes:
Related:
What?
Adds alignment and layout to the editor-only
core/blockwrapper to maintain visual consistency when converting to a pattern.Why?
It's important to provide a smooth editing experience around patterns. Creating a pattern from wide, full, left, or right-aligned blocks, only to see them move and get squashed into a pattern block wrapper in the editor is unexpected.
How?
core/blockinstance needs a layout applied it is inherited from the parent layout if available.Testing Instructions
See testing instructions on:
Screenshots or screencast