Add global button border-radius control to Style Cards#6152
Add global button border-radius control to Style Cards#6152kyrapieterse wants to merge 3 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughAdds button border-radius support: new default properties, editor controls to toggle/set a global/general radius, and style-generation logic that emits CSS/custom properties to apply the radius to Maxi and WordPress-native buttons. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Editor as Editor UI
participant Store as SC Variables Store
participant Generator as Style Generator
participant Browser as Runtime CSS
Editor->>Store: toggle `border-radius-global` / set `border-radius-general`
Store-->>Editor: acknowledge updated values (CSS vars)
Store->>Generator: supply styleCard + variables for generation
Generator->>Browser: emit CSS rules using vars (Maxi & WP-native selectors) with !important
Browser->>Browser: apply border-radius to targeted button elements
note right of Generator `#DDEBF7`: Conditional: only when global flag && general value present
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0c55024a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/extensions/style-cards/getSCStyles.js:
- Around line 805-812: The addedResponse block in getWPNativeStyles references
hasGlobalButtonRadius and buttonRadiusValue which are not declared in that
function; declare and initialize these two variables at the start of
getWPNativeStyles using the same logic used in getMaxiSCStyles (where they are
declared) so the condition and CSS string concatenation can run without a
ReferenceError—locate the hasGlobalButtonRadius and buttonRadiusValue
definitions in getMaxiSCStyles and replicate that initialization (same names)
inside getWPNativeStyles before the if (hasGlobalButtonRadius &&
buttonRadiusValue) block.
- Around line 444-456: getMaxiSCStyles currently reads styleCard but doesn't
receive it; update the getMaxiSCStyles function signature to accept a styleCard
parameter and replace internal references to styleCard accordingly, then pass
the existing styleCard when calling getMaxiSCStyles from within getSCStyles (and
update any other callers of getMaxiSCStyles to provide the new parameter);
ensure variable names match (e.g., hasGlobalButtonRadius, buttonRadiusValue) so
the border-radius logic uses the passed-in styleCard.
In @src/extensions/style-cards/getSCVariablesObject.js:
- Around line 278-294: The switch case declares the constant borderRadiusValue
(from getLastBreakpointAttribute) directly in the case which violates
noSwitchDeclarations; wrap the case body in a block (add { } around the case's
statements) so borderRadiusValue and any other declarations are scoped to that
block, or alternatively move the declaration outside the switch; update the case
that uses getIsValid and response[...]
(`--maxi-${style}-${element}-border-radius-general` and
`--maxi-${style}-${element}-border-radius-global`) to live inside the new block
to prevent leaking the constant to other cases.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/defaults/defaultSC.jsonsrc/editor/style-cards/maxiStyleCardsTab.jssrc/extensions/style-cards/getSCStyles.jssrc/extensions/style-cards/getSCVariablesObject.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/extensions/style-cards/getSCStyles.js (1)
src/extensions/style-cards/getSCVariablesObject.js (1)
prefix(20-20)
🪛 Biome (2.1.2)
src/extensions/style-cards/getSCVariablesObject.js
[error] 278-282: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: e2e-js
- GitHub Check: e2e-js-playwright
- GitHub Check: unit-js
🔇 Additional comments (4)
src/editor/style-cards/maxiStyleCardsTab.js (2)
260-267: LGTM! Derived values follow existing patterns.The conditional extraction of button border radius attributes is consistent with how other button-specific attributes are handled in this component.
321-351: LGTM! UI controls are properly wired.The border radius controls are correctly placed and follow the established pattern for button-specific settings. The unconditional rendering of the number control (regardless of toggle state) appears intentional and allows users to set values before enabling the global override.
core/defaults/defaultSC.json (2)
405-406: LGTM! Sensible default values.The defaults for button border radius (disabled toggle, 0px radius) are appropriate and consistent with the feature being opt-in.
988-989: LGTM! Consistent with dark theme.The light theme defaults mirror the dark theme settings, maintaining consistency across both style card variants.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/extensions/style-cards/getSCStyles.js:
- Around line 444-447: getMaxiSCStyles currently references styleCard (used to
compute hasGlobalButtonRadius and buttonRadiusValue) but styleCard is not
defined in its scope; update getMaxiSCStyles to accept a styleCard parameter
(e.g., function getMaxiSCStyles(style, styleCard, ...) or add styleCard to its
existing params) and then update every call site that invokes getMaxiSCStyles to
pass the appropriate styleCard object so hasGlobalButtonRadius and
buttonRadiusValue can read from styleCard without causing a ReferenceError.
- Around line 805-812: The code in getWPNativeStyles references
hasGlobalButtonRadius and buttonRadiusValue which are only defined in
getMaxiSCStyles; declare and compute these variables inside getWPNativeStyles
(or extract the shared logic into a helper used by both functions).
Specifically, copy or refactor the logic that sets hasGlobalButtonRadius and
buttonRadiusValue from getMaxiSCStyles (lines where they are derived) into
getWPNativeStyles so those symbols exist in that function scope before the block
that appends addedResponse for .wp-element-button and .comment-form input.
In @src/extensions/style-cards/getSCVariablesObject.js:
- Around line 278-290: The switch case currently declares the constant
borderRadiusValue (and uses obj, getIsValid, response, style, element) directly
in the case body, which violates the noSwitchDeclarations rule; fix it by
wrapping that case’s body in a block (add { ... } around the existing statements
for the case) so the declaration is scoped to the block and the logic that sets
--maxi-...-border-radius-general and --maxi-...-border-radius-global remains
unchanged.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/extensions/style-cards/getSCStyles.jssrc/extensions/style-cards/getSCVariablesObject.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/extensions/style-cards/getSCVariablesObject.js (1)
src/editor/style-cards/maxiStyleCardsTab.js (1)
borderRadiusValue(264-267)
🪛 Biome (2.1.2)
src/extensions/style-cards/getSCVariablesObject.js
[error] 278-278: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: e2e-js-playwright
- GitHub Check: e2e-js
- GitHub Check: unit-js
1 participant
Motivation
Description
border-radius-generalandborder-radius-globalto the style card JSON for button groups (light/dark) incore/defaults/defaultSC.json.src/editor/style-cards/maxiStyleCardsTab.jsto set and enable the global radius.src/extensions/style-cards/getSCVariablesObject.jsto emit--maxi-<style>-button-border-radius-general(value in px) and a--maxi-<style>-button-border-radius-globalflag when applicable.src/extensions/style-cards/getSCStyles.jsto applyborder-radius: var(--maxi-<style>-button-border-radius-general) !importantto both Maxi button blocks and native.wp-element-buttonwhen the global toggle is active.Testing
fixes #5236