Skip to content

chore(site): refactor popover to make it easier to extend #13611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion site/src/components/HelpTooltip/HelpTooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export const HelpTooltipAction: FC<HelpTooltipActionProps> = ({
onClick={(event) => {
event.stopPropagation();
onClick();
popover.setIsOpen(false);
popover.setOpen(false);
}}
>
<Icon css={styles.actionIcon} />
Expand Down
48 changes: 22 additions & 26 deletions site/src/components/IconField/IconField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -37,6 +37,7 @@ export const IconField: FC<IconFieldProps> = ({

const theme = useTheme();
const hasIcon = textFieldProps.value && textFieldProps.value !== "";
const [open, setOpen] = useState(false);

return (
<Stack spacing={1}>
Expand Down Expand Up @@ -86,31 +87,26 @@ export const IconField: FC<IconFieldProps> = ({
}
`}
/>
<Popover>
{(popover) => (
<>
<PopoverTrigger>
<Button fullWidth endIcon={<DropdownArrow />}>
Select emoji
</Button>
</PopoverTrigger>
<PopoverContent
id="emoji"
css={{ marginTop: 0, ".MuiPaper-root": { width: "auto" } }}
>
<Suspense fallback={<Loader />}>
<EmojiPicker
onEmojiSelect={(emoji) => {
const value =
emoji.src ?? urlFromUnifiedCode(emoji.unified);
onPickEmoji(value);
popover.setIsOpen(false);
}}
/>
</Suspense>
</PopoverContent>
</>
)}
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger>
<Button fullWidth endIcon={<DropdownArrow />}>
Select emoji
</Button>
</PopoverTrigger>
<PopoverContent
id="emoji"
css={{ marginTop: 0, ".MuiPaper-root": { width: "auto" } }}
>
<Suspense fallback={<Loader />}>
<EmojiPicker
onEmojiSelect={(emoji) => {
const value = emoji.src ?? urlFromUnifiedCode(emoji.unified);
onPickEmoji(value);
setOpen(false);
}}
/>
</Suspense>
</PopoverContent>
</Popover>

{/*
Expand Down
81 changes: 38 additions & 43 deletions site/src/components/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
type ReactNode,
type RefObject,
useContext,
useEffect,
useId,
useRef,
useState,
Expand All @@ -25,14 +24,12 @@ type TriggerRef = RefObject<HTMLElement>;
type TriggerElement = ReactElement<{
ref: TriggerRef;
onClick?: () => void;
"aria-haspopup"?: boolean;
"aria-owns"?: string | undefined;
}>;

type PopoverContextValue = {
id: string;
isOpen: boolean;
setIsOpen: React.Dispatch<React.SetStateAction<boolean>>;
open: boolean;
setOpen: (open: boolean) => void;
triggerRef: TriggerRef;
mode: TriggerMode;
};
Expand All @@ -41,32 +38,41 @@ const PopoverContext = createContext<PopoverContextValue | undefined>(
undefined,
);

export interface PopoverProps {
children: ReactNode | ((popover: PopoverContextValue) => ReactNode); // Allows inline usage
type BasePopoverProps = {
children: ReactNode;
mode?: TriggerMode;
isDefaultOpen?: boolean;
}
};

export const Popover: FC<PopoverProps> = ({
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<PopoverProps> = (props) => {
const hookId = useId();
const [isOpen, setIsOpen] = useState(isDefaultOpen ?? false);
const triggerRef = useRef<HTMLElement>(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 (
<PopoverContext.Provider value={value}>
{typeof children === "function" ? children(value) : children}
{props.children}
</PopoverContext.Provider>
);
};
Expand All @@ -82,31 +88,34 @@ export const usePopover = () => {
};

export const PopoverTrigger = (
props: HTMLAttributes<HTMLElement> & { children: TriggerElement },
props: HTMLAttributes<HTMLElement> & {
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);
},
};

return cloneElement(props.children, {
...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,
});
};
Expand All @@ -125,22 +134,8 @@ export const PopoverContent: FC<PopoverContentProps> = ({
...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 (
<MuiPopover
disablePortal
Expand All @@ -161,8 +156,8 @@ export const PopoverContent: FC<PopoverContentProps> = ({
{...modeProps(popover)}
{...popoverProps}
id={popover.id}
open={popover.isOpen}
onClose={() => popover.setIsOpen(false)}
open={popover.open}
onClose={() => popover.setOpen(false)}
anchorEl={popover.triggerRef.current}
/>
);
Expand All @@ -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);
},
};
}
Expand Down
2 changes: 1 addition & 1 deletion site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const DeploymentDropdownContent: FC<DeploymentDropdownProps> = ({
}) => {
const popover = usePopover();

const onPopoverClose = () => popover.setIsOpen(false);
const onPopoverClose = () => popover.setOpen(false);

return (
<nav>
Expand Down
7 changes: 2 additions & 5 deletions site/src/modules/dashboard/Navbar/ProxyMenu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
MockUser,
MockWorkspaceProxies,
} from "testHelpers/entities";
import { withDesktopViewport } from "testHelpers/storybook";
import { ProxyMenu } from "./ProxyMenu";

const defaultProxyContextValue = {
Expand All @@ -36,11 +37,7 @@ const meta: Meta<typeof ProxyMenu> = {
<Story />
</AuthProvider>
),
(Story) => (
<div css={{ width: 1200, height: 800 }}>
<Story />
</div>
),
withDesktopViewport,
],
parameters: {
queries: [
Expand Down
77 changes: 37 additions & 40 deletions site/src/modules/dashboard/Navbar/UserDropdown/UserDropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { css, type Interpolation, type Theme, useTheme } from "@emotion/react";
import Badge from "@mui/material/Badge";
import type { FC } from "react";
import { useState, type FC } from "react";
import type * as TypesGen from "api/typesGenerated";
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
import {
Expand All @@ -26,48 +26,45 @@ export const UserDropdown: FC<UserDropdownProps> = ({
onSignOut,
}) => {
const theme = useTheme();
const [open, setOpen] = useState(false);

return (
<Popover>
{(popover) => (
<>
<PopoverTrigger>
<button css={styles.button} data-testid="user-dropdown-trigger">
<div css={styles.badgeContainer}>
<Badge overlap="circular">
<UserAvatar
css={styles.avatar}
username={user.username}
avatarURL={user.avatar_url}
/>
</Badge>
<DropdownArrow
color={theme.experimental.l2.fill.solid}
close={popover.isOpen}
/>
</div>
</button>
</PopoverTrigger>

<PopoverContent
horizontal="right"
css={{
".MuiPaper-root": {
minWidth: "auto",
width: 260,
boxShadow: theme.shadows[6],
},
}}
>
<UserDropdownContent
user={user}
buildInfo={buildInfo}
supportLinks={supportLinks}
onSignOut={onSignOut}
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger>
<button css={styles.button} data-testid="user-dropdown-trigger">
<div css={styles.badgeContainer}>
<Badge overlap="circular">
<UserAvatar
css={styles.avatar}
username={user.username}
avatarURL={user.avatar_url}
/>
</Badge>
<DropdownArrow
color={theme.experimental.l2.fill.solid}
close={open}
/>
</PopoverContent>
</>
)}
</div>
</button>
</PopoverTrigger>

<PopoverContent
horizontal="right"
css={{
".MuiPaper-root": {
minWidth: "auto",
width: 260,
boxShadow: theme.shadows[6],
},
}}
>
<UserDropdownContent
user={user}
buildInfo={buildInfo}
supportLinks={supportLinks}
onSignOut={onSignOut}
/>
</PopoverContent>
</Popover>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const UserDropdownContent: FC<UserDropdownContentProps> = ({
const popover = usePopover();

const onPopoverClose = () => {
popover.setIsOpen(false);
popover.setOpen(false);
};

const renderMenuIcon = (icon: string): JSX.Element => {
Expand Down
Loading
Loading