feat: Make sidebar width adjustable #2872
Conversation
|
🚀 Thanks for opening this pull request! |
|
The label |
📝 WalkthroughWalkthroughThe changes introduce a user-resizable sidebar by updating the Sidebar component to support drag-based resizing and synchronizing its width with a CSS custom property ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant CSS
participant LocalStorage
User->>Sidebar: Mouse down on resize handle
Sidebar->>Sidebar: startResize()
Sidebar->>User: Track mousemove events
loop While dragging
User->>Sidebar: Mouse move
Sidebar->>Sidebar: Update width state (min 200px)
Sidebar->>CSS: Update --sidebar-width property
end
User->>Sidebar: Mouse up
Sidebar->>LocalStorage: Save new width
Sidebar->>Sidebar: Remove event listeners
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (5)
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 (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
mtrezza
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/stylesheets/base.scss (1)
10-12: Consider documenting the new CSS custom propertyFuture maintainers may not immediately realize this variable is manipulated at runtime by the Sidebar component. A short comment would improve discoverability.
:root { + /* Width of the left-hand sidebar – updated dynamically via Sidebar.react.js */ --sidebar-width: 300px; }src/components/Sidebar/Sidebar.scss (1)
25-33: Well-implemented resize handle, consider usability.The resize handle implementation is technically correct with proper positioning, cursor, and user interaction properties. However, the 5px width might be challenging for users to target precisely.
Consider increasing the handle width slightly for better usability:
- width: 5px; + width: 8px;Alternatively, you could add a hover effect to provide visual feedback when the handle is targetable.
src/components/Sidebar/Sidebar.react.js (2)
39-41: Improve localStorage parsing robustness.The localStorage parsing could be more robust to handle edge cases like invalid values or empty strings.
Consider adding validation for the parsed value:
- const initialWidth = parseInt(localStorage.getItem('sidebarWidth') || '300', 10); + const getInitialWidth = () => { + const stored = localStorage.getItem('sidebarWidth'); + const parsed = parseInt(stored || '300', 10); + return Number.isNaN(parsed) || parsed < 200 ? 300 : parsed; + }; + const initialWidth = getInitialWidth();
123-139: Well-implemented drag-to-resize functionality.The resize implementation correctly handles mouse drag interactions with proper event listener management and cleanup. The minimum width constraint of 200px prevents the sidebar from becoming unusably narrow.
Consider making the minimum width configurable:
+const MIN_SIDEBAR_WIDTH = 200; - const newWidth = Math.max(200, startWidth + ev.clientX - startX); + const newWidth = Math.max(MIN_SIDEBAR_WIDTH, startWidth + ev.clientX - startX);This would make the constraint more maintainable if requirements change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/components/FlowFooter/FlowFooter.scss(1 hunks)src/components/Sidebar/Sidebar.react.js(5 hunks)src/components/Sidebar/Sidebar.scss(2 hunks)src/components/Toolbar/Toolbar.scss(1 hunks)src/dashboard/Dashboard.scss(1 hunks)src/dashboard/Data/Browser/Browser.scss(2 hunks)src/dashboard/Data/Browser/BrowserFooter.scss(1 hunks)src/dashboard/TableView.scss(1 hunks)src/parse-interface-guide/PIG.scss(2 hunks)src/stylesheets/base.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/Sidebar/Sidebar.react.js (1)
Learnt from: mtrezza
PR: parse-community/parse-dashboard#2726
File: src/components/ContextMenu/ContextMenu.react.js:56-74
Timestamp: 2025-05-24T18:24:15.988Z
Learning: In the Parse Dashboard ContextMenu component, the one-time positioning calculation using hasPositioned ref works well even with window resizing and movement, so additional window resize event listeners are not necessary for menu positioning.
⏰ 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). (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (10)
src/components/Toolbar/Toolbar.scss (1)
14-14: Good switch to CSS variable – enables real-time resizingThe fallback guarantees legacy support while allowing the new dynamic width. Looks correct.
src/dashboard/TableView.scss (1)
13-13: LGTM – dynamic sidebar width reflected in table headers
No issues spotted.src/components/FlowFooter/FlowFooter.scss (1)
13-13: Variable use is correct; confirm animation still behaves as intended
leftis transitioned, so changing--sidebar-widthshould still animate the footer. Just keep in mind some browsers skip transitions when only a custom property changes—test in target browsers.src/dashboard/Dashboard.scss (1)
11-11: LGTM – content area now respects adjustable sidebar
Implementation is straightforward and consistent with the other updates.src/dashboard/Data/Browser/Browser.scss (1)
13-13: LGTM! Consistent implementation of dynamic sidebar width.The changes properly replace hardcoded sidebar width values with the CSS custom property
--sidebar-width, maintaining backward compatibility with the 300px fallback. This enables responsive layout adjustments based on user-resizable sidebar width.Also applies to: 27-27
src/parse-interface-guide/PIG.scss (1)
14-14: LGTM! Proper implementation of dynamic sidebar width.The changes correctly replace fixed pixel values with the CSS custom property
--sidebar-widthwhile maintaining consistent fallback behavior. The implementation aligns with the overall refactor objective.Also applies to: 56-56
src/dashboard/Data/Browser/BrowserFooter.scss (1)
5-5: LGTM! Correct implementation of dynamic width calculation.The footer width calculation properly adapts to the dynamic sidebar width using CSS custom properties within the calc() function. The implementation maintains the original calculation logic while enabling responsive behavior.
src/components/Sidebar/Sidebar.scss (1)
17-17: LGTM! Consistent implementation of dynamic sidebar width.The width properties are correctly updated to use the CSS custom property with appropriate fallback values.
Also applies to: 174-174
src/components/Sidebar/Sidebar.react.js (2)
72-75: LGTM! Proper CSS custom property synchronization.The useEffect correctly updates the CSS custom property and maintains the current width reference to prevent stale closures during drag operations.
221-221: LGTM! Properly positioned resize handle.The resize handle is correctly positioned and connected to the resize functionality.
Uffizzi Ephemeral Environment
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/CategoryList/CategoryList.scss (1)
40-44: Addmin-width: 0to guarantee the ellipsis works inside a flex rowWhen a flex-item is allowed to grow (
flex: 1 1 auto), some browsers (notably Chrome) will refuse to applytext-overflow: ellipsisunless the item may shrink below its content width.
Explicitly settingmin-width: 0solves this and prevents layout breakage when class names are long.&:last-of-type { flex: 1 1 auto; + min-width: 0; /* allow shrinking for proper ellipsis */ overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }
🧹 Nitpick comments (1)
src/components/CategoryList/CategoryList.scss (1)
31-38:display: blockon flex items is redundantEach
spanis already a flex item because its parent<a>isdisplay: flex.
The additionaldisplay: blockdeclaration adds no benefit and can be removed for cleaner CSS.No code change required if you keep it, just flagging the redundancy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/CategoryList/CategoryList.scss(2 hunks)src/components/Sidebar/Sidebar.scss(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/Sidebar/Sidebar.scss
⏰ 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). (1)
- GitHub Check: Docker linux/amd64
This reverts commit ef2f3d3.
|
Resizing works, but it needs further adjustment for min and max width of sidebar. Leaving this open for someone to pick up. |
Summary
--sidebar-widthTesting
npm test(fails: Cannot read properties of undefined (reading 'statusCode'))https://chatgpt.com/codex/tasks/task_e_686bb3991f68832dbaacc10d547636ca
Summary by CodeRabbit
New Features
Style