chore: clean up base nav button

Parkreiner · Parkreiner · commit 4547519feeb0 · 2023-11-07T22:37:02.000Z
diff --git a/site/src/components/PaginationWidget/PaginationNavButtons.tsx b/site/src/components/PaginationWidget/PaginationNavButtons.tsx
@@ -74,6 +74,11 @@ function BaseNavButton({
   const theme = useTheme();
   const [showDisabledMessage, setShowDisabledMessage] = useState(false);
 
+  // Inline state sync - this is safe/recommended by the React team in this case
+  if (!disabled && showDisabledMessage) {
+    setShowDisabledMessage(false);
+  }
+
   useEffect(() => {
     if (!showDisabledMessage) {
       return;
@@ -84,19 +89,24 @@ function BaseNavButton({
       disabledMessageTimeout,
     );
 
-    return () => window.clearTimeout(timeoutId);
+    return () => {
+      setShowDisabledMessage(false);
+      window.clearTimeout(timeoutId);
+    };
   }, [showDisabledMessage, disabledMessageTimeout]);
 
-  // Using inline state sync to help MUI render accurately more quickly; same
-  // idea as the useEffect approach, but state changes flush faster
-  if (!disabled && showDisabledMessage) {
-    setShowDisabledMessage(false);
-  }
-
   return (
     <Tooltip title={disabledMessage} open={showDisabledMessage}>
+      {/*
+       * Going more out of the way to avoid attaching the disabled prop directly
+       * to avoid unwanted side effects of using the prop:
+       * - Not being focusable/keyboard-navigable
+       * - Not being able to call functions in response to invalid actions
+       *   (mostly for giving direct UI feedback to those actions)
+       */}
       <Button
         aria-label={ariaLabel}
+        aria-disabled={disabled}
         css={
           disabled && {
             borderColor: theme.palette.divider,
