From 581aae3356e0b3dcf6c05e0f23b728ee5540c1dc Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sun, 30 Nov 2025 07:48:36 -0600 Subject: [PATCH] fix layout race condition --- web/src/hooks/use-user-persistence.ts | 42 ++++- web/src/views/live/DraggableGridLayout.tsx | 175 ++++++++++++--------- web/src/views/settings/UiSettingsView.tsx | 20 ++- 3 files changed, 153 insertions(+), 84 deletions(-) diff --git a/web/src/hooks/use-user-persistence.ts b/web/src/hooks/use-user-persistence.ts index 2eb8a8ccb..519a8aa5a 100644 --- a/web/src/hooks/use-user-persistence.ts +++ b/web/src/hooks/use-user-persistence.ts @@ -12,6 +12,29 @@ type useUserPersistenceReturn = [ // Key used to track which keys have been migrated to prevent re-reading old keys const MIGRATED_KEYS_STORAGE_KEY = "frigate-migrated-user-keys"; +/** + * Compute the user-namespaced key for a given base key and username. + */ +export function getUserNamespacedKey( + key: string, + username: string | undefined, +): string { + const isAuthenticated = username && username !== "anonymous"; + return isAuthenticated ? `${key}:${username}` : key; +} + +/** + * Delete a user-namespaced key from storage. + * This is useful for clearing user-specific data from settings pages. + */ +export async function deleteUserNamespacedKey( + key: string, + username: string | undefined, +): Promise { + const namespacedKey = getUserNamespacedKey(key, username); + await delData(namespacedKey); +} + /** * Get the set of keys that have already been migrated for a specific user. */ @@ -60,8 +83,16 @@ export function useUserPersistence( username && username !== "anonymous" && !auth.isLoading; const namespacedKey = isAuthenticated ? `${key}:${username}` : key; + // Track the key that was used when loading to prevent cross-key writes + const loadedKeyRef = useRef(null); + const setValue = useCallback( (newValue: S | undefined) => { + // Only allow writes if we've loaded for this key + // This prevents stale callbacks from writing to the wrong key + if (loadedKeyRef.current !== namespacedKey) { + return; + } setInternalValue(newValue); async function update() { await setData(namespacedKey, newValue); @@ -72,6 +103,9 @@ export function useUserPersistence( ); const deleteValue = useCallback(async () => { + if (loadedKeyRef.current !== namespacedKey) { + return; + } await delData(namespacedKey); setInternalValue(defaultValue); }, [namespacedKey, defaultValue]); @@ -82,7 +116,8 @@ export function useUserPersistence( return; } - // Reset migration flag when key changes + // Reset state when key changes - this prevents stale writes + loadedKeyRef.current = null; migrationAttemptedRef.current = false; setLoaded(false); @@ -99,6 +134,7 @@ export function useUserPersistence( if (typeof existingNamespacedValue !== "undefined") { // Already have namespaced data, use it setInternalValue(existingNamespacedValue); + loadedKeyRef.current = namespacedKey; setLoaded(true); return; } @@ -107,6 +143,7 @@ export function useUserPersistence( if (migratedKeys.has(key)) { // Already migrated, don't read from legacy key setInternalValue(defaultValue); + loadedKeyRef.current = namespacedKey; setLoaded(true); return; } @@ -119,6 +156,7 @@ export function useUserPersistence( await delData(key); await markKeyAsMigrated(username, key); setInternalValue(legacyValue); + loadedKeyRef.current = namespacedKey; setLoaded(true); return; } @@ -126,6 +164,7 @@ export function useUserPersistence( // No legacy value, just mark as migrated so we don't check again await markKeyAsMigrated(username, key); setInternalValue(defaultValue); + loadedKeyRef.current = namespacedKey; setLoaded(true); return; } @@ -137,6 +176,7 @@ export function useUserPersistence( } else { setInternalValue(defaultValue); } + loadedKeyRef.current = namespacedKey; setLoaded(true); } diff --git a/web/src/views/live/DraggableGridLayout.tsx b/web/src/views/live/DraggableGridLayout.tsx index 1370bc93b..5b875ae5c 100644 --- a/web/src/views/live/DraggableGridLayout.tsx +++ b/web/src/views/live/DraggableGridLayout.tsx @@ -145,6 +145,11 @@ export default function DraggableGridLayout({ useEffect(() => { setIsEditMode(false); setEditGroup(false); + // Reset camera tracking state when group changes to prevent the camera-change + // effect from incorrectly overwriting the loaded layout + setCurrentCameras(undefined); + setCurrentIncludeBirdseye(undefined); + setCurrentGridLayout(undefined); }, [cameraGroup, setIsEditMode]); // camera state @@ -168,104 +173,120 @@ export default function DraggableGridLayout({ [setGridLayout, isGridLayoutLoaded, gridLayout, currentGridLayout], ); - const generateLayout = useCallback(() => { - if (!isGridLayoutLoaded) { - return; - } - - const cameraNames = - includeBirdseye && birdseyeConfig?.enabled - ? ["birdseye", ...cameras.map((camera) => camera?.name || "")] - : cameras.map((camera) => camera?.name || ""); - - const optionsMap: Layout[] = currentGridLayout - ? currentGridLayout.filter((layout) => cameraNames?.includes(layout.i)) - : []; - - cameraNames.forEach((cameraName, index) => { - const existingLayout = optionsMap.find( - (layout) => layout.i === cameraName, - ); - - // Skip if the camera already exists in the layout - if (existingLayout) { + const generateLayout = useCallback( + (baseLayout: Layout[] | undefined) => { + if (!isGridLayoutLoaded) { return; } - let aspectRatio; - let col; + const cameraNames = + includeBirdseye && birdseyeConfig?.enabled + ? ["birdseye", ...cameras.map((camera) => camera?.name || "")] + : cameras.map((camera) => camera?.name || ""); - // Handle "birdseye" camera as a special case - if (cameraName === "birdseye") { - aspectRatio = - (birdseyeConfig?.width || 1) / (birdseyeConfig?.height || 1); - col = 0; // Set birdseye camera in the first column - } else { - const camera = cameras.find((cam) => cam.name === cameraName); - aspectRatio = - (camera && camera?.detect.width / camera?.detect.height) || 16 / 9; - col = index % 3; // Regular cameras distributed across columns - } + const optionsMap: Layout[] = baseLayout + ? baseLayout.filter((layout) => cameraNames?.includes(layout.i)) + : []; - // Calculate layout options based on aspect ratio - const columnsPerPlayer = 4; - let height; - let width; + cameraNames.forEach((cameraName, index) => { + const existingLayout = optionsMap.find( + (layout) => layout.i === cameraName, + ); - if (aspectRatio < 1) { - // Portrait - height = 2 * columnsPerPlayer; - width = columnsPerPlayer; - } else if (aspectRatio > 2) { - // Wide - height = 1 * columnsPerPlayer; - width = 2 * columnsPerPlayer; - } else { - // Landscape - height = 1 * columnsPerPlayer; - width = columnsPerPlayer; - } + // Skip if the camera already exists in the layout + if (existingLayout) { + return; + } - const options = { - i: cameraName, - x: col * width, - y: 0, // don't set y, grid does automatically - w: width, - h: height, - }; + let aspectRatio; + let col; - optionsMap.push(options); - }); + // Handle "birdseye" camera as a special case + if (cameraName === "birdseye") { + aspectRatio = + (birdseyeConfig?.width || 1) / (birdseyeConfig?.height || 1); + col = 0; // Set birdseye camera in the first column + } else { + const camera = cameras.find((cam) => cam.name === cameraName); + aspectRatio = + (camera && camera?.detect.width / camera?.detect.height) || 16 / 9; + col = index % 3; // Regular cameras distributed across columns + } - return optionsMap; - }, [ - cameras, - isGridLayoutLoaded, - currentGridLayout, - includeBirdseye, - birdseyeConfig, - ]); + // Calculate layout options based on aspect ratio + const columnsPerPlayer = 4; + let height; + let width; + + if (aspectRatio < 1) { + // Portrait + height = 2 * columnsPerPlayer; + width = columnsPerPlayer; + } else if (aspectRatio > 2) { + // Wide + height = 1 * columnsPerPlayer; + width = 2 * columnsPerPlayer; + } else { + // Landscape + height = 1 * columnsPerPlayer; + width = columnsPerPlayer; + } + + const options = { + i: cameraName, + x: col * width, + y: 0, // don't set y, grid does automatically + w: width, + h: height, + }; + + optionsMap.push(options); + }); + + return optionsMap; + }, + [cameras, isGridLayoutLoaded, includeBirdseye, birdseyeConfig], + ); useEffect(() => { if (isGridLayoutLoaded) { if (gridLayout) { - // set current grid layout from loaded - setCurrentGridLayout(gridLayout); + // set current grid layout from loaded, possibly adding new cameras + const updatedLayout = generateLayout(gridLayout); + setCurrentGridLayout(updatedLayout); + // Only save if cameras were added (layout changed) + if (!isEqual(updatedLayout, gridLayout)) { + setGridLayout(updatedLayout); + } + // Set camera tracking state so the camera-change effect has a baseline + setCurrentCameras(cameras); + setCurrentIncludeBirdseye(includeBirdseye); } else { // idb is empty, set it with an initial layout - setGridLayout(generateLayout()); + const newLayout = generateLayout(undefined); + setCurrentGridLayout(newLayout); + setGridLayout(newLayout); + setCurrentCameras(cameras); + setCurrentIncludeBirdseye(includeBirdseye); } } }, [ - isEditMode, gridLayout, - currentGridLayout, setGridLayout, isGridLayoutLoaded, generateLayout, + cameras, + includeBirdseye, ]); useEffect(() => { + // Only regenerate layout when cameras change WITHIN an already-loaded group + // Skip if currentCameras is undefined (means we just switched groups and + // the first useEffect hasn't run yet to set things up) + if (!isGridLayoutLoaded || currentCameras === undefined) { + return; + } + if ( !isEqual(cameras, currentCameras) || includeBirdseye !== currentIncludeBirdseye @@ -273,15 +294,17 @@ export default function DraggableGridLayout({ setCurrentCameras(cameras); setCurrentIncludeBirdseye(includeBirdseye); - // set new grid layout in idb - setGridLayout(generateLayout()); + // Regenerate layout based on current layout, adding any new cameras + const updatedLayout = generateLayout(currentGridLayout); + setCurrentGridLayout(updatedLayout); + setGridLayout(updatedLayout); } }, [ cameras, includeBirdseye, currentCameras, currentIncludeBirdseye, - setCurrentGridLayout, + currentGridLayout, generateLayout, setGridLayout, isGridLayoutLoaded, diff --git a/web/src/views/settings/UiSettingsView.tsx b/web/src/views/settings/UiSettingsView.tsx index a2293685e..717e3a3af 100644 --- a/web/src/views/settings/UiSettingsView.tsx +++ b/web/src/views/settings/UiSettingsView.tsx @@ -1,15 +1,17 @@ import Heading from "@/components/ui/heading"; import { Label } from "@/components/ui/label"; import { Switch } from "@/components/ui/switch"; -import { useCallback, useEffect } from "react"; +import { useCallback, useContext, useEffect } from "react"; import { Toaster } from "sonner"; import { toast } from "sonner"; import { Separator } from "../../components/ui/separator"; import { Button } from "../../components/ui/button"; import useSWR from "swr"; import { FrigateConfig } from "@/types/frigateConfig"; -import { del as delData } from "idb-keyval"; -import { useUserPersistence } from "@/hooks/use-user-persistence"; +import { + useUserPersistence, + deleteUserNamespacedKey, +} from "@/hooks/use-user-persistence"; import { isSafari } from "react-device-detect"; import { Select, @@ -19,6 +21,7 @@ import { SelectTrigger, } from "../../components/ui/select"; import { useTranslation } from "react-i18next"; +import { AuthContext } from "@/context/auth-context"; const PLAYBACK_RATE_DEFAULT = isSafari ? [0.5, 1, 2] : [0.5, 1, 2, 4, 8, 16]; const WEEK_STARTS_ON = ["Sunday", "Monday"]; @@ -26,13 +29,16 @@ const WEEK_STARTS_ON = ["Sunday", "Monday"]; export default function UiSettingsView() { const { data: config } = useSWR("config"); const { t } = useTranslation("views/settings"); + const { auth } = useContext(AuthContext); + const username = auth?.user?.username; + const clearStoredLayouts = useCallback(() => { if (!config) { return []; } Object.entries(config.camera_groups).forEach(async (value) => { - await delData(`${value[0]}-draggable-layout`) + await deleteUserNamespacedKey(`${value[0]}-draggable-layout`, username) .then(() => { toast.success( t("general.toast.success.clearStoredLayout", { @@ -56,14 +62,14 @@ export default function UiSettingsView() { ); }); }); - }, [config, t]); + }, [config, t, username]); const clearStreamingSettings = useCallback(async () => { if (!config) { return []; } - await delData(`streaming-settings`) + await deleteUserNamespacedKey(`streaming-settings`, username) .then(() => { toast.success(t("general.toast.success.clearStreamingSettings"), { position: "top-center", @@ -83,7 +89,7 @@ export default function UiSettingsView() { }, ); }); - }, [config, t]); + }, [config, t, username]); useEffect(() => { document.title = t("documentTitle.general");