Skip to content

Commit 738b1b8

Browse files
BrunoQuaresmaBrunoQuaresma
committed
chore(site): refactor popover to make it easier to extend
1 parent a1db6d8 commit 738b1b8

File tree

13 files changed

+211
-232
lines changed

13 files changed

+211
-232
lines changed

‎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

+36-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,40 @@ 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+
type UncontrolledPopoverProps = BasePopoverProps & {
47+
open?: undefined;
48+
onOpenChange?: undefined;
49+
};
50+
51+
type ControlledPopoverProps = BasePopoverProps & {
52+
open: boolean;
53+
onOpenChange: (open: boolean) => void;
54+
};
55+
56+
export type PopoverProps = UncontrolledPopoverProps | ControlledPopoverProps;
57+
58+
export const Popover: FC<PopoverProps> = (props) => {
5559
const hookId = useId();
56-
const [isOpen, setIsOpen] = useState(isDefaultOpen ?? false);
57-
const triggerRef = useRef<HTMLElement>(null);
60+
const [open, setOpen] = useState(false);
61+
const triggerRef: TriggerRef = useRef(null);
62+
const isControlled = props.open !== undefined;
5863

5964
const value: PopoverContextValue = {
60-
isOpen,
61-
setIsOpen,
6265
triggerRef,
6366
id: `${hookId}-popover`,
64-
mode: mode ?? "click",
67+
mode: props.mode ?? "click",
68+
open: isControlled ? props.open : open,
69+
setOpen: isControlled ? props.onOpenChange : setOpen,
6570
};
6671

6772
return (
6873
<PopoverContext.Provider value={value}>
69-
{typeof children === "function" ? children(value) : children}
74+
{props.children}
7075
</PopoverContext.Provider>
7176
);
7277
};
@@ -82,31 +87,33 @@ export const usePopover = () => {
8287
};
8388

8489
export const PopoverTrigger = (
85-
props: HTMLAttributes<HTMLElement> & { children: TriggerElement },
90+
props: HTMLAttributes<HTMLElement> & {
91+
children: TriggerElement;
92+
},
8693
) => {
8794
const popover = usePopover();
8895
const { children, ...elementProps } = props;
8996

9097
const clickProps = {
9198
onClick: () => {
92-
popover.setIsOpen((isOpen) => !isOpen);
99+
popover.setOpen(true);
93100
},
94101
};
95102

96103
const hoverProps = {
97104
onPointerEnter: () => {
98-
popover.setIsOpen(true);
105+
popover.setOpen(true);
99106
},
100107
onPointerLeave: () => {
101-
popover.setIsOpen(false);
108+
popover.setOpen(false);
102109
},
103110
};
104111

105112
return cloneElement(props.children, {
106113
...elementProps,
107114
...(popover.mode === "click" ? clickProps : hoverProps),
108115
"aria-haspopup": true,
109-
"aria-owns": popover.isOpen ? popover.id : undefined,
116+
"aria-owns": popover.open ? popover.id : undefined,
110117
ref: popover.triggerRef,
111118
});
112119
};
@@ -125,22 +132,8 @@ export const PopoverContent: FC<PopoverContentProps> = ({
125132
...popoverProps
126133
}) => {
127134
const popover = usePopover();
128-
const [isReady, setIsReady] = useState(false);
129135
const hoverMode = popover.mode === "hover";
130136

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-
144137
return (
145138
<MuiPopover
146139
disablePortal
@@ -161,8 +154,8 @@ export const PopoverContent: FC<PopoverContentProps> = ({
161154
{...modeProps(popover)}
162155
{...popoverProps}
163156
id={popover.id}
164-
open={popover.isOpen}
165-
onClose={() => popover.setIsOpen(false)}
157+
open={popover.open}
158+
onClose={() => popover.setOpen(false)}
166159
anchorEl={popover.triggerRef.current}
167160
/>
168161
);
@@ -172,10 +165,10 @@ const modeProps = (popover: PopoverContextValue) => {
172165
if (popover.mode === "hover") {
173166
return {
174167
onPointerEnter: () => {
175-
popover.setIsOpen(true);
168+
popover.setOpen(true);
176169
},
177170
onPointerLeave: () => {
178-
popover.setIsOpen(false);
171+
popover.setOpen(false);
179172
},
180173
};
181174
}

‎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 => {

‎site/src/modules/resources/SSHButton/SSHButton.stories.tsx

+8-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { Meta, StoryObj } from "@storybook/react";
2+
import { userEvent, within } from "@storybook/test";
23
import { MockWorkspace, MockWorkspaceAgent } from "testHelpers/entities";
4+
import { withDesktopViewport } from "testHelpers/storybook";
35
import { SSHButton } from "./SSHButton";
46

57
const meta: Meta<typeof SSHButton> = {
@@ -22,7 +24,12 @@ export const Opened: Story = {
2224
args: {
2325
workspaceName: MockWorkspace.name,
2426
agentName: MockWorkspaceAgent.name,
25-
isDefaultOpen: true,
2627
sshPrefix: "coder.",
2728
},
29+
play: async ({ canvasElement }) => {
30+
const canvas = within(canvasElement);
31+
const button = canvas.getByRole("button");
32+
await userEvent.click(button);
33+
},
34+
decorators: [withDesktopViewport],
2835
};

0 commit comments

Comments
 (0)