fix: dashboard flicker & improvements#1022
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReworks dashboard shell into a CSS grid, replaces the inline header with a new Top bar, adjusts desktop sidebar semantics/width, refines folder rename handlers, updates global scrollbar and grid rules, and adds an SVG Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router
participant Grid as dashboard-grid
participant Sidebar as Desktop Nav
participant Dashboard as DashboardInner
participant Top as Top Bar
participant Context as DashboardContext
participant Notifications
participant Theme
User->>Router: navigate to dashboard
Router->>Grid: render dashboard-grid
Grid->>Sidebar: mount Desktop Nav (motion.aside)
Grid->>Dashboard: mount DashboardInner (main area)
Dashboard->>Context: read activeOrganization/activeSpace
Dashboard->>Top: render Top with context
User->>Top: click bell
Top->>Notifications: toggle panel (click-away)
User->>Top: toggle theme
Top->>Theme: setTheme (use startViewTransition if available)
User->>Dashboard: rename folder
Dashboard->>Dashboard: prevent navigation, await rename update on Enter/blur
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/(org)/dashboard/caps/components/Folder.tsx (2)
274-281: Don’t nest a textarea (interactive) inside a Link (interactive).This is invalid HTML/a11y and can cause focus/nav bugs. Refactor so the card isn’t an when renaming is possible; navigate via onClick/router instead.
Here’s a minimal direction (remove Link wrapper and handle navigation on the card):
- <Link - prefetch={false} - href={ - spaceId - ? `/dashboard/spaces/${spaceId}/folder/${id}` - : `/dashboard/folder/${id}` - } - > + {/* Use router-based navigation; avoids nesting interactive content inside <a> */} <div ref={folderRef} + role="link" + tabIndex={0} + onClick={() => { + if (isRenaming) return; + router.push( + spaceId ? `/dashboard/spaces/${spaceId}/folder/${id}` : `/dashboard/folder/${id}`, + ); + }} + onKeyDown={(e) => { + if (isRenaming) return; + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + router.push( + spaceId ? `/dashboard/spaces/${spaceId}/folder/${id}` : `/dashboard/folder/${id}`, + ); + } + }} ... - </div> -</Link> + </div>If keeping , ensure the renaming UI is outside the anchor element.
Also applies to: 340-361
263-265: Inconsistent spaceId handling may break moves outside a space.registerDropTarget uses a fallback spaceId; handleDrop doesn’t. Align both to avoid undefined spaceId.
Apply this diff:
- await moveVideoToFolder({ videoId: capData.id, folderId: id, spaceId }); + await moveVideoToFolder({ + videoId: capData.id, + folderId: id, + spaceId: spaceId ?? activeOrganization?.organization.id, + });apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (1)
36-43: MembersDialog open state is unreachable — Top doesn't trigger itmembersDialogOpen is declared and passed to MembersDialog in apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (useState at line ~15; render at lines ~36–41), but there are no other callers of setMembersDialogOpen in the repo (search shows only this file). MembersDialog is a controlled component (apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx). Action: either lift the open state into the parent that renders Top or pass a callback to Top and call it from Top (example: <Top onOpenMembers={() => setMembersDialogOpen(true)} />) so the dialog can actually be opened.
🧹 Nitpick comments (18)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsx (1)
21-23: Make the accessible name configurable (prop) and expose it via aria-label.Defaulting to “Box” may be misleading in some contexts. Let callers override the label and set it both on <title> and aria-label for robust a11y.
Apply this diff:
interface ReferIconProps extends MotionProps { size?: number; + label?: string; } const ReferIcon = forwardRef<ReferIconHandle, ReferIconProps>( - ({ size = 28, ...props }, ref) => { + ({ size = 28, label = "Refer", ...props }, ref) => { ... - <motion.svg + <motion.svg width={size} height={size} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" + aria-label={label} variants={referVariants} animate={isAnimating ? "animate" : "normal"} onAnimationComplete={() => setIsAnimating(false)} {...props} > - <title>Box</title> + <title>{label}</title>Also applies to: 26-26, 44-55, 56-56
apps/web/app/(org)/dashboard/caps/components/Folder.tsx (6)
55-55: Use a cross‑env timeout type.NodeJS.Timeout is incorrect in the browser. Use ReturnType.
- const animationTimerRef = useRef<NodeJS.Timeout | null>(null); + const animationTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
352-357: Prevent newline on Enter in textarea.Enter currently inserts a newline before exiting rename. Stop the default.
onKeyDown={async (e) => { if (e.key === "Enter") { + e.preventDefault(); setIsRenaming(false); if (updateName.trim() !== name) { await updateFolderNameHandler(); } } }}
181-187: Trim before saving and avoid redundant state toggles.You compare with trim() but save the untrimmed value; also setIsRenaming(false) is duplicated here and in updateFolderNameHandler.
- await updateFolder({ folderId: id, name: updateName }); + await updateFolder({ folderId: id, name: updateName.trim() });onBlur={async () => { - setIsRenaming(false); if (updateName.trim() !== name) { await updateFolderNameHandler(); } }}if (e.key === "Enter") { - e.preventDefault(); - setIsRenaming(false); + e.preventDefault(); if (updateName.trim() !== name) { await updateFolderNameHandler(); } }Also applies to: 345-350, 353-356
249-253: Comment contradicts behavior.Comment says keep open after drop but you play “folder-close”. Fix the comment or animation for clarity.
- // Keep the folder open after a successful drop + // Close the folder after a successful drop
49-52: Remove unused field in dragStateRef.isAnimating is never read.
const dragStateRef = useRef({ isDragging: false, - isAnimating: false, });
322-325: Minor a11y: cursor semantics while renaming.Consider toggling cursor-text on the renaming area when isRenaming to reflect edit mode.
apps/web/app/globals.css (2)
55-66: Dashboard grid: consider explicit row sizing.Two identical “main” rows without template rows may be redundant. A single row or explicit grid-template-rows can simplify layout.
.dashboard-grid { display: grid; grid-template-columns: auto 1fr 1fr; - grid-template-areas: - "sidebar main main" - "sidebar main main"; + grid-template-areas: "sidebar main main"; + /* Optionally: grid-template-rows: 1fr; */ height: 100dvh; width: 100vw; min-height: 100dvh; }
311-318: Mobile grid collapse: OK; minor simplification possible.Single-column template can be one row.
@media all and (max-width: 1024px) { .dashboard-grid { grid-template-columns: 1fr; - grid-template-areas: - "main" - "main"; + grid-template-areas: "main"; } }apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsx (2)
45-46: Remove meaningless flex-1 on a grid item.flex-1 has no effect on a direct grid child; width is controlled by the animation.
- className={clsx( - "hidden relative z-50 flex-1 h-full [grid-area:sidebar] will-change-[width] lg:flex group bg-gray-1", - )} + className={clsx( + "hidden relative z-50 h-full [grid-area:sidebar] will-change-[width] lg:flex group bg-gray-1", + )}
68-81: Add an aria-label to the collapse toggle.Improves screen-reader discoverability.
<Button variant="white" onClick={toggleSidebarCollapsed} + aria-label="Toggle sidebar" className="absolute right-[-12px] size-7 p-0 min-w-[unset] top-[50%] transform -translate-y-1/2 rounded-full transition-colors z-10" >apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (1)
23-24: grid-area on a non-grid child is a no-op.Only direct children of the grid container participate. Remove [grid-area:main] here to reduce confusion; the parent already sets it.
- className={ - "flex relative flex-col flex-1 h-full [grid-area:main] bg-gray-1" - } + className={"flex relative flex-col flex-1 h-full bg-gray-1"}apps/web/app/(org)/dashboard/layout.tsx (1)
82-90: Place MobileTab inside the main grid area (or render it with [grid-area:main])layout.tsx currently renders as an unplaced sibling of the [grid-area:main] container (apps/web/app/(org)/dashboard/layout.tsx) — that will create implicit grid rows. MobileTab (apps/web/app/(org)/dashboard/_components/MobileTab.tsx) does not accept/forward className, so adding a prop to the component won't propagate. Fix by moving the component into the main area or by updating/wrapping MobileTab so it renders with [grid-area:main].
- <DashboardInner>{children}</DashboardInner> - </div> - <MobileTab /> + <DashboardInner>{children}</DashboardInner> + <MobileTab /> + </div>apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (5)
144-160: Make the theme toggle a button for keyboard/a11y.Current clickable div isn’t keyboard-focusable; use a button with an accessible label.
- <div - onClick={() => { - if (document.startViewTransition) { - document.startViewTransition(() => { - setThemeHandler(theme === "light" ? "dark" : "light"); - }); - } else { - setThemeHandler(theme === "light" ? "dark" : "light"); - } - }} - className="hidden justify-center items-center rounded-full transition-colors cursor-pointer bg-gray-3 lg:flex hover:bg-gray-5 size-9" - > + <button + type="button" + aria-label={theme === "light" ? "Switch to dark theme" : "Switch to light theme"} + onClick={() => { + const startViewTransition = + (document as any).startViewTransition?.bind(document); + if (typeof startViewTransition === "function") { + startViewTransition(() => + setThemeHandler(theme === "light" ? "dark" : "light"), + ); + } else { + setThemeHandler(theme === "light" ? "dark" : "light"); + } + }} + className="hidden justify-center items-center rounded-full transition-colors bg-gray-3 lg:flex hover:bg-gray-5 size-9" + > <FontAwesomeIcon className="text-gray-12 size-3.5 view-transition-theme-icon" icon={theme === "dark" ? faMoon : faSun} /> - </div> + </button>
240-270: Use a real button for the user menu trigger.Improves semantics, focus handling, and ARIA without custom key handlers.
- <PopoverTrigger asChild> - <div - data-state={menuOpen ? "open" : "closed"} - className="flex gap-2 justify-between items-center p-2 rounded-xl border data-[state=open]:border-gray-3 data-[state=open]:bg-gray-3 border-transparent transition-colors cursor-pointer group lg:gap-6 hover:border-gray-3" - > + <PopoverTrigger asChild> + <button + type="button" + aria-haspopup="menu" + aria-expanded={menuOpen} + data-state={menuOpen ? "open" : "closed"} + className="flex gap-2 justify-between items-center p-2 rounded-xl border data-[state=open]:border-gray-3 data-[state=open]:bg-gray-3 border-transparent transition-colors group lg:gap-6 hover:border-gray-3" + > ... - </div> + </button> </PopoverTrigger>
338-338: Add accessible label to Refer link (icon-only).Screen readers need a descriptive label.
- <Link href="/dashboard/refer" className="hidden relative lg:block"> + <Link + href="/dashboard/refer" + aria-label="Refer friends and earn 40% commission" + className="hidden relative lg:block" + >
172-231: Avoid staleuseMemodependencies for menu items.
isSubscribedchanges won’t update the menu. Add it to deps.- const menuItems = useMemo( + const menuItems = useMemo( () => [ ... ], - [], + [isSubscribed], );
1-1: Rename file to kebab-case per guidelines.
Top.tsx→top.tsx(and update imports).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/DashboardInner.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsx(2 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/Folder.tsx(3 hunks)apps/web/app/(org)/dashboard/layout.tsx(1 hunks)apps/web/app/globals.css(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsxapps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/caps/components/Folder.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsxapps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/caps/components/Folder.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsxapps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/caps/components/Folder.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsxapps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/_components/DashboardInner.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/caps/components/Folder.tsx
🧬 Code graph analysis (3)
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (4)
apps/web/app/(org)/dashboard/Contexts.tsx (2)
useDashboardContext(42-42)useTheme(40-40)apps/web/components/UpgradeModal.tsx (1)
UpgradeModal(59-305)apps/web/app/(org)/dashboard/_components/AnimatedIcons/Download.tsx (1)
DownloadIconHandle(9-12)apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsx (1)
ReferIconHandle(16-19)
apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (1)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(42-42)
apps/web/app/(org)/dashboard/layout.tsx (1)
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsx (1)
DesktopNav(13-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Refer.tsx (1)
56-56: A11y label addition looks good.Adding an SVG <title> improves accessible naming.
apps/web/app/globals.css (1)
82-85: Scrollbar tweaks LGTM.Thumb colors + radius for light/dark are reasonable defaults.
Also applies to: 86-90
apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (1)
20-21: Header integration LGTM.Delegating header UI to Top simplifies this component.
apps/web/app/(org)/dashboard/caps/components/Folder.tsx (1)
1-403: Do not rename this file in isolation — repo uses PascalCase filenames.Multiple TS/TSX files under apps/web (including apps/web/app/(org)/dashboard/caps/components/Folder.tsx) use PascalCase; renaming only this file to kebab-case would be inconsistent. Either perform a repo-wide kebab-case migration with lint/CI, or keep the existing PascalCase convention.
Likely an incorrect or invalid review comment.
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (1)
141-142: VerifyNotificationsforwards its ref.
useClickAwayexpects the ref to be attached to the root element insideNotifications. If it doesn’t forward a ref to a focusable/container element, click-away may misfire.
| const notificationsRef: MutableRefObject<HTMLDivElement> = useClickAway( | ||
| (e) => { | ||
| if (bellRef.current && !bellRef.current.contains(e.target as Node)) { | ||
| setToggleNotifications(false); | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
TS strictness: nullability and experimental API types will fail builds.
useClickAwayref must allownull(and can use its generic).document.startViewTransitionneeds a safe/typed access.useRef<...>(null)must include| null.
- const notificationsRef: MutableRefObject<HTMLDivElement> = useClickAway(
+ const notificationsRef = useClickAway<HTMLDivElement>(
(e) => {
if (bellRef.current && !bellRef.current.contains(e.target as Node)) {
setToggleNotifications(false);
}
},
);
- if (document.startViewTransition) {
- document.startViewTransition(() => {
- setThemeHandler(theme === "light" ? "dark" : "light");
- });
- } else {
- setThemeHandler(theme === "light" ? "dark" : "light");
- }
+ const startViewTransition =
+ (document as any).startViewTransition?.bind(document);
+ if (typeof startViewTransition === "function") {
+ startViewTransition(() => {
+ setThemeHandler(theme === "light" ? "dark" : "light");
+ });
+ } else {
+ setThemeHandler(theme === "light" ? "dark" : "light");
+ }
- const iconRef = useRef<DownloadIconHandle>(null);
+ const iconRef = useRef<DownloadIconHandle | null>(null);
- const iconRef = useRef<ReferIconHandle>(null);
+ const iconRef = useRef<ReferIconHandle | null>(null);Also applies to: 146-153, 303-303, 335-335
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx around lines 66-72
(and similarly at 146-153, 303, 335), the refs and experimental API uses are not
typed for nullability or safe access: change the useClickAway ref to use the
generic and allow null (e.g., MutableRefObject<HTMLDivElement | null> by using
useClickAway<HTMLDivElement | null>(...)), update every useRef<T>(null) to
useRef<T | null>(null), and guard/cast accesses to document.startViewTransition
with a runtime/type-safe check (e.g., check typeof (document as
any).startViewTransition === "function" or narrow document with an interface
that has optional startViewTransition before calling) so TypeScript strict mode
and experimental API typings do not fail builds.
| </div> | ||
| </div> | ||
| <div className="flex gap-4 items-center"> | ||
| {buildEnv.NEXT_PUBLIC_IS_CAP && <ReferButton />} |
There was a problem hiding this comment.
Fix env flag checks (string vs boolean) to avoid incorrect UI gating.
buildEnv.NEXT_PUBLIC_IS_CAP is a string (e.g., "true"/"false"). Current truthy checks will render CAP-only UI even when set to "false".
- {buildEnv.NEXT_PUBLIC_IS_CAP && <ReferButton />}
+ {buildEnv.NEXT_PUBLIC_IS_CAP === "true" && <ReferButton />}
- showCondition: !isSubscribed && buildEnv.NEXT_PUBLIC_IS_CAP,
+ showCondition: !isSubscribed && buildEnv.NEXT_PUBLIC_IS_CAP === "true",
- showCondition: buildEnv.NEXT_PUBLIC_IS_CAP,
+ showCondition: buildEnv.NEXT_PUBLIC_IS_CAP === "true",Also applies to: 190-191, 198-199
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx around lines 106,
190-191, and 198-199, the code currently uses the string env variable
buildEnv.NEXT_PUBLIC_IS_CAP in a truthy check which will render CAP-only UI when
the variable is the string "false"; change each conditional to explicitly check
the string value (for example normalize and compare:
String(buildEnv.NEXT_PUBLIC_IS_CAP).toLowerCase() === 'true' or
buildEnv.NEXT_PUBLIC_IS_CAP === 'true') so the UI is gated only when the env
flag is actually set to "true".
| const MenuItem = memo(({ icon, name, href, onClick, iconClassName }: Props) => { | ||
| const iconRef = useRef<DownloadIconHandle>(null); | ||
| return ( | ||
| <CommandItem | ||
| key={name} | ||
| className="px-2 py-1.5 rounded-lg transition-colors duration-300 cursor-pointer hover:bg-gray-5 group" | ||
| onSelect={onClick} | ||
| onMouseEnter={() => { | ||
| iconRef.current?.startAnimation(); | ||
| }} | ||
| onMouseLeave={() => { | ||
| iconRef.current?.stopAnimation(); | ||
| }} | ||
| > | ||
| <Link | ||
| className="flex gap-2 items-center w-full" | ||
| href={href ?? "#"} | ||
| onClick={onClick} | ||
| > | ||
| <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5"> | ||
| {cloneElement(icon, { | ||
| ref: iconRef, | ||
| className: iconClassName, | ||
| size: 14, | ||
| })} | ||
| </div> | ||
| <p className={clsx("text-sm text-gray-12")}>{name}</p> | ||
| </Link> | ||
| </CommandItem> | ||
| ); |
There was a problem hiding this comment.
Prevent double-invocation on menu item clicks; avoid nested interactive conflicts.
CommandItem's onSelect and the inner Link’s onClick both call handlers, causing duplicate actions (e.g., double signOut, modal opens twice).
- <CommandItem
- key={name}
- className="px-2 py-1.5 rounded-lg transition-colors duration-300 cursor-pointer hover:bg-gray-5 group"
- onSelect={onClick}
- onMouseEnter={() => {
- iconRef.current?.startAnimation();
- }}
- onMouseLeave={() => {
- iconRef.current?.stopAnimation();
- }}
- >
- <Link
- className="flex gap-2 items-center w-full"
- href={href ?? "#"}
- onClick={onClick}
- >
+ <CommandItem
+ key={name}
+ className="px-2 py-1.5 rounded-lg transition-colors duration-300 cursor-pointer hover:bg-gray-5 group"
+ onMouseEnter={() => {
+ iconRef.current?.startAnimation();
+ }}
+ onMouseLeave={() => {
+ iconRef.current?.stopAnimation();
+ }}
+ >
+ {href ? (
+ <Link className="flex gap-2 items-center w-full" href={href} onClick={onClick}>
+ <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5">
+ {cloneElement(icon, { ref: iconRef, className: iconClassName, size: 14 })}
+ </div>
+ <p className={clsx("text-sm text-gray-12")}>{name}</p>
+ </Link>
+ ) : (
+ <button
+ type="button"
+ className="flex gap-2 items-center w-full text-left"
+ onClick={onClick}
+ >
+ <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5">
+ {cloneElement(icon, { ref: iconRef, className: iconClassName, size: 14 })}
+ </div>
+ <p className={clsx("text-sm text-gray-12")}>{name}</p>
+ </button>
+ )}
- <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5">
- {cloneElement(icon, {
- ref: iconRef,
- className: iconClassName,
- size: 14,
- })}
- </div>
- <p className={clsx("text-sm text-gray-12")}>{name}</p>
- </Link>
</CommandItem>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const MenuItem = memo(({ icon, name, href, onClick, iconClassName }: Props) => { | |
| const iconRef = useRef<DownloadIconHandle>(null); | |
| return ( | |
| <CommandItem | |
| key={name} | |
| className="px-2 py-1.5 rounded-lg transition-colors duration-300 cursor-pointer hover:bg-gray-5 group" | |
| onSelect={onClick} | |
| onMouseEnter={() => { | |
| iconRef.current?.startAnimation(); | |
| }} | |
| onMouseLeave={() => { | |
| iconRef.current?.stopAnimation(); | |
| }} | |
| > | |
| <Link | |
| className="flex gap-2 items-center w-full" | |
| href={href ?? "#"} | |
| onClick={onClick} | |
| > | |
| <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5"> | |
| {cloneElement(icon, { | |
| ref: iconRef, | |
| className: iconClassName, | |
| size: 14, | |
| })} | |
| </div> | |
| <p className={clsx("text-sm text-gray-12")}>{name}</p> | |
| </Link> | |
| </CommandItem> | |
| ); | |
| const MenuItem = memo(({ icon, name, href, onClick, iconClassName }: Props) => { | |
| const iconRef = useRef<DownloadIconHandle>(null); | |
| return ( | |
| <CommandItem | |
| key={name} | |
| className="px-2 py-1.5 rounded-lg transition-colors duration-300 cursor-pointer hover:bg-gray-5 group" | |
| onMouseEnter={() => { | |
| iconRef.current?.startAnimation(); | |
| }} | |
| onMouseLeave={() => { | |
| iconRef.current?.stopAnimation(); | |
| }} | |
| > | |
| {href ? ( | |
| <Link className="flex gap-2 items-center w-full" href={href} onClick={onClick}> | |
| <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5"> | |
| {cloneElement(icon, { ref: iconRef, className: iconClassName, size: 14 })} | |
| </div> | |
| <p className={clsx("text-sm text-gray-12")}>{name}</p> | |
| </Link> | |
| ) : ( | |
| <button | |
| type="button" | |
| className="flex gap-2 items-center w-full text-left" | |
| onClick={onClick} | |
| > | |
| <div className="flex-shrink-0 flex items-center justify-center w-3.5 h-3.5"> | |
| {cloneElement(icon, { ref: iconRef, className: iconClassName, size: 14 })} | |
| </div> | |
| <p className={clsx("text-sm text-gray-12")}>{name}</p> | |
| </button> | |
| )} | |
| </CommandItem> | |
| ); |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx around lines 302-331,
the MenuItem currently passes the same handler to CommandItem.onSelect and the
inner Link.onClick causing duplicate invocations; fix by removing the onClick
from the inner Link (leave href for navigation and keep Link as non-interactive
wrapper) and centralize the action in CommandItem.onSelect (move any navigation
or action logic into the onSelect handler so keyboard/mouse both trigger exactly
once), ensuring no nested interactive handlers remain.
1 participant
This PR:
Summary by CodeRabbit