feat: add collapsible folder management section to settings #997
Conversation
|
|
📝 WalkthroughWalkthroughThe pull request adds UI collapsibility to the FolderManagementCard component with local state management, extends SettingsCard to support optional action elements in its header, and introduces a new devDependency for browser mapping tooling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 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: 1
🤖 Fix all issues with AI agents
In @frontend/package.json:
- Line 91: The devDependency "baseline-browser-mapping" is unused and should be
removed to avoid bloat; edit package.json to delete the
"baseline-browser-mapping": "^2.9.11" entry from the devDependencies (or, if you
intend to keep it, add actual imports/usages where the collapsible folder or
related code references baseline-browser-mapping so the package is justified),
then run package manager prune/install to update lockfile.
🧹 Nitpick comments (2)
frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (2)
1-3: Consolidate React imports.You're importing
Reacton line 1 anduseStateseparately on line 3. For consistency and cleaner imports, consolidate them into a single import statement.♻️ Proposed consolidation
-import React from 'react'; import { Folder, Trash2, Check, ChevronDown, ChevronUp } from 'lucide-react'; -import { useState } from 'react'; +import React, { useState } from 'react';
40-53: Add aria-label for accessibility.The toggle button should include an
aria-labeloraria-expandedattribute to improve screen reader support. This helps assistive technology users understand the button's purpose and current state.♿ Proposed accessibility enhancement
action={ <Button variant="ghost" size="sm" className="h-8 w-8 p-0" onClick={() => setIsExpanded(!isExpanded)} + aria-label={isExpanded ? "Collapse folder list" : "Expand folder list"} + aria-expanded={isExpanded} > {isExpanded ? ( <ChevronUp className="h-4 w-4" /> ) : ( <ChevronDown className="h-4 w-4" /> )} </Button> }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
frontend/package.jsonfrontend/src/pages/SettingsPage/components/FolderManagementCard.tsxfrontend/src/pages/SettingsPage/components/SettingsCard.tsx
🔇 Additional comments (3)
frontend/src/pages/SettingsPage/components/SettingsCard.tsx (2)
17-20: LGTM! Clean extensibility enhancement.The addition of the optional
actionprop is well-typed and maintains backward compatibility. This provides a flexible way to inject header controls without modifying the core card structure.
47-47: LGTM! Proper action rendering.The action is conditionally rendered with
ml-autofor correct right-alignment in the header.frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (1)
55-157: LGTM! Collapsible implementation is well-structured.The collapsible functionality is correctly implemented with:
- Proper state management using
isExpanded- Smooth animation using Tailwind utility classes
- All folder content appropriately wrapped in the conditional block
- Existing functionality preserved within the collapsible section
| "@vitejs/plugin-react": "^4.2.1", | ||
| "autoprefixer": "^10.4.20", | ||
| "babel-jest": "^29.7.0", | ||
| "baseline-browser-mapping": "^2.9.11", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if baseline-browser-mapping is referenced anywhere in the codebase
echo "Searching for baseline-browser-mapping usage in source files..."
rg -n "baseline-browser-mapping" --type ts --type tsx --type js --type json -g '!package*.json'
echo -e "\nSearching for any imports or requires of this package..."
rg -n "from ['\"]baseline-browser-mapping['\"]" --type ts --type tsx --type js
rg -n "require\(['\"]baseline-browser-mapping['\"]\)" --type ts --type tsx --type js
echo -e "\nChecking package.json references..."
rg -n "baseline-browser-mapping" frontend/package.jsonRepository: AOSSIE-Org/PictoPy
Length of output: 367
Remove the unused baseline-browser-mapping devDependency.
The package is added to package.json but is not imported or used anywhere in the codebase. Unused dependencies bloat the project and increase build time. Either use this package as part of the collapsible folder feature or remove it from package.json.
🤖 Prompt for AI Agents
In @frontend/package.json at line 91, The devDependency
"baseline-browser-mapping" is unused and should be removed to avoid bloat; edit
package.json to delete the "baseline-browser-mapping": "^2.9.11" entry from the
devDependencies (or, if you intend to keep it, add actual imports/usages where
the collapsible folder or related code references baseline-browser-mapping so
the package is justified), then run package manager prune/install to update
lockfile.
1 participant
Fixes #969
Implementation Approach
1. Refactor SettingsCard Component:
actionprop (React.ReactNode).actionin the header, allowing flexible injection of controls (like toggle buttons) without breaking existing usages inUserPreferencesCardorApplicationControlsCard.2. Enhance FolderManagementCard:
isExpanded(defaulting totruefor visibility).ChevronDown/ChevronUpicons fromlucide-react.actionprop of SettingsCard.Before
After :
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.