Upgrade #3
Upgrade #3
Conversation
WalkthroughThe changes introduce two new responsive breakpoints, "xs" and "xxs", across the grid layout system, updating default properties and type definitions to support finer control on smaller screens. The grid compaction algorithm is optimized by passing a bottom boundary parameter. Additional tests and a demo component are added to verify bounded dragging behavior. Minor adjustments include a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IsBounded (Component)
participant ReactGridLayout
participant DOM
User->>IsBounded: Mount component
IsBounded->>ReactGridLayout: Render with isBounded=true
User->>DOM: Initiate drag on GridItem
DOM->>ReactGridLayout: Drag events
ReactGridLayout->>DOM: Apply transform (bounded)
ReactGridLayout->>IsBounded: Call onDragStop callback
Poem
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. 🔧 ESLint
lib/GridItem.jsx(node:24478) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files Oops! Something went wrong! :( ESLint: 9.27.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by lib/responsiveUtils.js(node:24480) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files Oops! Something went wrong! :( ESLint: 9.27.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by lib/ResponsiveReactGridLayout.jsx(node:24479) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files Oops! Something went wrong! :( ESLint: 9.27.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/react-grid-layout.min.jsis excluded by!**/dist/**,!**/*.min.jsdist/react-grid-layout.min.js.mapis excluded by!**/dist/**,!**/*.map,!**/*.min.js.map
📒 Files selected for processing (14)
.gitignore(1 hunks)lib/GridItem.jsx(1 hunks)lib/ResponsiveReactGridLayout.jsx(1 hunks)lib/responsiveUtils.js(1 hunks)lib/utils.js(3 hunks)test/examples/00-showcase.jsx(1 hunks)test/examples/06-dynamic-add-remove.jsx(1 hunks)test/examples/08-localstorage-responsive.jsx(2 hunks)test/examples/14-toolbox.jsx(1 hunks)test/examples/15-drag-from-outside.jsx(1 hunks)test/examples/17-responsive-bootstrap-style.jsx(2 hunks)test/examples/test_demo.jsx(1 hunks)test/spec/lifecycle-test.js(2 hunks)test/test-hook.jsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
test/examples/test_demo.jsx (1)
index.d.ts (1)
WidthProvider(167-167)
lib/GridItem.jsx (1)
lib/calculateUtils.js (9)
calcGridItemWHPx(25-35)h(148-148)clamp(163-169)calcGridColWidth(14-19)positionParams(15-15)positionParams(55-55)positionParams(108-108)positionParams(141-141)w(147-147)
test/spec/lifecycle-test.js (1)
test/examples/test_demo.jsx (2)
IsBounded(7-25)IsBounded(7-25)
🪛 Biome (1.9.4)
lib/utils.js
[error] 243-243: Shouldn't redeclare 'b'. Consider to delete it or rename it.
'b' is defined here:
(lint/suspicious/noRedeclare)
[error] 325-325: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 325-325: expected , but instead found ?
Remove ?
(parse)
[error] 326-326: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 327-327: return types can only be used in TypeScript files
(parse)
[error] 328-329: This code is unreachable
(lint/correctness/noUnreachable)
[error] 330-330: This code is unreachable
(lint/correctness/noUnreachable)
🔇 Additional comments (20)
lib/responsiveUtils.js (1)
8-8: LGTM! Proper type extension for new breakpoints.The addition of "xs" and "xxs" breakpoints to the
DefaultBreakpointstype alias correctly extends the responsive system to support finer-grained screen size control.test/test-hook.jsx (1)
2-2: No createRoot usage found—regression claim is invalid.The file
test/test-hook.jsximports from"react-dom"and exclusively usesReactDOM.render; there’s noreact-dom/clientorcreateRootanywhere in the codebase. You can safely ignore the original regression comment.Likely an incorrect or invalid review comment.
test/examples/08-localstorage-responsive.jsx (2)
22-22: LGTM! Consistent breakpoint extension in defaultProps.The addition of
xs: 4, xxs: 2to the default column configuration properly extends responsive support for smaller screen sizes, following a logical progression from the existing breakpoints.
42-42: LGTM! Consistent breakpoint extension in render props.The breakpoint configuration matches the defaultProps, ensuring consistency between the static definition and runtime usage.
lib/GridItem.jsx (1)
513-521: Verify removal ofcontainerPaddingin drag boundary calculationsThe
onDraglogic now clampstopandleftwithout accounting forcontainerPadding, altering how items snap to the container edges. There are currently no tests covering bounded dragging (isBounded: true), so please manually confirm:
- Grids with non-zero
containerPaddingstill prevent items from spilling over padded edges- The new
xsandxxsbreakpoints (wherecontainerPaddingdefaults tonull) behave as expected- Visual consistency matches previous versions when
containerPaddingis providedtest/examples/14-toolbox.jsx (1)
42-42: LGTM! Consistent breakpoint extension for toolbox example.The addition of
xs: 4, xxs: 2to the column configuration maintains consistency with other example components and properly extends responsive support for the toolbox layout.test/examples/06-dynamic-add-remove.jsx (1)
12-12: LGTM! Responsive breakpoint addition follows best practices.The addition of
xs: 4, xxs: 2breakpoints creates a logical progression of decreasing column counts for smaller screen sizes, enhancing the responsive behavior of the grid layout.test/examples/00-showcase.jsx (1)
31-31: LGTM! Consistent breakpoint values across examples.The breakpoint values match other example files, maintaining consistency in the responsive grid system across the library.
test/examples/15-drag-from-outside.jsx (1)
11-11: LGTM! Maintains consistency across example suite.The breakpoint addition is consistent with other examples and follows the established responsive design pattern.
test/examples/test_demo.jsx (1)
1-25: LGTM! Well-structured test component for bounded dragging.The component follows established patterns:
- Proper use of WidthProvider HOC
- Clean layout definition for testing purposes
- Correct prop passing for
onDragStopcallback- Appropriate use of
isBounded={true}for testing drag constraintsThe implementation is focused and suitable for its testing purpose.
test/examples/17-responsive-bootstrap-style.jsx (2)
16-16: LGTM! Bootstrap-style 12-column grid pattern maintained.Using 12 columns for
xsandxxsbreakpoints is appropriate for a bootstrap-style layout, maintaining consistency with bootstrap's responsive grid system.
39-39: LGTM! Width configuration properly synchronized with column counts.The widths object correctly maps the new breakpoints to 12, ensuring consistency between the
colsandwidthsconfigurations in the bootstrap-style layout.lib/ResponsiveReactGridLayout.jsx (1)
164-166: LGTM! New responsive breakpoints enhance small screen support.The addition of
xs(480px) andxxs(0px) breakpoints with appropriate column counts (4 and 2 respectively) provides better granular control for smaller screens. The configuration is consistent with the existing pattern.test/spec/lifecycle-test.js (3)
19-42: LGTM! Well-implemented mouse event helper function.The
mouseMovefunction correctly creates and dispatches a syntheticmousemoveevent with proper initialization parameters. This is a good utility for testing drag behavior.
43-50: LGTM! Clean drag simulation helper function.The
simulateMovementFromTofunction provides a clean abstraction for simulating complete drag sequences with mouseDown, mouseMove, and mouseUp events. Good reusable test utility.
466-480: LGTM! Test correctly verifies bounded drag behavior.The test properly mounts the
IsBoundedcomponent, simulates a drag operation, and verifies that the transform output matches the expected"translate(10px,0px)"format. This effectively tests the bounded dragging functionality.lib/utils.js (4)
242-243: LGTM! Good optimization to track bottom position.Tracking the bottom position of static items upfront and passing it to
compactItemavoids repeated bottom calculations, improving performance during layout compaction.
254-255: LGTM! Efficient bottom position tracking.The bottom position is correctly updated after each item compaction and passed to subsequent
compactItemcalls. This maintains accuracy while avoiding redundant calculations.
325-327: Ignore static analysis false positives - Flow types are correct.The static analysis tool is incorrectly flagging Flow type annotations as TypeScript syntax. The function signature correctly adds the
b: numberparameter for the bottom position optimization. The "unreachable code" warnings are also false positives caused by the linter's confusion with Flow syntax.
334-334: LGTM! Efficient use of pre-calculated bottom position.Using the passed
bparameter instead of recalculatingbottom(compareWith)improves performance while maintaining the same logical behavior.
| dist | ||
| dist/* | ||
| !dist/react-grid-layout.min* | ||
| !dist/*.html |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
dist entry shadows the later negate rules – minified bundles & HTML will now be ignored
Adding the bare dist pattern ignores the directory itself, so Git stops traversing its contents.
The subsequent negations (!dist/react-grid-layout.min*, !dist/*.html) therefore no longer take effect, meaning the artefacts you explicitly wanted to keep tracked will be excluded from commits and npm publishes.
Remove the new line or restructure the rules as follows:
-# Ignore entire dist directory (breaks negate rules below)
-dist
+# Keep specific build artefacts, ignore everything else inside dist
+dist/*
+!dist/react-grid-layout.min*
+!dist/*.htmlFailing to fix this will drop your distributable assets from the repo and packaged tarball.
Please verify npm pack output after the change.
Restructure .gitignore to preserve specific dist artefacts
The bare dist entry prevents Git from descending into that directory, so your negations never take effect. Remove it and ignore everything inside dist/ first, then re-include the files you need.
• File: .gitignore
• Lines: 3–6
- dist
+# Keep only our minified bundle & HTML, ignore all other files under dist
+ dist/*
!dist/react-grid-layout.min*
!dist/*.htmlAfter updating, confirm your changes with git check-ignore -v dist/react-grid-layout.min.js or by running npm pack to ensure the artefacts are included.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In the .gitignore file at lines 3 to 6, remove the bare "dist" entry because it
prevents Git from checking inside the dist directory, making the negation rules
ineffective. Instead, ignore all files inside dist/ with "dist/*" first, then
explicitly re-include the needed files like "dist/react-grid-layout.min*" and
"dist/*.html" using negation patterns. After making these changes, verify with
"git check-ignore -v dist/react-grid-layout.min.js" or by running "npm pack" to
confirm the correct files are included.
| ReactDOM.render( | ||
| React.createElement(ListeningLayout, gridProps), | ||
| contentDiv | ||
| ); |
There was a problem hiding this comment.
Update rendering logic to use modern React 18 API.
Correspondingly, the rendering logic should use the modern createRoot API instead of the deprecated ReactDOM.render.
- ReactDOM.render(
- React.createElement(ListeningLayout, gridProps),
- contentDiv
- );
+ const root = createRoot(contentDiv);
+ root.render(React.createElement(ListeningLayout, gridProps));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In test/test-hook.jsx around lines 47 to 50, replace the deprecated
ReactDOM.render call with the modern React 18 createRoot API. Import createRoot
from 'react-dom/client', create a root using createRoot(contentDiv), and then
call root.render with the React element. This updates the rendering logic to
conform to React 18 standards.
Description
This PR [adds/removes/fixes/replaces] the [feature/bug/etc].
What type of PR is this? (check all applicable)
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings
Added tests?
Added to documentation?
Summary by CodeRabbit
New Features
xsandxxs) across grid layouts, enabling finer control for smaller screen sizes.Bug Fixes
Tests
Chores
.gitignoreto ignore thedistdirectory.