fix: address code review feedback for settings persistence
- 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
This commit is contained in:
Generated
+2
-2
@@ -1,12 +1,12 @@
|
||||
{
|
||||
"name": "openscreen",
|
||||
"version": "1.2.0",
|
||||
"version": "1.3.0",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "openscreen",
|
||||
"version": "1.2.0",
|
||||
"version": "1.3.0",
|
||||
"dependencies": {
|
||||
"@fix-webm-duration/fix": "^1.0.1",
|
||||
"@pixi/filter-drop-shadow": "^5.2.0",
|
||||
|
||||
@@ -362,7 +362,7 @@ export default function VideoEditor() {
|
||||
|
||||
// Track whether user preferences have been loaded to avoid
|
||||
// overwriting saved prefs with defaults on the first render
|
||||
const prefsLoadedRef = useRef(false);
|
||||
const [prefsHydrated, setPrefsHydrated] = useState(false);
|
||||
|
||||
// Load persisted user preferences on mount
|
||||
useEffect(() => {
|
||||
@@ -373,16 +373,16 @@ export default function VideoEditor() {
|
||||
});
|
||||
setExportQuality(prefs.exportQuality);
|
||||
setExportFormat(prefs.exportFormat);
|
||||
prefsLoadedRef.current = true;
|
||||
setPrefsHydrated(true);
|
||||
// We intentionally only want this to run once on mount
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: mount-only effect
|
||||
}, []);
|
||||
|
||||
// Auto-save user preferences when settings change
|
||||
useEffect(() => {
|
||||
if (!prefsLoadedRef.current) return;
|
||||
if (!prefsHydrated) return;
|
||||
saveUserPreferences({ padding, aspectRatio, exportQuality, exportFormat });
|
||||
}, [padding, aspectRatio, exportQuality, exportFormat]);
|
||||
}, [prefsHydrated, padding, aspectRatio, exportQuality, exportFormat]);
|
||||
|
||||
const saveProject = useCallback(
|
||||
async (forceSaveAs: boolean) => {
|
||||
|
||||
@@ -3,6 +3,17 @@ import type { AspectRatio } from "@/utils/aspectRatioUtils";
|
||||
|
||||
const PREFS_KEY = "openscreen_user_preferences";
|
||||
|
||||
const VALID_ASPECT_RATIOS: readonly string[] = [
|
||||
"16:9",
|
||||
"9:16",
|
||||
"1:1",
|
||||
"4:3",
|
||||
"4:5",
|
||||
"16:10",
|
||||
"10:16",
|
||||
"native",
|
||||
];
|
||||
|
||||
export interface UserPreferences {
|
||||
/** Default padding % */
|
||||
padding: number;
|
||||
@@ -35,7 +46,12 @@ function safeJsonParse(text: string | null): Record<string, unknown> | null {
|
||||
* Returns defaults for any missing or invalid fields.
|
||||
*/
|
||||
export function loadUserPreferences(): UserPreferences {
|
||||
const raw = safeJsonParse(localStorage.getItem(PREFS_KEY));
|
||||
let raw: Record<string, unknown> | null = null;
|
||||
try {
|
||||
raw = safeJsonParse(localStorage.getItem(PREFS_KEY));
|
||||
} catch {
|
||||
return { ...DEFAULT_PREFS };
|
||||
}
|
||||
if (!raw || typeof raw !== "object") return { ...DEFAULT_PREFS };
|
||||
|
||||
return {
|
||||
@@ -44,13 +60,17 @@ export function loadUserPreferences(): UserPreferences {
|
||||
? raw.padding
|
||||
: DEFAULT_PREFS.padding,
|
||||
aspectRatio:
|
||||
typeof raw.aspectRatio === "string" ? (raw.aspectRatio as AspectRatio) : DEFAULT_PREFS.aspectRatio,
|
||||
typeof raw.aspectRatio === "string" && VALID_ASPECT_RATIOS.includes(raw.aspectRatio)
|
||||
? (raw.aspectRatio as AspectRatio)
|
||||
: DEFAULT_PREFS.aspectRatio,
|
||||
exportQuality:
|
||||
raw.exportQuality === "medium" || raw.exportQuality === "source"
|
||||
raw.exportQuality === "medium" || raw.exportQuality === "good" || raw.exportQuality === "source"
|
||||
? (raw.exportQuality as ExportQuality)
|
||||
: DEFAULT_PREFS.exportQuality,
|
||||
exportFormat:
|
||||
raw.exportFormat === "gif" ? (raw.exportFormat as ExportFormat) : DEFAULT_PREFS.exportFormat,
|
||||
raw.exportFormat === "gif" || raw.exportFormat === "mp4"
|
||||
? (raw.exportFormat as ExportFormat)
|
||||
: DEFAULT_PREFS.exportFormat,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user