-
Notifications
You must be signed in to change notification settings - Fork 107
fix(backups): add remove button for selected backup paths #272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(backups): add remove button for selected backup paths #272
Conversation
…ilesystem Add X button to selected paths list in backup schedule form. When files/folders are selected for backup but later deleted from the volume, restic fails because `--files-from` references non-existent paths. Previously, users couldn't remove these stale paths since they don't appear in the file browser to uncheck. The X button enables direct removal of any selected path, fixing stuck backup configurations that would always fail with "file not found" errors.
WalkthroughThe changes add functionality to remove selected include paths from a backup schedule form. A new Changes
Pre-merge checks✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a removal button for selected backup paths in the backup schedule form, addressing a critical usability issue where users couldn't remove paths that no longer exist on the filesystem since they wouldn't appear in the file browser.
Key Changes:
- Added an X button next to each selected path for direct removal
- Implemented
handleRemovePathcallback to handle path deletion from both UI state and form state - Enhanced UI with hover effects and accessibility labels
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/modules/backups/components/create-schedule-form.tsx (1)
173-181: LGTM!The removal logic is correct and properly synchronizes both the local state and form value. The dependency array includes
selectedPaths, which is appropriate since the function reads from it.Optional: Functional update pattern to reduce callback recreation
If you want to optimize callback stability, you could use functional updates to avoid the
selectedPathsdependency:const handleRemovePath = useCallback( (pathToRemove: string) => { - const newPaths = new Set(selectedPaths); - newPaths.delete(pathToRemove); - setSelectedPaths(newPaths); - form.setValue("includePatterns", Array.from(newPaths)); + setSelectedPaths((prev) => { + const newPaths = new Set(prev); + newPaths.delete(pathToRemove); + form.setValue("includePatterns", Array.from(newPaths)); + return newPaths; + }); }, - [selectedPaths, form], + [form], );However, the current implementation is perfectly fine for this use case.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/modules/backups/components/create-schedule-form.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting with
bunx biome check --write ., format only withbunx biome format --write ., or lint withbunx biome lint .
Files:
app/client/modules/backups/components/create-schedule-form.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses"type": "module"
Files:
app/client/modules/backups/components/create-schedule-form.tsx
app/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/client/**/*.{ts,tsx}: Client uses TanStack Query for server state management
Client uses Radix UI primitives with custom Tailwind styling
Use Server-Sent Events hook (use-server-events.ts) for real-time updates in the client
Files:
app/client/modules/backups/components/create-schedule-form.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/client/**/*.{ts,tsx} : Client uses TanStack Query for server state management
Applied to files:
app/client/modules/backups/components/create-schedule-form.tsx
⏰ 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: Upload results
🔇 Additional comments (2)
app/client/modules/backups/components/create-schedule-form.tsx (2)
5-5: LGTM!The X icon import is appropriate for the remove button functionality.
387-400: LGTM!The UI implementation for removable path chips is well-designed:
- The
inline-flexlayout properly aligns the text and buttontype="button"prevents accidental form submission- The
aria-labelattribute ensures screen reader accessibility- Hover states provide clear visual feedback
- The small X icon is appropriately sized for compact chips
Adds an X button to each selected path in the backup schedule form, allowing users to remove paths that no longer exist on the filesystem.
Problem: When files/folders selected for backup are deleted from the volume, they couldn't be removed from the configuration since they don't appear in the file browser to uncheck. This caused backups to always fail with restic errors.
Solution: Added a clickable X button next to each path in the "Selected paths" list for direct removal.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.