fix(flex): ensure utility styles override component styles (DS-5175) - #438
Conversation
|
Warning Review limit reached
Next review available in: 23 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughFlex prop types moved to a shared module. The flex helper now handles ChangesFlex utility migration
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Visit the preview URL for this PR (updated for commit e38993e): https://react-koobiq-next--prs-438-8ws4eeb6.web.app (expires Sat, 08 Aug 2026 09:29:55 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: fc29847d4a9e5cb1adf458c76a9b681c76e2eeff |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/src/components/FlexBox/FlexBox.tsx (1)
48-57: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward the
orderflex prop to the class generator.
orderis supported byFlexProps,getFlexClassNames, and the stylesheet, butFlexBoxnever resolves or passes it here. It is instead forwarded through...other, soorder={1}does not applykbq-flex-order_1.Proposed fix
rowGap: rowGapProp, + order: orderProp, direction: directionProp, @@ const rowGap = getResponsiveValue(rowGapProp, breakpoints); + const order = getResponsiveValue(orderProp, breakpoints); @@ rowGap, + order, direction,Add an
orderassertion to the FlexBox regression test.🤖 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/components/src/components/FlexBox/FlexBox.tsx` around lines 48 - 57, Update the FlexBox component’s getFlexClassNames call to resolve and pass the supported order prop alongside the other flex properties, preventing it from being handled only through ...other. Add a regression assertion verifying that FlexBox with order={1} applies the kbq-flex-order_1 class.packages/components/src/components/layout/flex/flex.css (1)
1-10: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset the row and column gap variables.
gapreads--flex-row-gapand--flex-column-gap, but neither is initialized. Custom properties inherit, so a nested.kbq-flexwithout a gap prop inherits its parent’s gap. Line 2 initializes an unused variable instead.Proposed fix
.kbq-flex { - --flex-gap: 0; + --flex-row-gap: 0; + --flex-column-gap: 0; --flex-order: 0;🤖 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/components/src/components/layout/flex/flex.css` around lines 1 - 10, Initialize the row and column gap custom properties in the .kbq-flex rule to zero, replacing the unused --flex-gap declaration. Keep the existing gap declaration using --flex-row-gap and --flex-column-gap so nested flex elements reset inherited gaps when no explicit gap is provided.
🧹 Nitpick comments (1)
packages/components/src/components/layout/flex/flex.stories.tsx (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
FlexPropstype-only.Split
FlexPropsintoimport type { FlexProps } from './index'while retaining runtime imports separately. As per coding guidelines, useimport typefor type-only imports.🤖 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/components/src/components/layout/flex/flex.stories.tsx` at line 15, Update the imports in the Flex story to import FlexProps using a type-only import, while keeping the runtime imports from ./index in a separate regular import.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@packages/components/src/components/FlexBox/FlexBox.tsx`:
- Around line 48-57: Update the FlexBox component’s getFlexClassNames call to
resolve and pass the supported order prop alongside the other flex properties,
preventing it from being handled only through ...other. Add a regression
assertion verifying that FlexBox with order={1} applies the kbq-flex-order_1
class.
In `@packages/components/src/components/layout/flex/flex.css`:
- Around line 1-10: Initialize the row and column gap custom properties in the
.kbq-flex rule to zero, replacing the unused --flex-gap declaration. Keep the
existing gap declaration using --flex-row-gap and --flex-column-gap so nested
flex elements reset inherited gaps when no explicit gap is provided.
---
Nitpick comments:
In `@packages/components/src/components/layout/flex/flex.stories.tsx`:
- Line 15: Update the imports in the Flex story to import FlexProps using a
type-only import, while keeping the runtime imports from ./index in a separate
regular import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c31703d4-fc3a-4800-bdff-1ac3f75927a3
📒 Files selected for processing (10)
packages/components/src/components/FlexBox/FlexBox.test.tsxpackages/components/src/components/FlexBox/FlexBox.tsxpackages/components/src/components/layout/flex/classNames.tspackages/components/src/components/layout/flex/flex.csspackages/components/src/components/layout/flex/flex.stories.tsxpackages/components/src/components/layout/flex/flex.test.tspackages/components/src/components/layout/flex/flex.tspackages/components/src/components/layout/flex/index.tspackages/components/src/components/layout/flex/types.tstools/public_api_guard/components/layout.api.md
|
@coderabbitai rate limit |
|
Your plan includes PR reviews subject to rate limits. Reviews are available now. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/components/src/components/layout/flex/flex.ts (1)
23-27: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCover
order: 0in theflexclass tests.
ordersupports-1,0, and1, and.order_0exists in the CSS module, but the existing test only covers.order_-1. Add anorder: 0class generation assertion.🤖 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/components/src/components/layout/flex/flex.ts` around lines 23 - 27, Update the flex class-generation tests for the order prop to include order: 0 and assert that the generated classes contain the existing .order_0 CSS module class, while preserving the current coverage for other order values.
🤖 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/components/src/components/FlexBox/FlexBox.module.css`:
- Around line 1-13: Initialize --flex-row-gap and --flex-column-gap to normal
within the .base rule before the gap shorthand declaration, so either per-axis
override from FlexBox remains valid when the other gap is unset.
---
Nitpick comments:
In `@packages/components/src/components/layout/flex/flex.ts`:
- Around line 23-27: Update the flex class-generation tests for the order prop
to include order: 0 and assert that the generated classes contain the existing
.order_0 CSS module class, while preserving the current coverage for other order
values.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bd822419-5121-4684-9198-e6d9948466a9
📒 Files selected for processing (5)
packages/components/src/components/FlexBox/FlexBox.module.csspackages/components/src/components/FlexBox/FlexBox.test.tsxpackages/components/src/components/FlexBox/FlexBox.tsxpackages/components/src/components/layout/flex/flex.test.tspackages/components/src/components/layout/flex/flex.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/src/components/FlexBox/FlexBox.test.tsx
- packages/components/src/components/layout/flex/flex.test.ts
There was a problem hiding this comment.
Pull request overview
This PR updates the flex layout mixin and the FlexBox component to improve flex option typing, fix order handling (including 0), and adjust how flex-related classes/styles are generated and exported within the components package.
Changes:
- Extracted flex prop option constants/types into a dedicated
types.tsand re-exported them from the flex entrypoint. - Fixed
orderclass application soorder={0}is not skipped, and added unit tests for the flex mixin. - Refactored
FlexBoxto generate flex classes from its own CSS Module and added tests for class application (including responsive props).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/components/layout/flex/types.ts | Introduces shared typed flex prop option lists and FlexProps/FlexParams types. |
| packages/components/src/components/layout/flex/index.ts | Re-exports the new types alongside the flex mixin. |
| packages/components/src/components/layout/flex/flex.ts | Fixes order handling so 0 is treated as a valid value when building classnames. |
| packages/components/src/components/layout/flex/flex.test.ts | Adds tests for the flex mixin output (needs explicit coverage for order: 0). |
| packages/components/src/components/layout/flex/flex.stories.tsx | Updates story imports to use the package entrypoint exports. |
| packages/components/src/components/FlexBox/FlexBox.tsx | Refactors FlexBox to apply flex-related CSS module classes directly and aligns gap fallback behavior. |
| packages/components/src/components/FlexBox/FlexBox.test.tsx | Adds assertions for applied classes from direct and responsive props. |
| packages/components/src/components/FlexBox/FlexBox.module.css | Adds a FlexBox-local flex styling module using CSS variables and modifier classes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary by CodeRabbit
New Features
Bug Fixes
ordervalues, including zero.