CC-41: <Select/> is Missing size Prop#22
Conversation
WalkthroughThis PR introduces a new MasonryGrid component for responsive masonry layouts, adds a size prop to the existing Select component, and adjusts a padding utility mixin. The MasonryGrid component uses a ResizeObserver hook to dynamically compute column counts based on container width and CSS variables, distributes child elements across columns, and applies CSS Grid styling. The Select component gains a configurable size option (small, normal, large) that replaces hardcoded height and padding constraints. A padding utility mixin is updated to support asymmetric horizontal padding. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.11.0)src/components/MasonryGrid/MasonryGrid.module.scssConfigurationError: Could not find "stylelint-config-clean-order". Do you need to install the package or use the "configBasedir" option? src/style/sizes.module.scssConfigurationError: Could not find "stylelint-config-clean-order". Do you need to install the package or use the "configBasedir" option? 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/components/MasonryGrid/MasonryGrid.hooks.ts`:
- Line 41: The initial state for columnCount is set with minColumns which can be
out of range before the effect runs; change the useState initializer to use the
already-computed clampedMinColumns (i.e., useState(clampedMinColumns)) so
columnCount starts within [minColumns, maxColumns] consistently — update the
initializer where columnCount and setColumnCount are declared and ensure
clampedMinColumns is computed before that call.
In `@src/components/MasonryGrid/MasonryGrid.module.scss`:
- Around line 1-4: The .masonry-grid rule currently uses gap:
var(--masonry-column-gap) which affects both row and column spacing; update this
to use column-gap: var(--masonry-column-gap) in the .masonry-grid selector
(keeping the existing variable name --masonry-column-gap) so the intent of only
controlling horizontal spacing is explicit and row gaps remain unaffected.
In `@src/components/MasonryGrid/MasonryGrid.tsx`:
- Around line 40-44: The current comment above the distribution logic is
misleading: the code using Children.forEach and columns[index % columnCount]
implements a round-robin distribution across columns (left-to-right, wrapping
top-to-bottom), not "top-to-bottom, left-to-right"; update the comment to
something like "Distribute children round-robin across columns (left-to-right,
wrapping top-to-bottom)" and keep the existing logic using columns, columnCount,
and Children.forEach unchanged.
- Around line 40-55: The child wrapper uses rowIndex as the key which shifts
when items move; update the distribution in the Children.forEach loop (the code
that builds columns) to store each child's original index (e.g., push a tuple or
object containing { node: child, originalIndex: index } into columns) and then
update the rendering in columns.map / column.map to use that stored
originalIndex as the key for the outer wrapper instead of rowIndex so keys
remain stable across insertions/removals.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5834f92e-df26-41c8-b17a-a36c344d041c
📒 Files selected for processing (11)
src/components/MasonryGrid/MasonryGrid.hooks.tssrc/components/MasonryGrid/MasonryGrid.module.scsssrc/components/MasonryGrid/MasonryGrid.module.scss.d.tssrc/components/MasonryGrid/MasonryGrid.stories.tsxsrc/components/MasonryGrid/MasonryGrid.tsxsrc/components/MasonryGrid/index.tssrc/components/Select/Select.module.scsssrc/components/Select/Select.stories.tsxsrc/components/Select/Select.tsxsrc/index.tssrc/style/sizes.module.scss
💤 Files with no reviewable changes (1)
- src/components/Select/Select.module.scss
| const clampedMaxColumns = Math.max(minColumns, maxColumns); | ||
|
|
||
| const ref = useRef<HTMLDivElement>(null); | ||
| const [columnCount, setColumnCount] = useState(minColumns); |
There was a problem hiding this comment.
Initialize state with clamped value for consistency.
The initial state uses minColumns directly, but if minColumns > maxColumns, the state will briefly hold an invalid value until the effect runs and corrects it. Initialize with clampedMinColumns to maintain consistency from the first render.
✨ Proposed fix
- const [columnCount, setColumnCount] = useState(minColumns);
+ const [columnCount, setColumnCount] = useState(clampedMinColumns);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [columnCount, setColumnCount] = useState(minColumns); | |
| const [columnCount, setColumnCount] = useState(clampedMinColumns); |
🤖 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 `@src/components/MasonryGrid/MasonryGrid.hooks.ts` at line 41, The initial
state for columnCount is set with minColumns which can be out of range before
the effect runs; change the useState initializer to use the already-computed
clampedMinColumns (i.e., useState(clampedMinColumns)) so columnCount starts
within [minColumns, maxColumns] consistently — update the initializer where
columnCount and setColumnCount are declared and ensure clampedMinColumns is
computed before that call.
1 participant
Fixes CC-41
Summary by CodeRabbit
New Features
Bug Fixes
Style