fix: persist user settings across sessions#324
Merged
siddharthvaddem merged 4 commits intosiddharthvaddem:mainfrom Apr 5, 2026
Merged
fix: persist user settings across sessions#324siddharthvaddem merged 4 commits intosiddharthvaddem:mainfrom
siddharthvaddem merged 4 commits intosiddharthvaddem:mainfrom
Conversation
Add userPreferences module to save/load padding, aspect ratio, export format and quality to localStorage. Applied on mount in VideoEditor. Closes siddharthvaddem#306
Load saved preferences (padding, aspect ratio, export quality, export format) on mount and auto-save whenever these settings change. Uses the existing userPreferences.ts utility with a ref guard to prevent overwriting saved prefs with defaults before the initial load completes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughVideoEditor now hydrates display/export preferences from persisted storage on mount and persists subsequent changes. A new userPreferences module implements loading, validation, defaults, and saving to localStorage. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as VideoEditor (UI)
participant Pref as userPreferences module
participant LS as Browser localStorage
UI->>Pref: loadUserPreferences()
Pref->>LS: read PREFS_KEY
LS-->>Pref: JSON blob / null
Pref-->>UI: validated UserPreferences
UI-->>UI: apply to editor state, set prefsHydrated
Note over UI,Pref: Later, when settings change (padding/aspect/export...)
UI->>Pref: saveUserPreferences(partial)
Pref->>LS: read current, merge, write PREFS_KEY
LS-->>Pref: write ack / error (swallowed)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d746196d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 363-385: The guard currently sets prefsLoadedRef.current = true
synchronously inside the mount useEffect, which allows the autosave useEffect to
run in the same commit with default values; instead, delay flipping
prefsLoadedRef until after the state updates have been committed (e.g., wrap the
assignment in a microtask like queueMicrotask or setTimeout(...,0) inside the
same mount effect) so loadUserPreferences ->
updateState/setExportQuality/setExportFormat run first and only then
prefsLoadedRef is set, preventing saveUserPreferences from persisting defaults;
touch the mount useEffect where prefsLoadedRef, loadUserPreferences,
updateState, setExportQuality and setExportFormat are used.
In `@src/lib/userPreferences.ts`:
- Around line 37-39: The loadUserPreferences function reads
localStorage.getItem(PREFS_KEY) without guarding for storage errors; wrap that
call in a try/catch around the localStorage access (while still using
safeJsonParse on the result) and on any exception fall back to returning {
...DEFAULT_PREFS } so initialization won't throw in
private/disabled/quota-exceeded environments; reference loadUserPreferences,
PREFS_KEY, and safeJsonParse when making the change.
- Around line 47-53: The preference parsing allows invalid values: tighten
validation in the prefs construction where raw.aspectRatio, raw.exportQuality
and raw.exportFormat are read by checking membership against the accepted union
literals instead of broad type checks; for aspectRatio validate raw.aspectRatio
is one of the eight allowed AspectRatio strings before using it (otherwise use
DEFAULT_PREFS.aspectRatio), for exportQuality include "good" (so only "medium" |
"good" | "source" are accepted) and default to DEFAULT_PREFS.exportQuality if
not one of those, and for exportFormat include "mp4" (so only "gif" | "mp4" are
accepted) and default to DEFAULT_PREFS.exportFormat if not one of those.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4da6e219-88be-4cb8-b322-8081464f4e03
📒 Files selected for processing (2)
src/components/video-editor/VideoEditor.tsxsrc/lib/userPreferences.ts
- Replace useRef with useState for prefsHydrated to prevent race condition - Wrap localStorage.getItem in try/catch in loadUserPreferences - Validate aspectRatio against known valid values - Include 'good' in exportQuality validation, 'mp4' in exportFormat validation
- Add updateState to useEffect dependency array - Remove ineffective biome-ignore suppression comment - Fix formatting in userPreferences.ts per biome rules
Contributor
Author
|
Pushed fix for CI lint errors:
CI should pass now. |
Owner
|
🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Settings like padding, aspect ratio, export format, and quality reset to hardcoded defaults when the app restarts (no project loaded). Every new session requires reconfiguring.
Fix
src/lib/userPreferences.tswithloadUserPreferences()/saveUserPreferences()backed by localStorageVideoEditorviauseEffectprefsLoadedRefto prevent defaults from overwriting saved prefsTesting
Closes #306
Summary by CodeRabbit
New Features
Bug Fixes