Block gap: Check for splitOnAxis in the onChange callback #39788
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
… object only when the boxvalue control is activated. Rename vars for clarity Add some tests for getGapBoxControlValueFromStyle because we call it directly in flow.js (even though the tests for getGapCSSValue cover it indirectly)
ramonjd
added
[Type] Bug
|
Size Change: +14 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
andrewserong
approved these changes
Mar 27, 2022
There was a problem hiding this comment.
Thanks for the quick follow-up @ramonjd!
This looks like a good approach to me — preserves the string based approach for blockGap unless the block has opted in to axial controls 👍
I'd be keen personally to have a look at seeing if we can roll out the axial controls to the gallery block, too — I'm happy to take a look in a separate PR since I was planning to take a look at the blockGap support in global styles anyway. But this PR looks like a good fix for now!
✅ Gallery block uses string values for blockGap
✅ Social links block still uses axial controls and the box values
✅ Switching off axial controls for the social links block successfully gets it to serialize string values for blockGap
Looks good in the editor and frontend to me, and the markup in post-content looks like:
<!-- wp:gallery {"linkTo":"none","style":{"spacing":{"blockGap":"9px"}}} -->
...
<!-- wp:social-links {"style":{"spacing":{"blockGap":{"top":"10px","left":"5px"}}}} -->
LGTM (just a tiny linting issue that looks like Prettier needs before this can be merged). Thanks again! 🎉
glendaviesnz
approved these changes
Mar 27, 2022
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.
Resolves #39778
What?
#37736 introduced a bug whereby we always returning a box control value for block gap, even when
splitOnAxisand thereforeBoxControlcomponent were inactive.Why?
The consequence of the bug was that blocks that do not opt into split axis gap values, such as the Gallery Block, still expect a string value for gap and will bork when they receive an object.
They don't know what to do with it, kinda like me with a Rubik's cube.
How?
This PR checks whether
splitOnAxisis active and returns a string or box control value appropriately.I also tested that, if we roll back axial gap support on the Social Links block, a box control-like value in the block styles will be converted back to a string.
Testing Instructions
Use a theme that has support for blockGap, e.g., TwentyTwentyTwo
--wp--style--unstable-gallery-gapshould be a string, for example'20px'.npm run test-unit packages/block-editor/src/hooks/test/gap.jsHere's some test HTML
Mar-28-2022.10-05-39.mp4