Skip to content

Commit cbdaa63

Browse files
BrunoQuaresmaBrunoQuaresma
authored
chore(site): refactor popover to make it easier to extend (#13611)
1 parent 714f2ef commit cbdaa63

File tree

18 files changed

+218
-237
lines changed

18 files changed

+218
-237
lines changed

‎site/src/components/HelpTooltip/HelpTooltip.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ export const HelpTooltipAction: FC<HelpTooltipActionProps> = ({
160160
onClick={(event) => {
161161
event.stopPropagation();
162162
onClick();
163-
popover.setIsOpen(false);
163+
popover.setOpen(false);
164164
}}
165165
>
166166
<Icon css={styles.actionIcon} />

‎site/src/components/IconField/IconField.tsx

+22-26
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import Button from "@mui/material/Button";
33
import InputAdornment from "@mui/material/InputAdornment";
44
import TextField, { type TextFieldProps } from "@mui/material/TextField";
55
import { visuallyHidden } from "@mui/utils";
6-
import { type FC, lazy, Suspense } from "react";
6+
import { type FC, lazy, Suspense, useState } from "react";
77
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
88
import { ExternalImage } from "components/ExternalImage/ExternalImage";
99
import { Loader } from "components/Loader/Loader";
@@ -37,6 +37,7 @@ export const IconField: FC<IconFieldProps> = ({
3737

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

4142
return (
4243
<Stack spacing={1}>
@@ -86,31 +87,26 @@ export const IconField: FC<IconFieldProps> = ({
8687
}
8788
`}
8889
/>
89-
<Popover>
90-
{(popover) => (
91-
<>
92-
<PopoverTrigger>
93-
<Button fullWidth endIcon={<DropdownArrow />}>
94-
Select emoji
95-
</Button>
96-
</PopoverTrigger>
97-
<PopoverContent
98-
id="emoji"
99-
css={{ marginTop: 0, ".MuiPaper-root": { width: "auto" } }}
100-
>
101-
<Suspense fallback={<Loader />}>
102-
<EmojiPicker
103-
onEmojiSelect={(emoji) => {
104-
const value =
105-
emoji.src ?? urlFromUnifiedCode(emoji.unified);
106-
onPickEmoji(value);
107-
popover.setIsOpen(false);
108-
}}
109-
/>
110-
</Suspense>
111-
</PopoverContent>
112-
</>
113-
)}
90+
<Popover open={open} onOpenChange={setOpen}>
91+
<PopoverTrigger>
92+
<Button fullWidth endIcon={<DropdownArrow />}>
93+
Select emoji
94+
</Button>
95+
</PopoverTrigger>
96+
<PopoverContent
97+
id="emoji"
98+
css={{ marginTop: 0, ".MuiPaper-root": { width: "auto" } }}
99+
>
100+
<Suspense fallback={<Loader />}>
101+
<EmojiPicker
102+
onEmojiSelect={(emoji) => {
103+
const value = emoji.src ?? urlFromUnifiedCode(emoji.unified);
104+
onPickEmoji(value);
105+
setOpen(false);
106+
}}
107+
/>
108+
</Suspense>
109+
</PopoverContent>
114110
</Popover>
115111

116112
{/*

‎site/src/components/Popover/Popover.tsx

+38-43
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
type ReactNode,
1313
type RefObject,
1414
useContext,
15-
useEffect,
1615
useId,
1716
useRef,
1817
useState,
@@ -25,14 +24,12 @@ type TriggerRef = RefObject<HTMLElement>;
2524
type TriggerElement = ReactElement<{
2625
ref: TriggerRef;
2726
onClick?: () => void;
28-
"aria-haspopup"?: boolean;
29-
"aria-owns"?: string | undefined;
3027
}>;
3128

3229
type PopoverContextValue = {
3330
id: string;
34-
isOpen: boolean;
35-
setIsOpen: React.Dispatch<React.SetStateAction<boolean>>;
31+
open: boolean;
32+
setOpen: (open: boolean) => void;
3633
triggerRef: TriggerRef;
3734
mode: TriggerMode;
3835
};
@@ -41,32 +38,41 @@ const PopoverContext = createContext<PopoverContextValue | undefined>(
4138
undefined,
4239
);
4340

44-
export interface PopoverProps {
45-
children: ReactNode | ((popover: PopoverContextValue) => ReactNode); // Allows inline usage
41+
type BasePopoverProps = {
42+
children: ReactNode;
4643
mode?: TriggerMode;
47-
isDefaultOpen?: boolean;
48-
}
44+
};
4945

50-
export const Popover: FC<PopoverProps> = ({
51-
children,
52-
mode,
53-
isDefaultOpen,
54-
}) => {
46+
// By separating controlled and uncontrolled props, we achieve more accurate
47+
// type inference.
48+
type UncontrolledPopoverProps = BasePopoverProps & {
49+
open?: undefined;
50+
onOpenChange?: undefined;
51+
};
52+
53+
type ControlledPopoverProps = BasePopoverProps & {
54+
open: boolean;
55+
onOpenChange: (open: boolean) => void;
56+
};
57+
58+
export type PopoverProps = UncontrolledPopoverProps | ControlledPopoverProps;
59+
60+
export const Popover: FC<PopoverProps> = (props) => {
5561
const hookId = useId();
56-
const [isOpen, setIsOpen] = useState(isDefaultOpen ?? false);
57-
const triggerRef = useRef<HTMLElement>(null);
62+
const [uncontrolledOpen, setUncontrolledOpen] = useState(false);
63+
const triggerRef: TriggerRef = useRef(null);
5864

5965
const value: PopoverContextValue = {
60-
isOpen,
61-
setIsOpen,
6266
triggerRef,
6367
id: `${hookId}-popover`,
64-
mode: mode ?? "click",
68+
mode: props.mode ?? "click",
69+
open: props.open ?? uncontrolledOpen,
70+
setOpen: props.onOpenChange ?? setUncontrolledOpen,
6571
};
6672

6773
return (
6874
<PopoverContext.Provider value={value}>
69-
{typeof children === "function" ? children(value) : children}
75+
{props.children}
7076
</PopoverContext.Provider>
7177
);
7278
};
@@ -82,31 +88,34 @@ export const usePopover = () => {
8288
};
8389

8490
export const PopoverTrigger = (
85-
props: HTMLAttributes<HTMLElement> & { children: TriggerElement },
91+
props: HTMLAttributes<HTMLElement> & {
92+
children: TriggerElement;
93+
},
8694
) => {
8795
const popover = usePopover();
8896
const { children, ...elementProps } = props;
8997

9098
const clickProps = {
9199
onClick: () => {
92-
popover.setIsOpen((isOpen) => !isOpen);
100+
popover.setOpen(true);
93101
},
94102
};
95103

96104
const hoverProps = {
97105
onPointerEnter: () => {
98-
popover.setIsOpen(true);
106+
popover.setOpen(true);
99107
},
100108
onPointerLeave: () => {
101-
popover.setIsOpen(false);
109+
popover.setOpen(false);
102110
},
103111
};
104112

105113
return cloneElement(props.children, {
106114
...elementProps,
107115
...(popover.mode === "click" ? clickProps : hoverProps),
108116
"aria-haspopup": true,
109-
"aria-owns": popover.isOpen ? popover.id : undefined,
117+
"aria-owns": popover.id,
118+
"aria-expanded": popover.open,
110119
ref: popover.triggerRef,
111120
});
112121
};
@@ -125,22 +134,8 @@ export const PopoverContent: FC<PopoverContentProps> = ({
125134
...popoverProps
126135
}) => {
127136
const popover = usePopover();
128-
const [isReady, setIsReady] = useState(false);
129137
const hoverMode = popover.mode === "hover";
130138

131-
// This is a hack to make sure the popover is not rendered until the trigger
132-
// is ready. This is a limitation on MUI that does not support defaultIsOpen
133-
// on Popover but we need it to storybook the component.
134-
useEffect(() => {
135-
if (!isReady && popover.triggerRef.current !== null) {
136-
setIsReady(true);
137-
}
138-
}, [isReady, popover.triggerRef]);
139-
140-
if (!popover.triggerRef.current) {
141-
return null;
142-
}
143-
144139
return (
145140
<MuiPopover
146141
disablePortal
@@ -161,8 +156,8 @@ export const PopoverContent: FC<PopoverContentProps> = ({
161156
{...modeProps(popover)}
162157
{...popoverProps}
163158
id={popover.id}
164-
open={popover.isOpen}
165-
onClose={() => popover.setIsOpen(false)}
159+
open={popover.open}
160+
onClose={() => popover.setOpen(false)}
166161
anchorEl={popover.triggerRef.current}
167162
/>
168163
);
@@ -172,10 +167,10 @@ const modeProps = (popover: PopoverContextValue) => {
172167
if (popover.mode === "hover") {
173168
return {
174169
onPointerEnter: () => {
175-
popover.setIsOpen(true);
170+
popover.setOpen(true);
176171
},
177172
onPointerLeave: () => {
178-
popover.setIsOpen(false);
173+
popover.setOpen(false);
179174
},
180175
};
181176
}

‎site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ const DeploymentDropdownContent: FC<DeploymentDropdownProps> = ({
8787
}) => {
8888
const popover = usePopover();
8989

90-
const onPopoverClose = () => popover.setIsOpen(false);
90+
const onPopoverClose = () => popover.setOpen(false);
9191

9292
return (
9393
<nav>

‎site/src/modules/dashboard/Navbar/ProxyMenu.stories.tsx

+2-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
MockUser,
1212
MockWorkspaceProxies,
1313
} from "testHelpers/entities";
14+
import { withDesktopViewport } from "testHelpers/storybook";
1415
import { ProxyMenu } from "./ProxyMenu";
1516

1617
const defaultProxyContextValue = {
@@ -36,11 +37,7 @@ const meta: Meta<typeof ProxyMenu> = {
3637
<Story />
3738
</AuthProvider>
3839
),
39-
(Story) => (
40-
<div css={{ width: 1200, height: 800 }}>
41-
<Story />
42-
</div>
43-
),
40+
withDesktopViewport,
4441
],
4542
parameters: {
4643
queries: [

‎site/src/modules/dashboard/Navbar/UserDropdown/UserDropdown.tsx

+37-40
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { css, type Interpolation, type Theme, useTheme } from "@emotion/react";
22
import Badge from "@mui/material/Badge";
3-
import type { FC } from "react";
3+
import { useState, type FC } from "react";
44
import type * as TypesGen from "api/typesGenerated";
55
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
66
import {
@@ -26,48 +26,45 @@ export const UserDropdown: FC<UserDropdownProps> = ({
2626
onSignOut,
2727
}) => {
2828
const theme = useTheme();
29+
const [open, setOpen] = useState(false);
2930

3031
return (
31-
<Popover>
32-
{(popover) => (
33-
<>
34-
<PopoverTrigger>
35-
<button css={styles.button} data-testid="user-dropdown-trigger">
36-
<div css={styles.badgeContainer}>
37-
<Badge overlap="circular">
38-
<UserAvatar
39-
css={styles.avatar}
40-
username={user.username}
41-
avatarURL={user.avatar_url}
42-
/>
43-
</Badge>
44-
<DropdownArrow
45-
color={theme.experimental.l2.fill.solid}
46-
close={popover.isOpen}
47-
/>
48-
</div>
49-
</button>
50-
</PopoverTrigger>
51-
52-
<PopoverContent
53-
horizontal="right"
54-
css={{
55-
".MuiPaper-root": {
56-
minWidth: "auto",
57-
width: 260,
58-
boxShadow: theme.shadows[6],
59-
},
60-
}}
61-
>
62-
<UserDropdownContent
63-
user={user}
64-
buildInfo={buildInfo}
65-
supportLinks={supportLinks}
66-
onSignOut={onSignOut}
32+
<Popover open={open} onOpenChange={setOpen}>
33+
<PopoverTrigger>
34+
<button css={styles.button} data-testid="user-dropdown-trigger">
35+
<div css={styles.badgeContainer}>
36+
<Badge overlap="circular">
37+
<UserAvatar
38+
css={styles.avatar}
39+
username={user.username}
40+
avatarURL={user.avatar_url}
41+
/>
42+
</Badge>
43+
<DropdownArrow
44+
color={theme.experimental.l2.fill.solid}
45+
close={open}
6746
/>
68-
</PopoverContent>
69-
</>
70-
)}
47+
</div>
48+
</button>
49+
</PopoverTrigger>
50+
51+
<PopoverContent
52+
horizontal="right"
53+
css={{
54+
".MuiPaper-root": {
55+
minWidth: "auto",
56+
width: 260,
57+
boxShadow: theme.shadows[6],
58+
},
59+
}}
60+
>
61+
<UserDropdownContent
62+
user={user}
63+
buildInfo={buildInfo}
64+
supportLinks={supportLinks}
65+
onSignOut={onSignOut}
66+
/>
67+
</PopoverContent>
7168
</Popover>
7269
);
7370
};

‎site/src/modules/dashboard/Navbar/UserDropdown/UserDropdownContent.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export const UserDropdownContent: FC<UserDropdownContentProps> = ({
4343
const popover = usePopover();
4444

4545
const onPopoverClose = () => {
46-
popover.setIsOpen(false);
46+
popover.setOpen(false);
4747
};
4848

4949
const renderMenuIcon = (icon: string): JSX.Element => {

0 commit comments

Comments
 (0)