fix(dashboard): address PR review — AntD buttons, public page guard, dedupe regex

- Replace raw <button> with AntD Button in BlogDropdown, NotificationsBell, UserDropdown, and test mock
- Guard NotificationsBell + container behind !isPublicPage to avoid rendering on public pages
- Remove redundant equality checks in navDisplayName (regex already covers them)
- Remove unused `lower` variable after simplification

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Krrish Dholakia 2026-05-09 13:51:40 -07:00
parent 37645756c7
commit 75f5fa1956
6 changed files with 25 additions and 30 deletions

View file

@ -78,10 +78,10 @@ export const BlogDropdown: React.FC = () => {
// Blog opens a post list; Docs is a single outbound link — navbar adds a layout-only chevron there for alignment.
return (
<Dropdown menu={{ items }} trigger={["hover"]} placement="bottomRight">
<button type="button" className={`${NAV_PRODUCT_LINK_CLASS} cursor-pointer border-0 bg-transparent`}>
<Button type="text" className={`${NAV_PRODUCT_LINK_CLASS} !border-0 !bg-transparent`}>
Blog
<DownOutlined className="text-[10px] text-gray-500" aria-hidden />
</button>
</Button>
</Dropdown>
);
};

View file

@ -52,15 +52,15 @@ export const NotificationsBell: React.FC = () => {
return (
<Popover content={content} trigger="click" open={open} onOpenChange={setOpen} placement="bottomRight">
<button
type="button"
className="flex h-9 w-9 cursor-pointer items-center justify-center rounded-md border-0 bg-transparent text-gray-600 transition-colors hover:bg-gray-100 hover:text-gray-900"
<Button
type="text"
className="!flex !h-9 !w-9 items-center justify-center !rounded-md text-gray-600 transition-colors hover:!bg-gray-100 hover:!text-gray-900"
aria-label="Notifications"
>
<Badge dot={hasUnread} color="#1677ff" size="small" offset={[8, 2]}>
<BellOutlined className="text-base" aria-hidden />
</Badge>
</button>
</Button>
</Popover>
);
};

View file

@ -19,7 +19,7 @@ import {
UserOutlined,
} from "@ant-design/icons";
import type { MenuProps } from "antd";
import { Divider, Dropdown, Space, Switch, Tag, Tooltip, Typography } from "antd";
import { Button, Divider, Dropdown, Space, Switch, Tag, Tooltip, Typography } from "antd";
import React, { useEffect, useState } from "react";
const { Text } = Typography;
@ -230,9 +230,9 @@ const UserDropdown: React.FC<UserDropdownProps> = ({ onLogout }) => {
</div>
)}
>
<button
type="button"
className="flex max-w-[min(200px,34vw)] items-center gap-2 rounded-md py-0.5 pl-1 pr-2 transition-colors hover:bg-gray-100"
<Button
type="text"
className="!flex max-w-[min(200px,34vw)] items-center gap-2 !rounded-md !py-0.5 !pl-1 !pr-2 transition-colors hover:!bg-gray-100"
aria-label={`Account menu — ${userRole ?? "Unknown role"} — signed in as ${userEmail || userId || "unknown"}`}
aria-haspopup="menu"
>
@ -247,7 +247,7 @@ const UserDropdown: React.FC<UserDropdownProps> = ({ onLogout }) => {
{displayName}
</span>
<DownOutlined className="hidden shrink-0 text-[10px] text-gray-400 md:inline" aria-hidden />
</button>
</Button>
</Dropdown>
);
};

View file

@ -8,12 +8,7 @@ export function navAccountDisplayName(userEmail: string | null, userId: string |
if (!id) {
return "Account";
}
const lower = id.toLowerCase();
if (
lower === "default_user_id" ||
lower === "default-user-id" ||
/^default[_\s-]?user[_\s-]?id$/i.test(id)
) {
if (/^default[_\s-]?user[_\s-]?id$/i.test(id)) {
return "Account";
}
return id;

View file

@ -30,6 +30,7 @@ const mockUserDropdownData = vi.hoisted(() => ({
vi.mock("./Navbar/UserDropdown/UserDropdown", async (importOriginal) => {
const React = await import("react");
const { useState } = React;
const { Button } = await import("antd");
const localStorageUtils = await import("@/utils/localStorageUtils");
return {
default: function MockUserDropdown({ onLogout }: { onLogout: () => void }) {
@ -37,9 +38,9 @@ vi.mock("./Navbar/UserDropdown/UserDropdown", async (importOriginal) => {
const [open, setOpen] = useState(false);
return (
<div>
<button type="button" aria-label="Open account menu" onClick={() => setOpen(!open)}>
<Button type="text" aria-label="Open account menu" onClick={() => setOpen(!open)}>
Account
</button>
</Button>
{open && (
<div data-testid="user-dropdown-content">
<span>{userId}</span>
@ -227,11 +228,12 @@ describe("Navbar", () => {
mockUseThemeImpl = () => ({ logoUrl: null });
});
it("should hide user dropdown on public pages", () => {
it("should hide user dropdown and notifications on public pages", () => {
const publicPageProps = { ...defaultProps, isPublicPage: true };
renderWithProviders(<Navbar {...publicPageProps} />);
expect(screen.queryByRole("button", { name: /open account menu/i })).not.toBeInTheDocument();
expect(screen.queryByRole("button", { name: /^notifications$/i })).not.toBeInTheDocument();
});
it("should handle hide new features toggle", async () => {

View file

@ -169,17 +169,15 @@ const Navbar: React.FC<NavbarProps> = ({
</div>
)}
<div className="flex shrink-0 items-center border-l border-gray-200 pl-4">
<div className="flex items-center gap-0.5 rounded-lg bg-gray-50 px-1 py-0 transition-colors hover:bg-gray-100">
<NotificationsBell />
{!isPublicPage && (
<>
<span className="mx-0.5 h-6 w-px shrink-0 bg-gray-200" aria-hidden />
<UserDropdown onLogout={handleLogout} />
</>
)}
{!isPublicPage && (
<div className="flex shrink-0 items-center border-l border-gray-200 pl-4">
<div className="flex items-center gap-0.5 rounded-lg bg-gray-50 px-1 py-0 transition-colors hover:bg-gray-100">
<NotificationsBell />
<span className="mx-0.5 h-6 w-px shrink-0 bg-gray-200" aria-hidden />
<UserDropdown onLogout={handleLogout} />
</div>
</div>
</div>
)}
</div>
{/* Dark mode toggle: keep disabled until the dashboard supports dark styles end-to-end. */}
</div>