Skip to content

Commit 49fadb8

Browse files
BrunoQuaresmaBrunoQuaresma
authored
refactor: update the navbar to match the new designs (#15964)
Update the navbar to match the designs in [this Figma file](https://www.figma.com/design/WfqIgsTFXN2BscBSSyXWF8/Coder-kit?node-id=656-2354&t=4a6pX5tQU5Ti2Oyi-0). Related to #15617. **Desktop preview:** https://github.com/user-attachments/assets/01ce7cd2-baaa-49c4-9e9a-bf6e675151da **Mobile preview:** https://github.com/user-attachments/assets/155e2521-7293-4368-a5f5-425179d76326 For a closer look, you can check Chromatic snapshots or test the changes locally. **A few considerations:** - I made some adjustments to improve the design, such as removing the chevron from the profile menu and reducing the size of the chevrons in the dropdowns. I’ve documented these changes in the [Figma file](https://www.figma.com/design/WfqIgsTFXN2BscBSSyXWF8/Coder-kit?node-id=656-2354&t=4a6pX5tQU5Ti2Oyi-0) so @chrifro can review them after returning from vacation. - Some of the design questions involve how the proxy and account dropdown menus should look on desktop and mobile. For desktop, I decided to retain the current styles, and for mobile, I tried to infer how they should look based on the existing design. - There is some duplicated logic between the regular/desktop navbar menus and the mobile menus, which could lead to inconsistencies and make maintenance harder in the future. I plan to address this in a follow-up PR to keep this review manageable. - I’ve added tests to minimize inconsistencies and potential bugs while working on this refactor.
1 parent 459003f commit 49fadb8

20 files changed

+686
-326
lines changed

‎site/.storybook/preview.jsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ export const parameters = {
6464
},
6565
type: "tablet",
6666
},
67+
iphone12: {
68+
name: "iPhone 12",
69+
styles: {
70+
height: "844px",
71+
width: "390px",
72+
},
73+
type: "mobile",
74+
},
6775
terminal: {
6876
name: "Terminal",
6977
styles: {

‎site/src/components/Breadcrumb/Breadcrumb.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export const BreadcrumbList = forwardRef<
2828
<ol
2929
ref={ref}
3030
className={cn(
31-
"flex flex-wrap items-center text-sm pl-12 my-4 gap-1.5 break-words font-medium list-none sm:gap-2.5",
31+
"flex flex-wrap items-center text-sm pl-6 my-4 gap-1.5 break-words font-medium list-none sm:gap-2.5",
3232
className,
3333
)}
3434
{...props}

‎site/src/components/Button/Button.tsx

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55
import { Slot } from "@radix-ui/react-slot";
66
import { type VariantProps, cva } from "class-variance-authority";
7-
import { type FC, forwardRef } from "react";
7+
import { forwardRef } from "react";
88
import { cn } from "utils/cn";
99

1010
export const buttonVariants = cva(
@@ -13,7 +13,8 @@ export const buttonVariants = cva(
1313
text-sm font-semibold font-medium cursor-pointer
1414
focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-content-link
1515
disabled:pointer-events-none disabled:text-content-disabled
16-
[&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0`,
16+
[&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0
17+
px-3 py-2`,
1718
{
1819
variants: {
1920
variant: {
@@ -25,10 +26,15 @@ export const buttonVariants = cva(
2526
"border-none bg-transparent text-content-secondary hover:text-content-primary",
2627
warning:
2728
"border border-border-error text-content-primary bg-surface-error hover:bg-transparent",
29+
ghost:
30+
"text-content-primary bg-transparent border-0 hover:bg-surface-secondary",
2831
},
32+
2933
size: {
30-
default: "h-9 px-3 py-2",
31-
sm: "h-8 px-2 text-xs",
34+
lg: "h-10",
35+
default: "h-9",
36+
sm: "h-8 px-2 py-1.5 text-xs",
37+
icon: "h-10 w-10",
3238
},
3339
},
3440
defaultVariants: {
@@ -44,16 +50,15 @@ export interface ButtonProps
4450
asChild?: boolean;
4551
}
4652

47-
export const Button: FC<ButtonProps> = forwardRef<
48-
HTMLButtonElement,
49-
ButtonProps
50-
>(({ className, variant, size, asChild = false, ...props }, ref) => {
51-
const Comp = asChild ? Slot : "button";
52-
return (
53-
<Comp
54-
className={cn(buttonVariants({ variant, size, className }))}
55-
ref={ref}
56-
{...props}
57-
/>
58-
);
59-
});
53+
export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
54+
({ className, variant, size, asChild = false, ...props }, ref) => {
55+
const Comp = asChild ? Slot : "button";
56+
return (
57+
<Comp
58+
className={cn(buttonVariants({ variant, size, className }))}
59+
ref={ref}
60+
{...props}
61+
/>
62+
);
63+
},
64+
);

‎site/src/components/DropdownMenu/DropdownMenu.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
*/
88

99
import * as DropdownMenuPrimitive from "@radix-ui/react-dropdown-menu";
10-
import { Check, ChevronRight, Circle } from "lucide-react";
10+
import { Button } from "components/Button/Button";
11+
import { Check, ChevronDownIcon, ChevronRight, Circle } from "lucide-react";
1112
import {
1213
type ComponentPropsWithoutRef,
1314
type ElementRef,
15+
type FC,
1416
type HTMLAttributes,
1517
forwardRef,
1618
} from "react";
@@ -196,7 +198,7 @@ export const DropdownMenuSeparator = forwardRef<
196198
>(({ className, ...props }, ref) => (
197199
<DropdownMenuPrimitive.Separator
198200
ref={ref}
199-
className={cn(["-mx-1 my-1 h-px bg-border"], className)}
201+
className={cn(["-mx-1 my-3 h-px bg-border"], className)}
200202
{...props}
201203
/>
202204
));

‎site/src/contexts/ProxyContext.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
import { useQuery } from "react-query";
1616
import { type ProxyLatencyReport, useProxyLatency } from "./useProxyLatency";
1717

18+
export type Proxies = readonly Region[] | readonly WorkspaceProxy[];
19+
export type ProxyLatencies = Record<string, ProxyLatencyReport>;
1820
export interface ProxyContextValue {
1921
// proxy is **always** the workspace proxy that should be used.
2022
// The 'proxy.selectedProxy' field is the proxy being used and comes from either:
@@ -43,15 +45,15 @@ export interface ProxyContextValue {
4345
// WorkspaceProxy[] is returned if the user is an admin. WorkspaceProxy extends Region with
4446
// more information about the proxy and the status. More information includes the error message if
4547
// the proxy is unhealthy.
46-
proxies?: readonly Region[] | readonly WorkspaceProxy[];
48+
proxies?: Proxies;
4749
// isFetched is true when the 'proxies' api call is complete.
4850
isFetched: boolean;
4951
isLoading: boolean;
5052
error?: unknown;
5153
// proxyLatencies is a map of proxy id to latency report. If the proxyLatencies[proxy.id] is undefined
5254
// then the latency has not been fetched yet. Calculations happen async for each proxy in the list.
5355
// Refer to the returned report for a given proxy for more information.
54-
proxyLatencies: Record<string, ProxyLatencyReport>;
56+
proxyLatencies: ProxyLatencies;
5557
// refetchProxyLatencies will trigger refreshing of the proxy latencies. By default the latencies
5658
// are loaded once.
5759
refetchProxyLatencies: () => Date;

‎site/src/index.css

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,17 @@
7070
* {
7171
@apply border-border;
7272
}
73+
74+
/*
75+
By default, Radix adds a margin to the `body` element when a dropdown is displayed,
76+
causing some shifting when the dropdown has a full-width size, as is the case with the mobile menu.
77+
To prevent this, we need to apply the styles below.
78+
79+
There’s a related issue on GitHub: Radix UI Primitives Issue #3251
80+
https://github.com/radix-ui/primitives/issues/3251
81+
*/
82+
html body[data-scroll-locked] {
83+
--removed-body-scroll-bar-size: 0 !important;
84+
margin-right: 0 !important;
85+
}
7386
}

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

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,28 @@
11
import { type Interpolation, type Theme, css, useTheme } from "@emotion/react";
2-
import Button from "@mui/material/Button";
32
import MenuItem from "@mui/material/MenuItem";
4-
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
3+
import { Button } from "components/Button/Button";
54
import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge";
65
import {
76
Popover,
87
PopoverContent,
98
PopoverTrigger,
109
usePopover,
1110
} from "components/deprecated/Popover/Popover";
11+
import { ChevronDownIcon } from "lucide-react";
1212
import { linkToAuditing } from "modules/navigation";
1313
import type { FC } from "react";
1414
import { NavLink } from "react-router-dom";
1515

1616
interface DeploymentDropdownProps {
1717
canViewDeployment: boolean;
1818
canViewOrganizations: boolean;
19-
canViewAllUsers: boolean;
2019
canViewAuditLog: boolean;
2120
canViewHealth: boolean;
2221
}
2322

2423
export const DeploymentDropdown: FC<DeploymentDropdownProps> = ({
2524
canViewDeployment,
2625
canViewOrganizations,
27-
canViewAllUsers,
2826
canViewAuditLog,
2927
canViewHealth,
3028
}) => {
@@ -34,7 +32,6 @@ export const DeploymentDropdown: FC<DeploymentDropdownProps> = ({
3432
!canViewAuditLog &&
3533
!canViewOrganizations &&
3634
!canViewDeployment &&
37-
!canViewAllUsers &&
3835
!canViewHealth
3936
) {
4037
return null;
@@ -43,17 +40,9 @@ export const DeploymentDropdown: FC<DeploymentDropdownProps> = ({
4340
return (
4441
<Popover>
4542
<PopoverTrigger>
46-
<Button
47-
size="small"
48-
endIcon={
49-
<DropdownArrow
50-
color={theme.experimental.l2.fill.solid}
51-
close={false}
52-
margin={false}
53-
/>
54-
}
55-
>
43+
<Button variant="outline" size="lg">
5644
Admin settings
45+
<ChevronDownIcon className="text-content-primary !size-icon-xs" />
5746
</Button>
5847
</PopoverTrigger>
5948

@@ -70,7 +59,6 @@ export const DeploymentDropdown: FC<DeploymentDropdownProps> = ({
7059
<DeploymentDropdownContent
7160
canViewDeployment={canViewDeployment}
7261
canViewOrganizations={canViewOrganizations}
73-
canViewAllUsers={canViewAllUsers}
7462
canViewAuditLog={canViewAuditLog}
7563
canViewHealth={canViewHealth}
7664
/>

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

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import type { Meta, StoryObj } from "@storybook/react";
2+
import { fn, userEvent, within } from "@storybook/test";
3+
import { PointerEventsCheckLevel } from "@testing-library/user-event";
4+
import type { FC } from "react";
5+
import { chromaticWithTablet } from "testHelpers/chromatic";
6+
import {
7+
MockPrimaryWorkspaceProxy,
8+
MockProxyLatencies,
9+
MockSupportLinks,
10+
MockUser,
11+
MockUser2,
12+
MockWorkspaceProxies,
13+
} from "testHelpers/entities";
14+
import { MobileMenu } from "./MobileMenu";
15+
16+
const meta: Meta<typeof MobileMenu> = {
17+
title: "modules/dashboard/MobileMenu",
18+
parameters: {
19+
layout: "fullscreen",
20+
viewport: {
21+
defaultViewport: "iphone12",
22+
},
23+
},
24+
component: MobileMenu,
25+
args: {
26+
proxyContextValue: {
27+
proxy: {
28+
preferredPathAppURL: "",
29+
preferredWildcardHostname: "",
30+
proxy: MockPrimaryWorkspaceProxy,
31+
},
32+
isLoading: false,
33+
isFetched: true,
34+
setProxy: fn(),
35+
clearProxy: fn(),
36+
refetchProxyLatencies: fn(),
37+
proxyLatencies: MockProxyLatencies,
38+
proxies: MockWorkspaceProxies,
39+
},
40+
user: MockUser,
41+
supportLinks: MockSupportLinks,
42+
docsHref: "https://coder.com/docs",
43+
onSignOut: fn(),
44+
isDefaultOpen: true,
45+
canViewAuditLog: true,
46+
canViewDeployment: true,
47+
canViewHealth: true,
48+
canViewOrganizations: true,
49+
},
50+
decorators: [withNavbarMock],
51+
};
52+
53+
export default meta;
54+
type Story = StoryObj<typeof MobileMenu>;
55+
56+
export const Closed: Story = {
57+
args: {
58+
isDefaultOpen: false,
59+
},
60+
};
61+
62+
export const Admin: Story = {
63+
play: openAdminSettings,
64+
};
65+
66+
export const Auditor: Story = {
67+
args: {
68+
user: MockUser2,
69+
canViewAuditLog: true,
70+
canViewDeployment: false,
71+
canViewHealth: false,
72+
canViewOrganizations: false,
73+
},
74+
play: openAdminSettings,
75+
};
76+
77+
export const OrgAdmin: Story = {
78+
args: {
79+
user: MockUser2,
80+
canViewAuditLog: true,
81+
canViewDeployment: false,
82+
canViewHealth: false,
83+
canViewOrganizations: true,
84+
},
85+
play: openAdminSettings,
86+
};
87+
88+
export const Member: Story = {
89+
args: {
90+
user: MockUser2,
91+
canViewAuditLog: false,
92+
canViewDeployment: false,
93+
canViewHealth: false,
94+
canViewOrganizations: false,
95+
},
96+
};
97+
98+
export const ProxySettings: Story = {
99+
play: async ({ canvasElement }) => {
100+
const user = setupUser();
101+
const body = within(canvasElement.ownerDocument.body);
102+
const menuItem = await body.findByRole("menuitem", {
103+
name: /workspace proxy settings/i,
104+
});
105+
await user.click(menuItem);
106+
},
107+
};
108+
109+
export const UserSettings: Story = {
110+
play: async ({ canvasElement }) => {
111+
const user = setupUser();
112+
const body = within(canvasElement.ownerDocument.body);
113+
const menuItem = await body.findByRole("menuitem", {
114+
name: /user settings/i,
115+
});
116+
await user.click(menuItem);
117+
},
118+
};
119+
120+
function withNavbarMock(Story: FC) {
121+
return (
122+
<div className="h-[72px] border-0 border-b border-solid px-6 flex items-center justify-end">
123+
<Story />
124+
</div>
125+
);
126+
}
127+
128+
function setupUser() {
129+
// It seems the dropdown component is disabling pointer events, which is
130+
// causing Testing Library to throw an error. As a workaround, we can
131+
// disable the pointer events check.
132+
return userEvent.setup({
133+
pointerEventsCheck: PointerEventsCheckLevel.Never,
134+
});
135+
}
136+
137+
async function openAdminSettings({
138+
canvasElement,
139+
}: { canvasElement: HTMLElement }) {
140+
const user = setupUser();
141+
const body = within(canvasElement.ownerDocument.body);
142+
const menuItem = await body.findByRole("menuitem", {
143+
name: /admin settings/i,
144+
});
145+
await user.click(menuItem);
146+
}

0 commit comments

Comments
 (0)