From db64b54c79cdea35910557ca4aaee96dd4f7b916 Mon Sep 17 00:00:00 2001
From: Gregory Schier <gschier1990@gmail.com>
Date: Mon, 13 Jan 2025 16:59:39 -0800
Subject: [PATCH] Slight cleanup

---
 src-web/components/core/Dropdown.tsx         | 50 +++++++++-----------
 src-web/components/core/SegmentedControl.tsx |  5 +-
 2 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src-web/components/core/Dropdown.tsx b/src-web/components/core/Dropdown.tsx
index 558cdcf9..13cd4a72 100644
--- a/src-web/components/core/Dropdown.tsx
+++ b/src-web/components/core/Dropdown.tsx
@@ -12,11 +12,11 @@ import type {
   SetStateAction,
 } from 'react';
 import React, {
+  useEffect,
   Children,
   cloneElement,
   forwardRef,
   useCallback,
-  useEffect,
   useImperativeHandle,
   useMemo,
   useRef,
@@ -101,35 +101,29 @@ export const Dropdown = forwardRef<DropdownRef, DropdownProps>(function Dropdown
       setOpenId((prevId) => {
         const prevIsOpen = prevId === id;
         const newIsOpen = typeof o === 'function' ? o(prevIsOpen) : o;
-
-        if (newIsOpen) {
-          onOpen?.();
-
-          // Persist background color of button until we close the dropdown
-          buttonRef.current!.style.backgroundColor = window
-            .getComputedStyle(buttonRef.current!)
-            .getPropertyValue('background-color');
-        } else {
-          onClose?.();
-
-          // Reset background color of button
-          buttonRef.current!.style.backgroundColor = '';
-        }
-
-        // Set to different value when opened and closed to force it to update. This is to force
-        // <Menu/> to reset its selected-index state, which it does when this prop changes
-        setDefaultSelectedIndex(newIsOpen ? -1 : null);
-
         return newIsOpen ? id : null; // Set global atom to current ID to signify open state
       });
     },
-    [id, onClose, onOpen, setOpenId],
+    [id, setOpenId],
   );
 
-  const handleClose = useCallback(() => {
-    setIsOpen(false);
-    buttonRef.current?.focus();
-  }, [setIsOpen]);
+  useEffect(() => {
+    if (isOpen) {
+      // Persist background color of button until we close the dropdown
+      buttonRef.current!.style.backgroundColor = window
+        .getComputedStyle(buttonRef.current!)
+        .getPropertyValue('background-color');
+      onOpen?.();
+    } else {
+      onClose?.();
+      buttonRef.current?.focus(); // Focus button
+      buttonRef.current!.style.backgroundColor = ''; // Clear persisted BG
+    }
+
+    // Set to different value when opened and closed to force it to update. This is to force
+    // <Menu/> to reset its selected-index state, which it does when this prop changes
+    setDefaultSelectedIndex(isOpen ? -1 : null);
+  }, [isOpen, onClose, onOpen]);
 
   // Pull into variable so linter forces us to add it as a hook dep to useImperativeHandle. If we don't,
   // the ref will not update when menuRef updates, causing stale callback state to be used.
@@ -148,10 +142,10 @@ export const Dropdown = forwardRef<DropdownRef, DropdownProps>(function Dropdown
         setIsOpen(true);
       },
       close() {
-        handleClose();
+        setIsOpen(false);
       },
     }),
-    [handleClose, isOpen, setIsOpen, menuRefCurrent],
+    [isOpen, setIsOpen, menuRefCurrent],
   );
 
   useHotKey(hotKeyAction ?? null, () => {
@@ -199,7 +193,7 @@ export const Dropdown = forwardRef<DropdownRef, DropdownProps>(function Dropdown
         defaultSelectedIndex={defaultSelectedIndex}
         items={items}
         triggerShape={triggerRect ?? null}
-        onClose={handleClose}
+        onClose={() => setIsOpen(false)}
         isOpen={isOpen}
       />
     </>
diff --git a/src-web/components/core/SegmentedControl.tsx b/src-web/components/core/SegmentedControl.tsx
index 10f28e06..3165d956 100644
--- a/src-web/components/core/SegmentedControl.tsx
+++ b/src-web/components/core/SegmentedControl.tsx
@@ -44,9 +44,8 @@ export function SegmentedControl<T extends string>({ value, onChange, options, n
         return (
           <IconButton
             size="xs"
-            variant={isActive ? "border" : "solid"}
-            // color={isActive ? "secondary": "default"}
-            color={isActive ? "default" : "default"}
+            variant="solid"
+            color={isActive ? "secondary" : "default"}
             role="radio"
             event={{ id: name, value: String(o.value) }}
             tabIndex={isSelected ? 0 : -1}