[3914] added flowMetaTheme in buildThemeConfigFromFlowMeta#30
Conversation
Signed-off-by: Zeeshan Mehboob <[email protected]>
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughFlow metadata now supports per-size theme tokens and additional resolved style fields. A shared normalizer converts border-radius inputs, and React and Vue theme builders use the normalized per-size configuration. ChangesTheme configuration propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FlowMeta
participant normalizeBorderRadius
participant ReactThemeBuilder
participant VueThemeBuilder
FlowMeta->>ReactThemeBuilder: provide shape.borderRadius
FlowMeta->>VueThemeBuilder: provide shape.borderRadius
ReactThemeBuilder->>normalizeBorderRadius: normalize border-radius
VueThemeBuilder->>normalizeBorderRadius: normalize border-radius
normalizeBorderRadius-->>ReactThemeBuilder: return per-size tokens
normalizeBorderRadius-->>VueThemeBuilder: return per-size tokens
ReactThemeBuilder-->>ReactThemeBuilder: include borderRadius configuration
VueThemeBuilder-->>VueThemeBuilder: include borderRadius configuration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Signed-off-by: Zeeshan Mehboob <[email protected]>
| /** Spacing configuration for the theme */ | ||
| spacing?: {unit: number}; | ||
|
|
||
| borderRadius?: FlowMetaThemeSize; |
There was a problem hiding this comment.
borderRadius is already a part of shape. Is there a reason for having a first class property?
There was a problem hiding this comment.
Can we try to stick to the MUI Var ThemeProvider structure?
Note
You'll have to transform in buildThemeConfigFromFlowMeta.ts
{
"alpha": "[Function: alpha]",
"applyStyles": "[Function: applyStyles$1]",
"blur": {
"heavy": "blur(15px)",
"light": "blur(5px)",
"medium": "..."
},
"border": {
"style": "solid",
"width": "1px"
},
"breakpoints": {
"between": "[Function: between]",
"down": "[Function: down]",
"keys": "[Array...]"
},
"colorSchemeSelector": "data-color-scheme",
"colorSchemes": {
"dark": "{...}",
"light": "{...}"
},
"colorSpace": null,
"components": {
"MuiAlert": "{...}",
"MuiAppBar": "{...}",
"MuiAutocomplete": "{...}"
},
"containerQueries": "[Function: containerQueries]",
"cssVarPrefix": "oxygen",
"darken": "[Function: darken]",
"defaultColorScheme": "light",
"direction": "ltr",
"font": {
"body1": "400 0.875rem/1.5 'Inter Variable', sans-serif..."
},
"generateSpacing": "[Function: generateSpacing]",
"generateStyleSheets": "[Function: generateStyleSheets]",
"generateThemeVars": "[Function: generateThemeVars]",
"getColorSchemeSelector": "[Function: getColorSchemeSelector]",
"getCssVar": "[Function: getCssVar]",
"gradient": {
"primary": "linear-gradient(90deg, #3688FF 0%, #1d5e...)"
},
"lighten": "[Function: lighten]",
"mixins": {
"toolbar": "{...}"
},
"opacity": {
"inputPlaceholder": 0.42,
"inputUnderline": 0.42,
"switch...": "..."
},
"overlays": [],
"palette": {
"Alert": "{...}",
"AppBar": "{...}",
"Avatar": "{...}",
"Button": "{...}"
},
"rootSelector": ":root",
"shadows": ["none", "none", "none", "none", "none", "none", "..."],
"shape": {
"borderRadius": 8
},
"shouldSkipGeneratingVar": "[Function: shouldSkipGeneratingVar]",
"spacing": "[Function: spacing]",
"syntax": {
"dark": "{...}",
"light": "{...}"
},
"toRuntimeSource": "[Function: stringifyTheme$1]",
"transitions": {
"create": "[Function: create]",
"duration": "{...}",
"easing": "{...}"
},
"typography": {
"body1": "{...}",
"body2": "{...}",
"button": "{...}",
"caption": "{...}"
},
"unstable_sx": "[Function: sx]",
"unstable_sxConfig": {
"alignContent": "{...}",
"alignItems": "{...}",
"alignSelf": "{...}"
},
"vars": {
"blur": "{...}",
"border": "{...}",
"font": "{...}",
"gradient": "{...}"
},
"zIndex": {
"appBar": 1100,
"drawer": 1200,
"fab": 1050,
"mobileStepper": "..."
}
}There was a problem hiding this comment.
But what if I want to have 3 different borderRadius sizes for different layout (small, medium & large)?
Current implementation will give same radius in all the layout,
...(borderRadiusStr
? {
borderRadius: {
large: borderRadiusStr,
medium: borderRadiusStr,
small: borderRadiusStr,
},
}
: {})There was a problem hiding this comment.
Understood. My concern was that whatever configs we provide for the Theme, should be reflected in ThunderID Console's Theme configurator.
Shall we do this instead? And change the Console & Theme contract to support the different sizes?
"shape": {
"borderRadius": {
"small": ...
}
},There was a problem hiding this comment.
oh ok got it @brionmario , i will shift to the shape only, any other issue?
There was a problem hiding this comment.
updated the same @brionmario , can you please review it again
Signed-off-by: Zeeshan Mehboob <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react/src/utils/buildThemeConfigFromFlowMeta.ts (1)
45-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd automated coverage for all accepted input forms.
packages/react/src/utils/buildThemeConfigFromFlowMeta.ts#L45-L59: test scalar, numeric-string, CSS-string, partial-token, and omitted inputs.packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts#L40-L54: add equivalent coverage, or cover the shared lower-layer helper after extraction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/utils/buildThemeConfigFromFlowMeta.ts` around lines 45 - 59, The border-radius parsing in packages/react/src/utils/buildThemeConfigFromFlowMeta.ts (lines 45-59) requires automated coverage for scalar, numeric-string, CSS-string, partial-token, and omitted inputs; add tests covering each accepted form and its expected configuration. Apply equivalent coverage to packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts (lines 40-54), or extract and test a shared lower-layer helper while preserving both adapters’ behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/src/utils/buildThemeConfigFromFlowMeta.ts`:
- Around line 39-60: Centralize the duplicated border-radius normalization used
by buildThemeConfigFromFlowMeta: add or reuse a lower-package normalizer and
export it for consumers. In
packages/react/src/utils/buildThemeConfigFromFlowMeta.ts lines 39-60, replace
the local normalization with that shared export; make the same replacement in
packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts lines 34-55, preserving
the existing token behavior for object, numeric, and string values.
---
Nitpick comments:
In `@packages/react/src/utils/buildThemeConfigFromFlowMeta.ts`:
- Around line 45-59: The border-radius parsing in
packages/react/src/utils/buildThemeConfigFromFlowMeta.ts (lines 45-59) requires
automated coverage for scalar, numeric-string, CSS-string, partial-token, and
omitted inputs; add tests covering each accepted form and its expected
configuration. Apply equivalent coverage to
packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts (lines 40-54), or extract
and test a shared lower-layer helper while preserving both adapters’ behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fe77fbe2-0804-4e3a-ab06-6ae81508cf70
📒 Files selected for processing (3)
packages/javascript/src/models/flow-meta.tspackages/react/src/utils/buildThemeConfigFromFlowMeta.tspackages/vue/src/utils/buildThemeConfigFromFlowMeta.ts
Signed-off-by: Zeeshan Mehboob <[email protected]>
Signed-off-by: Zeeshan Mehboob <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/browser/src/theme/normalizeBorderRadius.ts (1)
39-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit coverage for the shared normalizer.
Cover undefined values, numeric and pixel strings, partial size objects, empty objects, whitespace, and unusual numeric inputs. This helper is consumed by both framework builders, so one focused test suite protects both paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/browser/src/theme/normalizeBorderRadius.ts` around lines 39 - 54, Add a focused unit test suite for normalizeBorderRadius covering undefined input, numeric and pixel strings, partial size objects, empty objects, whitespace trimming, and unusual numeric inputs. Assert the normalized output and undefined results as appropriate, using the shared helper directly so both framework-builder consumers are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/browser/src/theme/normalizeBorderRadius.ts`:
- Around line 52-54: Update the normalization logic around the raw value
conversion so finite numeric inputs are detected by type and always receive a px
suffix, including exponent-form numbers such as 1e-7. Define and apply the
intended numeric-string format coverage separately, preserving nonnumeric
strings unchanged and returning the same value for large, medium, and small.
---
Nitpick comments:
In `@packages/browser/src/theme/normalizeBorderRadius.ts`:
- Around line 39-54: Add a focused unit test suite for normalizeBorderRadius
covering undefined input, numeric and pixel strings, partial size objects, empty
objects, whitespace trimming, and unusual numeric inputs. Assert the normalized
output and undefined results as appropriate, using the shared helper directly so
both framework-builder consumers are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7170fa6e-e6f9-4248-9ef5-d51f33f3ba4f
📒 Files selected for processing (4)
packages/browser/src/index.tspackages/browser/src/theme/normalizeBorderRadius.tspackages/react/src/utils/buildThemeConfigFromFlowMeta.tspackages/vue/src/utils/buildThemeConfigFromFlowMeta.ts
Signed-off-by: Zeeshan Mehboob <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts (1)
40-40: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for border-radius normalization and omission behavior.
Test numeric, pixel-string, full per-size object, partial object, and
undefinedinputs through the Vue builder. Verify that absent metadata does not addborderRadiusand that per-size values are preserved.Also applies to: 74-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts` at line 40, Add tests for buildThemeConfigFromFlowMeta covering normalizeBorderRadius with numeric, pixel-string, complete per-size object, partial object, and undefined inputs. Verify through the Vue builder that absent border-radius metadata omits borderRadius entirely and that provided per-size values remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts`:
- Line 40: Add tests for buildThemeConfigFromFlowMeta covering
normalizeBorderRadius with numeric, pixel-string, complete per-size object,
partial object, and undefined inputs. Verify through the Vue builder that absent
border-radius metadata omits borderRadius entirely and that provided per-size
values remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e94ce16d-682d-4c96-8e3b-d1107e7d2e0b
📒 Files selected for processing (1)
packages/vue/src/utils/buildThemeConfigFromFlowMeta.ts
Signed-off-by: Zeeshan Mehboob <[email protected]>
Signed-off-by: Zeeshan Mehboob <[email protected]>
2 participants
Purpose
Some theme values were unintentionally getting overwritten during runtime. This PR ensures that the actual theme configuration returned by the backend is properly mapped and prioritized across the UI components.
Enhance SDK theme building to support flowMetaTheme properties
Changes:
Core Interfaces: Expanded the FlowMetaTheme interface in the JavaScript package to support border, shadows and spacing properties.
SDK Integration: Updated the buildThemeConfigFromFlowMeta method across both the React and Vue wrappers to properly inject and forward the flowMetaTheme configuration. This ensures all metadata-driven theme properties are correctly transported and applied down to the SDK.
Update: Broaden shape.borderRadius type to accept a bare number, a px string, or a sized {small, medium, large} object
Approach
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Summary
New Features
px), CSS values, and per-size configurations.border,shadows, andspacingtokens.Improvements