Apply PR review suggestions

BrunoQuaresma · BrunoQuaresma · commit 07ce67758793 · 2024-12-30T13:20:41.000Z
diff --git a/site/src/components/Button/Button.tsx b/site/src/components/Button/Button.tsx
@@ -4,7 +4,7 @@
  */
 import { Slot } from "@radix-ui/react-slot";
 import { type VariantProps, cva } from "class-variance-authority";
-import { type FC, forwardRef } from "react";
+import { forwardRef } from "react";
 import { cn } from "utils/cn";
 
 export const buttonVariants = cva(
@@ -62,4 +62,3 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
 		);
 	},
 );
-Button.displayName = "Button";
diff --git a/site/src/index.css b/site/src/index.css
@@ -71,7 +71,14 @@
 		@apply border-border;
 	}
 
-	/** Related to https://github.com/radix-ui/primitives/issues/3251 */
+	/*
+	By default, Radix adds a margin to the `body` element when a dropdown is displayed,
+	causing some shifting when the dropdown has a full-width size, as is the case with the mobile menu.
+	To prevent this, we need to apply the styles below.
+
+	There’s a related issue on GitHub: Radix UI Primitives Issue #3251
+	https://github.com/radix-ui/primitives/issues/3251
+	 */
 	html body[data-scroll-locked] {
 		--removed-body-scroll-bar-size: 0 !important;
 		margin-right: 0 !important;
diff --git a/site/src/modules/dashboard/Navbar/MobileMenu.tsx b/site/src/modules/dashboard/Navbar/MobileMenu.tsx
@@ -34,6 +34,13 @@ const itemStyles = {
 	open: "text-content-primary",
 };
 
+type MobileMenuPermissions = {
+	canViewDeployment: boolean;
+	canViewOrganizations: boolean;
+	canViewAuditLog: boolean;
+	canViewHealth: boolean;
+};
+
 type MobileMenuProps = MobileMenuPermissions & {
 	proxyContextValue?: ProxyContextValue;
 	user?: TypesGen.User;
@@ -43,13 +50,6 @@ type MobileMenuProps = MobileMenuPermissions & {
 	isDefaultOpen?: boolean; // Useful for storybook
 };
 
-type MobileMenuPermissions = {
-	canViewDeployment: boolean;
-	canViewOrganizations: boolean;
-	canViewAuditLog: boolean;
-	canViewHealth: boolean;
-};
-
 export const MobileMenu: FC<MobileMenuProps> = ({
 	isDefaultOpen,
 	proxyContextValue,
@@ -136,7 +136,7 @@ const ProxySettingsSub: FC<ProxySettingsSubProps> = ({ proxyContextValue }) => {
 					}}
 				>
 					Workspace proxy settings:
-					<span className="leading-[0px] flex items-center gap-1">
+					<span className="leading-none flex items-center gap-1">
 						<img
 							className="w-4 h-4"
 							src={selectedProxy.icon_url}
diff --git a/site/src/modules/dashboard/Navbar/Navbar.test.tsx b/site/src/modules/dashboard/Navbar/Navbar.test.tsx
@@ -23,13 +23,7 @@ describe("Navbar", () => {
 		render(<App />);
 		const deploymentMenu = await screen.findByText("Admin settings");
 		await userEvent.click(deploymentMenu);
-		await waitFor(
-			() => {
-				const link = screen.getByText("Audit Logs");
-				expect(link).toBeDefined();
-			},
-			{ timeout: 2000 },
-		);
+		await screen.findByText("Audit Logs");
 	});
 
 	it("does not show Audit Log link when not entitled", async () => {
@@ -40,8 +34,7 @@ describe("Navbar", () => {
 		await userEvent.click(deploymentMenu);
 		await waitFor(
 			() => {
-				const link = screen.queryByText("Audit Logs");
-				expect(link).toBe(null);
+				expect(screen.queryByText("Audit Logs")).not.toBeInTheDocument();
 			},
 			{ timeout: 2000 },
 		);
@@ -63,8 +56,7 @@ describe("Navbar", () => {
 		render(<App />);
 		await waitFor(
 			() => {
-				const link = screen.queryByText("Deployment");
-				expect(link).toBe(null);
+				expect(screen.queryByText("Deployment")).not.toBeInTheDocument();
 			},
 			{ timeout: 2000 },
 		);
diff --git a/site/src/modules/dashboard/Navbar/NavbarView.test.tsx b/site/src/modules/dashboard/Navbar/NavbarView.test.tsx
@@ -35,8 +35,9 @@ describe("NavbarView", () => {
 				canViewAuditLog
 			/>,
 		);
-		const workspacesLink = await screen.findByText("Workspaces");
-		expect((workspacesLink as HTMLAnchorElement).href).toContain("/workspaces");
+		const workspacesLink =
+			await screen.findByText<HTMLAnchorElement>(/workspaces/i);
+		expect(workspacesLink.href).toContain("/workspaces");
 	});
 
 	it("templates nav link has the correct href", async () => {
@@ -52,8 +53,9 @@ describe("NavbarView", () => {
 				canViewAuditLog
 			/>,
 		);
-		const templatesLink = await screen.findByText("Templates");
-		expect((templatesLink as HTMLAnchorElement).href).toContain("/templates");
+		const templatesLink =
+			await screen.findByText<HTMLAnchorElement>(/templates/i);
+		expect(templatesLink.href).toContain("/templates");
 	});
 
 	it("audit nav link has the correct href", async () => {
@@ -71,8 +73,8 @@ describe("NavbarView", () => {
 		);
 		const deploymentMenu = await screen.findByText("Admin settings");
 		await userEvent.click(deploymentMenu);
-		const auditLink = await screen.findByText("Audit Logs");
-		expect((auditLink as HTMLAnchorElement).href).toContain("/audit");
+		const auditLink = await screen.findByText<HTMLAnchorElement>(/audit logs/i);
+		expect(auditLink.href).toContain("/audit");
 	});
 
 	it("deployment nav link has the correct href", async () => {
@@ -90,9 +92,8 @@ describe("NavbarView", () => {
 		);
 		const deploymentMenu = await screen.findByText("Admin settings");
 		await userEvent.click(deploymentMenu);
-		const deploymentSettingsLink = await screen.findByText("Deployment");
-		expect((deploymentSettingsLink as HTMLAnchorElement).href).toContain(
-			"/deployment/general",
-		);
+		const deploymentSettingsLink =
+			await screen.findByText<HTMLAnchorElement>(/deployment/i);
+		expect(deploymentSettingsLink.href).toContain("/deployment/general");
 	});
 });
diff --git a/site/src/modules/dashboard/Navbar/proxyUtils.tsx b/site/src/modules/dashboard/Navbar/proxyUtils.tsx
@@ -4,7 +4,7 @@ export function sortProxiesByLatency(
 	proxies: Proxies,
 	latencies: ProxyLatencies,
 ) {
-	return [...proxies].sort((a, b) => {
+	return proxies.toSorted((a, b) => {
 		const latencyA = latencies?.[a.id]?.latencyMS ?? Number.POSITIVE_INFINITY;
 		const latencyB = latencies?.[b.id]?.latencyMS ?? Number.POSITIVE_INFINITY;
 		return latencyA - latencyB;
