Skip to content

Commit ff18499

Browse files
aqandrewaqandrew
authored
refactor: replace Popover with Tooltip in HelpTooltip (#19635)
for #19397 Currently there are 24 files that import bindings from the deprecated `Popover` component. One of those is `HelpTooltip`, which is instantiated in 24 other files. After this PR, the remaining files that import the deprecated `Popover` should be able to be migrated in just 1-2 more PRs. 🤞🏽 I opted for `Tooltip` as a replacement because it's triggered on hover, unlike our new `Popover` which is triggered on click.
1 parent d7d69d1 commit ff18499

File tree

26 files changed

+211
-178
lines changed

26 files changed

+211
-178
lines changed

‎site/src/components/ActiveUserChart/ActiveUserChart.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import {
77
import {
88
HelpTooltip,
99
HelpTooltipContent,
10+
HelpTooltipIconTrigger,
1011
HelpTooltipText,
1112
HelpTooltipTitle,
12-
HelpTooltipTrigger,
1313
} from "components/HelpTooltip/HelpTooltip";
1414
import type { FC } from "react";
1515
import { Area, AreaChart, CartesianGrid, XAxis, YAxis } from "recharts";
@@ -116,7 +116,7 @@ export const ActiveUsersTitle: FC<ActiveUsersTitleProps> = ({ interval }) => {
116116
<div className="flex items-center gap-2">
117117
{interval === "day" ? "Daily" : "Weekly"} Active Users
118118
<HelpTooltip>
119-
<HelpTooltipTrigger size="small" />
119+
<HelpTooltipIconTrigger size="small" />
120120
<HelpTooltipContent>
121121
<HelpTooltipTitle>How do we calculate active users?</HelpTooltipTitle>
122122
<HelpTooltipText>

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

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,17 @@ import {
33
css,
44
type Interpolation,
55
type Theme,
6-
useTheme,
76
} from "@emotion/react";
87
import Link from "@mui/material/Link";
9-
import {
10-
Popover,
11-
PopoverContent,
12-
type PopoverContentProps,
13-
type PopoverProps,
14-
PopoverTrigger,
15-
usePopover,
16-
} from "components/deprecated/Popover/Popover";
178
import { Stack } from "components/Stack/Stack";
9+
import {
10+
Tooltip,
11+
TooltipContent,
12+
type TooltipContentProps,
13+
type TooltipProps,
14+
TooltipProvider,
15+
TooltipTrigger,
16+
} from "components/Tooltip/Tooltip";
1817
import { CircleHelpIcon, ExternalLinkIcon } from "lucide-react";
1918
import {
2019
type FC,
@@ -23,43 +22,50 @@ import {
2322
type PropsWithChildren,
2423
type ReactNode,
2524
} from "react";
25+
import { cn } from "utils/cn";
2626

2727
type Icon = typeof CircleHelpIcon;
2828

2929
type Size = "small" | "medium";
3030

31+
export const HelpTooltipTrigger = TooltipTrigger;
32+
3133
export const HelpTooltipIcon = CircleHelpIcon;
3234

33-
export const HelpTooltip: FC<PopoverProps> = (props) => {
34-
return <Popover mode="hover" {...props} />;
35+
export const HelpTooltip: FC<TooltipProps> = (props) => {
36+
return (
37+
<TooltipProvider>
38+
<Tooltip delayDuration={0} {...props} />
39+
</TooltipProvider>
40+
);
3541
};
3642

37-
export const HelpTooltipContent: FC<PopoverContentProps> = (props) => {
38-
const theme = useTheme();
39-
43+
export const HelpTooltipContent: FC<TooltipContentProps> = ({
44+
className,
45+
...props
46+
}) => {
4047
return (
41-
<PopoverContent
48+
<TooltipContent
49+
side="bottom"
50+
align="start"
51+
collisionPadding={16}
4252
{...props}
43-
css={{
44-
"& .MuiPaper-root": {
45-
fontSize: 14,
46-
width: 304,
47-
padding: 20,
48-
color: theme.palette.text.secondary,
49-
},
50-
}}
53+
className={cn(
54+
"w-[320px] p-5 bg-surface-secondary border-surface-quaternary text-sm",
55+
className,
56+
)}
5157
/>
5258
);
5359
};
5460

55-
type HelpTooltipTriggerProps = HTMLAttributes<HTMLButtonElement> & {
61+
type HelpTooltipIconTriggerProps = HTMLAttributes<HTMLButtonElement> & {
5662
size?: Size;
5763
hoverEffect?: boolean;
5864
};
5965

60-
export const HelpTooltipTrigger = forwardRef<
66+
export const HelpTooltipIconTrigger = forwardRef<
6167
HTMLButtonElement,
62-
HelpTooltipTriggerProps
68+
HelpTooltipIconTriggerProps
6369
>((props, ref) => {
6470
const {
6571
size = "medium",
@@ -76,7 +82,7 @@ export const HelpTooltipTrigger = forwardRef<
7682
});
7783

7884
return (
79-
<PopoverTrigger>
85+
<HelpTooltipTrigger asChild>
8086
<button
8187
{...buttonProps}
8288
aria-label="More info"
@@ -102,7 +108,7 @@ export const HelpTooltipTrigger = forwardRef<
102108
>
103109
{children}
104110
</button>
105-
</PopoverTrigger>
111+
</HelpTooltipTrigger>
106112
);
107113
});
108114

@@ -155,18 +161,12 @@ export const HelpTooltipAction: FC<HelpTooltipActionProps> = ({
155161
onClick,
156162
ariaLabel,
157163
}) => {
158-
const popover = usePopover();
159-
160164
return (
161165
<button
162166
type="button"
163167
aria-label={ariaLabel ?? ""}
164168
css={styles.action}
165-
onClick={(event) => {
166-
event.stopPropagation();
167-
onClick();
168-
popover.setOpen(false);
169-
}}
169+
onClick={onClick}
170170
>
171171
<Icon css={styles.actionIcon} />
172172
{children}

‎site/src/components/InfoTooltip/InfoTooltip.stories.tsx

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Meta, StoryObj } from "@storybook/react-vite";
2-
import { expect, userEvent, waitFor, within } from "storybook/test";
2+
import { expect, screen, userEvent, waitFor } from "storybook/test";
33
import { InfoTooltip } from "./InfoTooltip";
44

55
const meta = {
@@ -16,13 +16,13 @@ export default meta;
1616
type Story = StoryObj<typeof InfoTooltip>;
1717

1818
export const Example: Story = {
19-
play: async ({ canvasElement, step }) => {
20-
const screen = within(canvasElement);
21-
19+
play: async ({ step }) => {
2220
await step("activate hover trigger", async () => {
2321
await userEvent.hover(screen.getByRole("button"));
2422
await waitFor(() =>
25-
expect(screen.getByText(meta.args.message)).toBeInTheDocument(),
23+
expect(screen.getByRole("tooltip")).toHaveTextContent(
24+
meta.args.message,
25+
),
2626
);
2727
});
2828
},
@@ -33,13 +33,13 @@ export const Notice = {
3333
type: "notice",
3434
message: "Unfortunately, there's a radio connected to my brain",
3535
},
36-
play: async ({ canvasElement, step }) => {
37-
const screen = within(canvasElement);
38-
36+
play: async ({ step }) => {
3937
await step("activate hover trigger", async () => {
4038
await userEvent.hover(screen.getByRole("button"));
4139
await waitFor(() =>
42-
expect(screen.getByText(Notice.args.message)).toBeInTheDocument(),
40+
expect(screen.getByRole("tooltip")).toHaveTextContent(
41+
Notice.args.message,
42+
),
4343
);
4444
});
4545
},
@@ -50,13 +50,13 @@ export const Warning = {
5050
type: "warning",
5151
message: "Unfortunately, there's a radio connected to my brain",
5252
},
53-
play: async ({ canvasElement, step }) => {
54-
const screen = within(canvasElement);
55-
53+
play: async ({ step }) => {
5654
await step("activate hover trigger", async () => {
5755
await userEvent.hover(screen.getByRole("button"));
5856
await waitFor(() =>
59-
expect(screen.getByText(Warning.args.message)).toBeInTheDocument(),
57+
expect(screen.getByRole("tooltip")).toHaveTextContent(
58+
Warning.args.message,
59+
),
6060
);
6161
});
6262
},

‎site/src/components/InfoTooltip/InfoTooltip.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ import {
33
HelpTooltip,
44
HelpTooltipContent,
55
HelpTooltipIcon,
6+
HelpTooltipIconTrigger,
67
HelpTooltipText,
78
HelpTooltipTitle,
8-
HelpTooltipTrigger,
99
} from "components/HelpTooltip/HelpTooltip";
1010
import type { FC, ReactNode } from "react";
1111
import type { ThemeRole } from "theme/roles";
@@ -26,9 +26,9 @@ export const InfoTooltip: FC<InfoTooltipProps> = ({
2626

2727
return (
2828
<HelpTooltip>
29-
<HelpTooltipTrigger size="small" css={styles.button}>
29+
<HelpTooltipIconTrigger size="small" css={styles.button}>
3030
<HelpTooltipIcon css={{ color: iconColor }} />
31-
</HelpTooltipTrigger>
31+
</HelpTooltipIconTrigger>
3232
<HelpTooltipContent>
3333
<HelpTooltipTitle>{title}</HelpTooltipTitle>
3434
<HelpTooltipText>{message}</HelpTooltipText>
@@ -39,10 +39,10 @@ export const InfoTooltip: FC<InfoTooltipProps> = ({
3939

4040
const styles = {
4141
button: css`
42-
opacity: 1;
42+
opacity: 1;
4343
44-
&:hover {
45-
opacity: 1;
46-
}
47-
`,
44+
&:hover {
45+
opacity: 1;
46+
}
47+
`,
4848
} satisfies Record<string, Interpolation<Theme>>;

‎site/src/components/Tooltip/Tooltip.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,21 @@ import { cn } from "utils/cn";
88

99
export const TooltipProvider = TooltipPrimitive.Provider;
1010

11+
export type TooltipProps = TooltipPrimitive.TooltipProps;
12+
1113
export const Tooltip = TooltipPrimitive.Root;
1214

1315
export const TooltipTrigger = TooltipPrimitive.Trigger;
1416

17+
export type TooltipContentProps = React.ComponentPropsWithoutRef<
18+
typeof TooltipPrimitive.Content
19+
> & {
20+
disablePortal?: boolean;
21+
};
22+
1523
export const TooltipContent = React.forwardRef<
1624
React.ElementRef<typeof TooltipPrimitive.Content>,
17-
React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content> & {
18-
disablePortal?: boolean;
19-
}
25+
TooltipContentProps
2026
>(({ className, sideOffset = 4, disablePortal, ...props }, ref) => {
2127
const content = (
2228
<TooltipPrimitive.Content

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type ControlledPopoverProps = BasePopoverProps & {
6060
onOpenChange: (open: boolean) => void;
6161
};
6262

63-
export type PopoverProps = UncontrolledPopoverProps | ControlledPopoverProps;
63+
type PopoverProps = UncontrolledPopoverProps | ControlledPopoverProps;
6464

6565
/** @deprecated prefer `components.Popover` */
6666
export const Popover: FC<PopoverProps> = (props) => {
@@ -155,7 +155,7 @@ export const PopoverTrigger: FC<PopoverTriggerProps> = (props) => {
155155

156156
type Horizontal = "left" | "right";
157157

158-
export type PopoverContentProps = Omit<
158+
type PopoverContentProps = Omit<
159159
MuiPopoverProps,
160160
"open" | "onClose" | "anchorEl"
161161
> & {

‎site/src/modules/resources/AgentLatency.tsx

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { type Theme, useTheme } from "@emotion/react";
22
import type { DERPRegion, WorkspaceAgent } from "api/typesGenerated";
3-
import { PopoverTrigger } from "components/deprecated/Popover/Popover";
43
import {
54
HelpTooltip,
65
HelpTooltipContent,
76
HelpTooltipText,
87
HelpTooltipTitle,
8+
HelpTooltipTrigger,
99
} from "components/HelpTooltip/HelpTooltip";
1010
import { Stack } from "components/Stack/Stack";
1111
import type { FC } from "react";
@@ -44,43 +44,41 @@ export const AgentLatency: FC<AgentLatencyProps> = ({ agent }) => {
4444

4545
return (
4646
<HelpTooltip>
47-
<PopoverTrigger>
47+
<HelpTooltipTrigger asChild>
4848
<span
4949
role="presentation"
5050
aria-label="latency"
5151
css={{ cursor: "pointer", color: latency.color }}
5252
>
5353
{Math.round(latency.latency_ms)}ms
5454
</span>
55-
</PopoverTrigger>
55+
</HelpTooltipTrigger>
5656
<HelpTooltipContent>
5757
<HelpTooltipTitle>Latency</HelpTooltipTitle>
5858
<HelpTooltipText>
5959
This is the latency overhead on non peer to peer connections. The
6060
first row is the preferred relay.
6161
</HelpTooltipText>
62-
<HelpTooltipText>
63-
<Stack direction="column" spacing={1} css={{ marginTop: 16 }}>
64-
{Object.entries(agent.latency)
65-
.sort(([, a], [, b]) => a.latency_ms - b.latency_ms)
66-
.map(([regionName, region]) => (
67-
<Stack
68-
direction="row"
69-
key={regionName}
70-
spacing={0.5}
71-
justifyContent="space-between"
72-
css={
73-
region.preferred && {
74-
color: theme.palette.text.primary,
75-
}
62+
<Stack direction="column" spacing={1} css={{ marginTop: 16 }}>
63+
{Object.entries(agent.latency)
64+
.sort(([, a], [, b]) => a.latency_ms - b.latency_ms)
65+
.map(([regionName, region]) => (
66+
<Stack
67+
direction="row"
68+
key={regionName}
69+
spacing={0.5}
70+
justifyContent="space-between"
71+
css={
72+
region.preferred && {
73+
color: theme.palette.text.primary,
7674
}
77-
>
78-
<strong>{regionName}</strong>
79-
{Math.round(region.latency_ms)}ms
80-
</Stack>
81-
))}
82-
</Stack>
83-
</HelpTooltipText>
75+
}
76+
>
77+
<strong>{regionName}</strong>
78+
{Math.round(region.latency_ms)}ms
79+
</Stack>
80+
))}
81+
</Stack>
8482
</HelpTooltipContent>
8583
</HelpTooltip>
8684
);

0 commit comments

Comments
 (0)