| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
BorderRadius Presets: Fix Generating wrong variable names in pattern code #71631
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
BorderRadius Presets: Fix Generating wrong variable names in pattern code #71631
Conversation
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @EldarAgalarov. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
t-hamano
added
[Type] Bug
|
@USERSATOSHI Thanks for the PR! @aaronrobertshaw @ramonjd @andrewserong, can you take a look at this PR if you have the bandwidth? |
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.
Thanks for the PR and especially the added tests @USERSATOSHI 👍
I've taken this PR for a test drive and it fixes the reported issue for me.
It appears though that there are some unnecessary code changes which we should probably clean up before merging this one.
Additionally, when attempting to replicate the original issue I found that there were other inconsistencies with the serialization of preset values from this control e.g. sometimes if two corners used the same preset, only one would get the var preset string etc. The removal of the call to decodeValue looks to have cleaned this up as well.
packages/block-editor/src/components/border-radius-control/utils.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/border-radius-control/utils.js
Outdated
Show resolved
Hide resolved
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.
Thanks for the quick iterations here @USERSATOSHI 🚀
This is testing well for me. Border presets are correctly applied:
✅ In block, site, and pattern editors
✅ on the frontend
✅ via global styles
✅ when overriding global styles with block instance styles
There was a failing test that looks unrelated so I've kicked off those tests again. Also, there's a small nit/suggestion for the new tests.
Other than that, I think this is pretty much good to go.
Appreciate all the work on this one 🙇
packages/block-editor/src/components/border-radius-control/test/utils.js
Outdated
Show resolved
Hide resolved
Labels
3 participants
What?
Closes #71628
This PR fixes the handling of border radius presets in block patterns, ensuring that custom
radiusSizesdefined in a theme’stheme.jsonare correctly referenced in generated pattern code. It also adds comprehensive unit tests for the utility functions responsible for this logic.Why?
Previously, when using custom border radius presets, the generated block pattern code would sometimes output raw values or invalid preset references (e.g., using the raw clamp value or
var:preset|border-radius|0instead of"0"for "None"). This led to inconsistencies and made it difficult to maintain design token references in patterns.This PR ensures:
"0") always serializes as"0", not as a preset reference.How?
getPresetValueFromCustomValueandgetPresetValueFromControlValueinborder-radius-control/utils.jsto handle the"0"slug and preset references correctly.Testing Instructions
Follow the reproduction steps from #71628
Other than that Unit tests should suffice.
Screenshots
After fix