From ae30ac6e3cbfe6a08e33c03c8a08cda7dacd0978 Mon Sep 17 00:00:00 2001
From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com>
Date: Fri, 8 Nov 2024 07:45:00 -0600
Subject: [PATCH] Refactor general review filter to only call the update
 function once (#14866)

---
 .../components/filter/ReviewFilterGroup.tsx   | 190 +++++++++---------
 .../overlay/MobileReviewSettingsDrawer.tsx    |  38 ++--
 web/src/types/filter.ts                       |   6 +
 3 files changed, 117 insertions(+), 117 deletions(-)

diff --git a/web/src/components/filter/ReviewFilterGroup.tsx b/web/src/components/filter/ReviewFilterGroup.tsx
index 004fe5527..bea816203 100644
--- a/web/src/components/filter/ReviewFilterGroup.tsx
+++ b/web/src/components/filter/ReviewFilterGroup.tsx
@@ -14,7 +14,7 @@ import MobileReviewSettingsDrawer, {
 } from "../overlay/MobileReviewSettingsDrawer";
 import useOptimisticState from "@/hooks/use-optimistic-state";
 import FilterSwitch from "./FilterSwitch";
-import { FilterList } from "@/types/filter";
+import { FilterList, GeneralFilter } from "@/types/filter";
 import CalendarFilterButton from "./CalendarFilterButton";
 import { CamerasFilterButton } from "./CamerasFilterButton";
 import PlatformAwareDialog from "../overlay/dialog/PlatformAwareDialog";
@@ -214,15 +214,7 @@ export default function ReviewFilterGroup({
           showAll={filter?.showAll == true}
           allZones={filterValues.zones}
           selectedZones={filter?.zones}
-          setShowAll={(showAll) => {
-            onUpdateFilter({ ...filter, showAll });
-          }}
-          updateLabelFilter={(newLabels) => {
-            onUpdateFilter({ ...filter, labels: newLabels });
-          }}
-          updateZoneFilter={(newZones) =>
-            onUpdateFilter({ ...filter, zones: newZones })
-          }
+          onUpdateFilter={onUpdateFilter}
         />
       )}
       {isMobile && mobileSettingsFeatures.length > 0 && (
@@ -300,37 +292,40 @@ type GeneralFilterButtonProps = {
   showAll: boolean;
   allZones: string[];
   selectedZones?: string[];
-  setShowAll: (showAll: boolean) => void;
-  updateLabelFilter: (labels: string[] | undefined) => void;
-  updateZoneFilter: (zones: string[] | undefined) => void;
+  filter?: GeneralFilter;
+  onUpdateFilter: (filter: ReviewFilter) => void;
 };
+
 function GeneralFilterButton({
   allLabels,
   selectedLabels,
+  filter,
   currentSeverity,
   showAll,
   allZones,
   selectedZones,
-  setShowAll,
-  updateLabelFilter,
-  updateZoneFilter,
+  onUpdateFilter,
 }: GeneralFilterButtonProps) {
   const [open, setOpen] = useState(false);
-  const [currentLabels, setCurrentLabels] = useState<string[] | undefined>(
-    selectedLabels,
-  );
-  const [currentZones, setCurrentZones] = useState<string[] | undefined>(
-    selectedZones,
-  );
+  const [currentFilter, setCurrentFilter] = useState<GeneralFilter>({
+    labels: selectedLabels,
+    zones: selectedZones,
+    showAll: showAll,
+    ...filter,
+  });
 
-  // ui
+  // Update local state when props change
 
   useEffect(() => {
-    setCurrentLabels(selectedLabels);
-    setCurrentZones(selectedZones);
+    setCurrentFilter({
+      labels: selectedLabels,
+      zones: selectedZones,
+      showAll: showAll,
+      ...filter,
+    });
     // only refresh when state changes
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [selectedLabels, selectedZones]);
+  }, [selectedLabels, selectedZones, showAll, filter]);
 
   const trigger = (
     <Button
@@ -342,10 +337,18 @@ function GeneralFilterButton({
       aria-label="Filter"
     >
       <FaFilter
-        className={`${selectedLabels?.length || selectedZones?.length ? "text-selected-foreground" : "text-secondary-foreground"}`}
+        className={`${
+          selectedLabels?.length || selectedZones?.length
+            ? "text-selected-foreground"
+            : "text-secondary-foreground"
+        }`}
       />
       <div
-        className={`hidden md:block ${selectedLabels?.length || selectedZones?.length ? "text-selected-foreground" : "text-primary"}`}
+        className={`hidden md:block ${
+          selectedLabels?.length || selectedZones?.length
+            ? "text-selected-foreground"
+            : "text-primary"
+        }`}
       >
         Filter
       </div>
@@ -355,17 +358,22 @@ function GeneralFilterButton({
     <GeneralFilterContent
       allLabels={allLabels}
       selectedLabels={selectedLabels}
-      currentLabels={currentLabels}
       currentSeverity={currentSeverity}
-      showAll={showAll}
       allZones={allZones}
+      filter={currentFilter}
       selectedZones={selectedZones}
-      currentZones={currentZones}
-      setCurrentZones={setCurrentZones}
-      updateZoneFilter={updateZoneFilter}
-      setShowAll={setShowAll}
-      updateLabelFilter={updateLabelFilter}
-      setCurrentLabels={setCurrentLabels}
+      onUpdateFilter={setCurrentFilter}
+      onApply={() => {
+        if (currentFilter !== filter) {
+          onUpdateFilter(currentFilter);
+        }
+        setOpen(false);
+      }}
+      onReset={() => {
+        const resetFilter: GeneralFilter = {};
+        setCurrentFilter(resetFilter);
+        onUpdateFilter(resetFilter);
+      }}
       onClose={() => setOpen(false)}
     />
   );
@@ -377,7 +385,12 @@ function GeneralFilterButton({
       open={open}
       onOpenChange={(open) => {
         if (!open) {
-          setCurrentLabels(selectedLabels);
+          setCurrentFilter({
+            labels: selectedLabels,
+            zones: selectedZones,
+            showAll: showAll,
+            ...filter,
+          });
         }
 
         setOpen(open);
@@ -388,54 +401,50 @@ function GeneralFilterButton({
 
 type GeneralFilterContentProps = {
   allLabels: string[];
-  selectedLabels: string[] | undefined;
-  currentLabels: string[] | undefined;
+  allZones: string[];
   currentSeverity?: ReviewSeverity;
-  showAll?: boolean;
-  allZones?: string[];
+  filter: GeneralFilter;
+  selectedLabels?: string[];
   selectedZones?: string[];
-  currentZones?: string[];
-  setShowAll?: (showAll: boolean) => void;
-  updateLabelFilter: (labels: string[] | undefined) => void;
-  setCurrentLabels: (labels: string[] | undefined) => void;
-  updateZoneFilter?: (zones: string[] | undefined) => void;
-  setCurrentZones?: (zones: string[] | undefined) => void;
+  onUpdateFilter: (filter: GeneralFilter) => void;
+  onApply: () => void;
+  onReset: () => void;
   onClose: () => void;
 };
 export function GeneralFilterContent({
   allLabels,
-  selectedLabels,
-  currentLabels,
-  currentSeverity,
-  showAll,
   allZones,
-  selectedZones,
-  currentZones,
-  setShowAll,
-  updateLabelFilter,
-  setCurrentLabels,
-  updateZoneFilter,
-  setCurrentZones,
+  currentSeverity,
+  filter,
+  onUpdateFilter,
+  onApply,
+  onReset,
   onClose,
 }: GeneralFilterContentProps) {
   return (
     <>
       <div className="scrollbar-container h-auto max-h-[80dvh] overflow-y-auto overflow-x-hidden">
-        {currentSeverity && setShowAll && (
+        {currentSeverity && (
           <div className="my-2.5 flex flex-col gap-2.5">
             <FilterSwitch
               label="Alerts"
               disabled={currentSeverity == "alert"}
-              isChecked={currentSeverity == "alert" ? true : showAll == true}
-              onCheckedChange={setShowAll}
+              isChecked={
+                currentSeverity == "alert" ? true : filter.showAll === true
+              }
+              onCheckedChange={(checked) =>
+                onUpdateFilter({ ...filter, showAll: checked })
+              }
             />
             <FilterSwitch
               label="Detections"
               disabled={currentSeverity == "detection"}
               isChecked={
-                currentSeverity == "detection" ? true : showAll == true
+                currentSeverity == "detection" ? true : filter.showAll === true
+              }
+              onCheckedChange={(checked) =>
+                onUpdateFilter({ ...filter, showAll: checked })
               }
-              onCheckedChange={setShowAll}
             />
             <DropdownMenuSeparator />
           </div>
@@ -450,10 +459,11 @@ export function GeneralFilterContent({
           <Switch
             className="ml-1"
             id="allLabels"
-            checked={currentLabels == undefined}
+            checked={filter.labels === undefined}
             onCheckedChange={(isChecked) => {
               if (isChecked) {
-                setCurrentLabels(undefined);
+                const { labels: _labels, ...rest } = filter;
+                onUpdateFilter(rest);
               }
             }}
           />
@@ -463,20 +473,19 @@ export function GeneralFilterContent({
             <FilterSwitch
               key={item}
               label={item.replaceAll("_", " ")}
-              isChecked={currentLabels?.includes(item) ?? false}
+              isChecked={filter.labels?.includes(item) ?? false}
               onCheckedChange={(isChecked) => {
                 if (isChecked) {
-                  const updatedLabels = currentLabels ? [...currentLabels] : [];
-
+                  const updatedLabels = filter.labels ? [...filter.labels] : [];
                   updatedLabels.push(item);
-                  setCurrentLabels(updatedLabels);
+                  onUpdateFilter({ ...filter, labels: updatedLabels });
                 } else {
-                  const updatedLabels = currentLabels ? [...currentLabels] : [];
+                  const updatedLabels = filter.labels ? [...filter.labels] : [];
 
                   // can not deselect the last item
                   if (updatedLabels.length > 1) {
                     updatedLabels.splice(updatedLabels.indexOf(item), 1);
-                    setCurrentLabels(updatedLabels);
+                    onUpdateFilter({ ...filter, labels: updatedLabels });
                   }
                 }
               }}
@@ -484,7 +493,7 @@ export function GeneralFilterContent({
           ))}
         </div>
 
-        {allZones && setCurrentZones && (
+        {allZones && (
           <>
             <DropdownMenuSeparator />
             <div className="mb-5 mt-2.5 flex items-center justify-between">
@@ -497,10 +506,11 @@ export function GeneralFilterContent({
               <Switch
                 className="ml-1"
                 id="allZones"
-                checked={currentZones == undefined}
+                checked={filter.zones === undefined}
                 onCheckedChange={(isChecked) => {
                   if (isChecked) {
-                    setCurrentZones(undefined);
+                    const { zones: _zones, ...rest } = filter;
+                    onUpdateFilter(rest);
                   }
                 }}
               />
@@ -510,24 +520,24 @@ export function GeneralFilterContent({
                 <FilterSwitch
                   key={item}
                   label={item.replaceAll("_", " ")}
-                  isChecked={currentZones?.includes(item) ?? false}
+                  isChecked={filter.zones?.includes(item) ?? false}
                   onCheckedChange={(isChecked) => {
                     if (isChecked) {
-                      const updatedZones = currentZones
-                        ? [...currentZones]
+                      const updatedZones = filter.zones
+                        ? [...filter.zones]
                         : [];
 
                       updatedZones.push(item);
-                      setCurrentZones(updatedZones);
+                      onUpdateFilter({ ...filter, zones: updatedZones });
                     } else {
-                      const updatedZones = currentZones
-                        ? [...currentZones]
+                      const updatedZones = filter.zones
+                        ? [...filter.zones]
                         : [];
 
                       // can not deselect the last item
                       if (updatedZones.length > 1) {
                         updatedZones.splice(updatedZones.indexOf(item), 1);
-                        setCurrentZones(updatedZones);
+                        onUpdateFilter({ ...filter, zones: updatedZones });
                       }
                     }
                   }}
@@ -543,27 +553,13 @@ export function GeneralFilterContent({
           aria-label="Apply"
           variant="select"
           onClick={() => {
-            if (selectedLabels != currentLabels) {
-              updateLabelFilter(currentLabels);
-            }
-
-            if (updateZoneFilter && selectedZones != currentZones) {
-              updateZoneFilter(currentZones);
-            }
-
+            onApply();
             onClose();
           }}
         >
           Apply
         </Button>
-        <Button
-          aria-label="Reset"
-          onClick={() => {
-            setCurrentLabels(undefined);
-            setCurrentZones?.(undefined);
-            updateLabelFilter(undefined);
-          }}
-        >
+        <Button aria-label="Reset" onClick={onReset}>
           Reset
         </Button>
       </div>
diff --git a/web/src/components/overlay/MobileReviewSettingsDrawer.tsx b/web/src/components/overlay/MobileReviewSettingsDrawer.tsx
index d58d485b9..3eeb639cd 100644
--- a/web/src/components/overlay/MobileReviewSettingsDrawer.tsx
+++ b/web/src/components/overlay/MobileReviewSettingsDrawer.tsx
@@ -4,7 +4,7 @@ import { Button } from "../ui/button";
 import { FaArrowDown, FaCalendarAlt, FaCog, FaFilter } from "react-icons/fa";
 import { TimeRange } from "@/types/timeline";
 import { ExportContent, ExportPreviewDialog } from "./ExportDialog";
-import { ExportMode } from "@/types/filter";
+import { ExportMode, GeneralFilter } from "@/types/filter";
 import ReviewActivityCalendar from "./ReviewActivityCalendar";
 import { SelectSeparator } from "../ui/select";
 import { ReviewFilter, ReviewSeverity, ReviewSummary } from "@/types/review";
@@ -114,12 +114,12 @@ export default function MobileReviewSettingsDrawer({
 
   // filters
 
-  const [currentLabels, setCurrentLabels] = useState<string[] | undefined>(
-    filter?.labels,
-  );
-  const [currentZones, setCurrentZones] = useState<string[] | undefined>(
-    filter?.zones,
-  );
+  const [currentFilter, setCurrentFilter] = useState<GeneralFilter>({
+    labels: filter?.labels,
+    zones: filter?.zones,
+    showAll: filter?.showAll,
+    ...filter,
+  });
 
   if (!isMobile) {
     return;
@@ -260,23 +260,21 @@ export default function MobileReviewSettingsDrawer({
         <GeneralFilterContent
           allLabels={allLabels}
           selectedLabels={filter?.labels}
-          currentLabels={currentLabels}
           currentSeverity={currentSeverity}
-          showAll={filter?.showAll == true}
           allZones={allZones}
+          filter={currentFilter}
           selectedZones={filter?.zones}
-          currentZones={currentZones}
-          setCurrentZones={setCurrentZones}
-          updateZoneFilter={(newZones) =>
-            onUpdateFilter({ ...filter, zones: newZones })
-          }
-          setShowAll={(showAll) => {
-            onUpdateFilter({ ...filter, showAll });
+          onUpdateFilter={setCurrentFilter}
+          onApply={() => {
+            if (currentFilter !== filter) {
+              onUpdateFilter(currentFilter);
+            }
+          }}
+          onReset={() => {
+            const resetFilter: GeneralFilter = {};
+            setCurrentFilter(resetFilter);
+            onUpdateFilter(resetFilter);
           }}
-          setCurrentLabels={setCurrentLabels}
-          updateLabelFilter={(newLabels) =>
-            onUpdateFilter({ ...filter, labels: newLabels })
-          }
           onClose={() => setDrawerMode("select")}
         />
       </div>
diff --git a/web/src/types/filter.ts b/web/src/types/filter.ts
index 09ff5b99a..b7d2223c0 100644
--- a/web/src/types/filter.ts
+++ b/web/src/types/filter.ts
@@ -10,3 +10,9 @@ export type FilterList = {
 };
 
 export const LAST_24_HOURS_KEY = "last24Hours";
+
+export type GeneralFilter = {
+  showAll?: boolean;
+  labels?: string[];
+  zones?: string[];
+};