diff --git a/site/src/components/HelpTooltip/HelpTooltip.tsx b/site/src/components/HelpTooltip/HelpTooltip.tsx index 31e2fe1fb260d..ffa4aa61fe6a3 100644 --- a/site/src/components/HelpTooltip/HelpTooltip.tsx +++ b/site/src/components/HelpTooltip/HelpTooltip.tsx @@ -160,7 +160,7 @@ export const HelpTooltipAction: FC = ({ onClick={(event) => { event.stopPropagation(); onClick(); - popover.setIsOpen(false); + popover.setOpen(false); }} > diff --git a/site/src/components/IconField/IconField.tsx b/site/src/components/IconField/IconField.tsx index b2bf348d4d532..3690a78983195 100644 --- a/site/src/components/IconField/IconField.tsx +++ b/site/src/components/IconField/IconField.tsx @@ -3,7 +3,7 @@ import Button from "@mui/material/Button"; import InputAdornment from "@mui/material/InputAdornment"; import TextField, { type TextFieldProps } from "@mui/material/TextField"; import { visuallyHidden } from "@mui/utils"; -import { type FC, lazy, Suspense } from "react"; +import { type FC, lazy, Suspense, useState } from "react"; import { DropdownArrow } from "components/DropdownArrow/DropdownArrow"; import { ExternalImage } from "components/ExternalImage/ExternalImage"; import { Loader } from "components/Loader/Loader"; @@ -37,6 +37,7 @@ export const IconField: FC = ({ const theme = useTheme(); const hasIcon = textFieldProps.value && textFieldProps.value !== ""; + const [open, setOpen] = useState(false); return ( @@ -86,31 +87,26 @@ export const IconField: FC = ({ } `} /> - - {(popover) => ( - <> - - - - - }> - { - const value = - emoji.src ?? urlFromUnifiedCode(emoji.unified); - onPickEmoji(value); - popover.setIsOpen(false); - }} - /> - - - - )} + + + + + + }> + { + const value = emoji.src ?? urlFromUnifiedCode(emoji.unified); + onPickEmoji(value); + setOpen(false); + }} + /> + + {/* diff --git a/site/src/components/Popover/Popover.tsx b/site/src/components/Popover/Popover.tsx index 1ed4e90a73995..ffb56fd5d7349 100644 --- a/site/src/components/Popover/Popover.tsx +++ b/site/src/components/Popover/Popover.tsx @@ -12,7 +12,6 @@ import { type ReactNode, type RefObject, useContext, - useEffect, useId, useRef, useState, @@ -25,14 +24,12 @@ type TriggerRef = RefObject; type TriggerElement = ReactElement<{ ref: TriggerRef; onClick?: () => void; - "aria-haspopup"?: boolean; - "aria-owns"?: string | undefined; }>; type PopoverContextValue = { id: string; - isOpen: boolean; - setIsOpen: React.Dispatch>; + open: boolean; + setOpen: (open: boolean) => void; triggerRef: TriggerRef; mode: TriggerMode; }; @@ -41,32 +38,41 @@ const PopoverContext = createContext( undefined, ); -export interface PopoverProps { - children: ReactNode | ((popover: PopoverContextValue) => ReactNode); // Allows inline usage +type BasePopoverProps = { + children: ReactNode; mode?: TriggerMode; - isDefaultOpen?: boolean; -} +}; -export const Popover: FC = ({ - children, - mode, - isDefaultOpen, -}) => { +// By separating controlled and uncontrolled props, we achieve more accurate +// type inference. +type UncontrolledPopoverProps = BasePopoverProps & { + open?: undefined; + onOpenChange?: undefined; +}; + +type ControlledPopoverProps = BasePopoverProps & { + open: boolean; + onOpenChange: (open: boolean) => void; +}; + +export type PopoverProps = UncontrolledPopoverProps | ControlledPopoverProps; + +export const Popover: FC = (props) => { const hookId = useId(); - const [isOpen, setIsOpen] = useState(isDefaultOpen ?? false); - const triggerRef = useRef(null); + const [uncontrolledOpen, setUncontrolledOpen] = useState(false); + const triggerRef: TriggerRef = useRef(null); const value: PopoverContextValue = { - isOpen, - setIsOpen, triggerRef, id: `${hookId}-popover`, - mode: mode ?? "click", + mode: props.mode ?? "click", + open: props.open ?? uncontrolledOpen, + setOpen: props.onOpenChange ?? setUncontrolledOpen, }; return ( - {typeof children === "function" ? children(value) : children} + {props.children} ); }; @@ -82,23 +88,25 @@ export const usePopover = () => { }; export const PopoverTrigger = ( - props: HTMLAttributes & { children: TriggerElement }, + props: HTMLAttributes & { + children: TriggerElement; + }, ) => { const popover = usePopover(); const { children, ...elementProps } = props; const clickProps = { onClick: () => { - popover.setIsOpen((isOpen) => !isOpen); + popover.setOpen(true); }, }; const hoverProps = { onPointerEnter: () => { - popover.setIsOpen(true); + popover.setOpen(true); }, onPointerLeave: () => { - popover.setIsOpen(false); + popover.setOpen(false); }, }; @@ -106,7 +114,8 @@ export const PopoverTrigger = ( ...elementProps, ...(popover.mode === "click" ? clickProps : hoverProps), "aria-haspopup": true, - "aria-owns": popover.isOpen ? popover.id : undefined, + "aria-owns": popover.id, + "aria-expanded": popover.open, ref: popover.triggerRef, }); }; @@ -125,22 +134,8 @@ export const PopoverContent: FC = ({ ...popoverProps }) => { const popover = usePopover(); - const [isReady, setIsReady] = useState(false); const hoverMode = popover.mode === "hover"; - // This is a hack to make sure the popover is not rendered until the trigger - // is ready. This is a limitation on MUI that does not support defaultIsOpen - // on Popover but we need it to storybook the component. - useEffect(() => { - if (!isReady && popover.triggerRef.current !== null) { - setIsReady(true); - } - }, [isReady, popover.triggerRef]); - - if (!popover.triggerRef.current) { - return null; - } - return ( = ({ {...modeProps(popover)} {...popoverProps} id={popover.id} - open={popover.isOpen} - onClose={() => popover.setIsOpen(false)} + open={popover.open} + onClose={() => popover.setOpen(false)} anchorEl={popover.triggerRef.current} /> ); @@ -172,10 +167,10 @@ const modeProps = (popover: PopoverContextValue) => { if (popover.mode === "hover") { return { onPointerEnter: () => { - popover.setIsOpen(true); + popover.setOpen(true); }, onPointerLeave: () => { - popover.setIsOpen(false); + popover.setOpen(false); }, }; } diff --git a/site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx b/site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx index e54210d831d8e..4a79bc00c1931 100644 --- a/site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx +++ b/site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx @@ -87,7 +87,7 @@ const DeploymentDropdownContent: FC = ({ }) => { const popover = usePopover(); - const onPopoverClose = () => popover.setIsOpen(false); + const onPopoverClose = () => popover.setOpen(false); return (