Apply review suggestions and fix lint

BrunoQuaresma · BrunoQuaresma · commit 26951f598aa0 · 2024-06-21T14:02:30.000Z
diff --git a/site/src/components/HelpTooltip/HelpTooltip.tsx b/site/src/components/HelpTooltip/HelpTooltip.tsx
@@ -160,7 +160,7 @@ export const HelpTooltipAction: FC<HelpTooltipActionProps> = ({
       onClick={(event) => {
         event.stopPropagation();
         onClick();
-        popover.setIsOpen(false);
+        popover.setOpen(false);
       }}
     >
       <Icon css={styles.actionIcon} />
diff --git a/site/src/components/Popover/Popover.tsx b/site/src/components/Popover/Popover.tsx
@@ -43,6 +43,8 @@ type BasePopoverProps = {
   mode?: TriggerMode;
 };
 
+// By separating controlled and uncontrolled props, we achieve more accurate
+// type inference.
 type UncontrolledPopoverProps = BasePopoverProps & {
   open?: undefined;
   onOpenChange?: undefined;
@@ -57,16 +59,15 @@ export type PopoverProps = UncontrolledPopoverProps | ControlledPopoverProps;
 
 export const Popover: FC<PopoverProps> = (props) => {
   const hookId = useId();
-  const [open, setOpen] = useState(false);
+  const [uncontrolledOpen, setUncontrolledOpen] = useState(false);
   const triggerRef: TriggerRef = useRef(null);
-  const isControlled = props.open !== undefined;
 
   const value: PopoverContextValue = {
     triggerRef,
     id: `${hookId}-popover`,
     mode: props.mode ?? "click",
-    open: isControlled ? props.open : open,
-    setOpen: isControlled ? props.onOpenChange : setOpen,
+    open: props.open ?? uncontrolledOpen,
+    setOpen: props.onOpenChange ?? setUncontrolledOpen,
   };
 
   return (
@@ -113,7 +114,8 @@ export const PopoverTrigger = (
     ...elementProps,
     ...(popover.mode === "click" ? clickProps : hoverProps),
     "aria-haspopup": true,
-    "aria-owns": popover.open ? popover.id : undefined,
+    "aria-owns": popover.id,
+    "aria-expanded": popover.open,
     ref: popover.triggerRef,
   });
 };
diff --git a/site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx b/site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx
@@ -87,7 +87,7 @@ const DeploymentDropdownContent: FC<DeploymentDropdownProps> = ({
 }) => {
   const popover = usePopover();
 
-  const onPopoverClose = () => popover.setIsOpen(false);
+  const onPopoverClose = () => popover.setOpen(false);
 
   return (
     <nav>
diff --git a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx
@@ -56,7 +56,7 @@ export const WorkspaceOutdatedTooltipContent: FC<TooltipProps> = ({
   const popover = usePopover();
   const { data: activeVersion } = useQuery({
     ...templateVersion(latestVersionId),
-    enabled: popover.isOpen,
+    enabled: popover.open,
   });
   const theme = useTheme();
 
diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/BuildParametersPopover.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/BuildParametersPopover.tsx
@@ -117,7 +117,7 @@ const BuildParametersPopoverContent: FC<BuildParametersPopoverContentProps> = ({
               <Form
                 onSubmit={(buildParameters) => {
                   onSubmit(buildParameters);
-                  popover.setIsOpen(false);
+                  popover.setOpen(false);
                 }}
                 ephemeralParameters={ephemeralParameters}
                 buildParameters={buildParameters.map(
diff --git a/site/src/pages/WorkspacePage/WorkspaceNotifications/Notifications.tsx b/site/src/pages/WorkspacePage/WorkspaceNotifications/Notifications.tsx
@@ -70,7 +70,7 @@ const NotificationPill: FC<NotificationsProps> = ({
       icon={icon}
       css={(theme) => ({
         "& svg": { color: theme.roles[severity].outline },
-        borderColor: popover.isOpen ? theme.roles[severity].outline : undefined,
+        borderColor: popover.open ? theme.roles[severity].outline : undefined,
       })}
     >
       {items.length}

Original file line number	Diff line number	Diff line change
`@@ -160,7 +160,7 @@ export const HelpTooltipAction: FC<HelpTooltipActionProps> = ({`
`160`	`160`	`onClick={(event) => {`
`161`	`161`	`event.stopPropagation();`
`162`	`162`	`onClick();`
`163`		`- popover.setIsOpen(false);`
	`163`	`+ popover.setOpen(false);`
`164`	`164`	`}}`
`165`	`165`	`>`
`166`	`166`	`<Icon css={styles.actionIcon} />`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ const NotificationPill: FC<NotificationsProps> = ({`
`70`	`70`	`icon={icon}`
`71`	`71`	`css={(theme) => ({`
`72`	`72`	`"& svg": { color: theme.roles[severity].outline },`
`73`		`- borderColor: popover.isOpen ? theme.roles[severity].outline : undefined,`
	`73`	`+ borderColor: popover.open ? theme.roles[severity].outline : undefined,`
`74`	`74`	`})}`
`75`	`75`	`>`
`76`	`76`	`{items.length}`