feat(filters): save active filters for movies and tv#3219
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds saved filters persistence for the Discover filter slideover, auto-applies stored filters when appropriate, and adds controls to save or remove the saved filter set. Matching English and German strings were added for the new actions and errors. ChangesSaved Filters Feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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.
Actionable comments posted: 3
🤖 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/Discover/FilterSlideover/index.tsx`:
- Around line 70-83: The FilterSlideover effect is re-running endlessly because
save filters state is always truthy and updateLocalStorage.current never gets
turned off; update the logic in useSavedFilters/getSavedFilters usage so the
query-param sync runs only once per intended hydration. In the useEffect inside
FilterSlideover, set updateLocalStorage.current to false after the initial
batchUpdateQueryParams(savedFilters) call (or otherwise gate it with a stable
one-time flag), and avoid depending on the fresh object returned by
getSavedFilters() each render so the effect does not retrigger on every render.
- Around line 79-83: The restore effect in FilterSlideover is overwriting active
URL-provided filters whenever savedFilters exists. Update the useEffect around
batchUpdateQueryParams so it only restores savedFilters when currentFilters is
empty or otherwise not already set, and keep the guard tied to
updateLocalStorage.current to avoid clobbering intentional filters on load.
In `@src/hooks/useSavedFilters.ts`:
- Around line 32-33: Guard the localStorage writes in useSavedFilters so a
thrown setItem/removeItem error does not break callers like FilterSlideover.
Update saveFilters and the corresponding remove/reset path to catch storage
exceptions and fail safely, keeping onClick flows able to continue and onClose
still executing. Use the existing useSavedFilters hook and its
saveFilters/remove logic as the place to add the protection.
🪄 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: CHILL
Plan: Pro
Run ID: f0edb5de-42e8-4514-9b94-9df8bb25233f
📒 Files selected for processing (4)
src/components/Discover/FilterSlideover/index.tsxsrc/hooks/useSavedFilters.tssrc/i18n/locale/de.jsonsrc/i18n/locale/en.json
Description
This is the implementation of feature request to save active filters in movies and series.
Closes #369
The PR adds:
QueryFilterOptionsfor parsing.FilterSlideovercomponent.The hook gets initiliased in
FilterSlideoverand uses type movie and tv to create a storage key for each when clicking on the save button and use it for retrieving data on render.Clear active filters button also removes the values from local storage. I added a delete button because users might want to clear the active filters and set other filters without losing the one that they explicitly saved but then went agains it (three buttons is too much).
The saved filters in local storage will only be updated if the user clicks on the save button, so adding other filters without clicking save won't affect the saved ones.
I went with localStorage to keep it simple. If syncing between devices is required then we need a new column in some table somewhere.
AI:
Claude code for the first draft.
How Has This Been Tested?
typecheckpnpm buildScreenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Summary