chore: github button in navbar #1101
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughResponsive breakpoints in the Navbar and MobileMenu were shifted from md to lg (and md:hidden removed), a GitHub button using next/image was added to the Navbar, and the UI Button gained a Changes
Sequence Diagram(s)(omitted — changes are layout/responsive and prop forwarding; no new control-flow to diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/public/github.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
apps/web/app/(site)/Navbar.tsx(6 hunks)apps/web/components/ui/MobileMenu.tsx(1 hunks)packages/ui/src/components/Button.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/ui/src/components/Button.tsxapps/web/components/ui/MobileMenu.tsxapps/web/app/(site)/Navbar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/ui/src/components/Button.tsxapps/web/components/ui/MobileMenu.tsxapps/web/app/(site)/Navbar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/ui/src/components/Button.tsxapps/web/components/ui/MobileMenu.tsxapps/web/app/(site)/Navbar.tsx
packages/ui/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Component files in
packages/uishould use PascalCase naming if they define React/Solid components.
Files:
packages/ui/src/components/Button.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/components/ui/MobileMenu.tsxapps/web/app/(site)/Navbar.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/components/ui/MobileMenu.tsxapps/web/app/(site)/Navbar.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(site)/Navbar.tsx
🪛 GitHub Check: Typecheck
apps/web/app/(site)/Navbar.tsx
[failure] 220-220:
Type '{ children: string; variant: "outline"; icon: Element; target: string; href: string; size: "sm"; className: string; }' is not assignable to type 'IntrinsicAttributes & ButtonProps & RefAttributes'.
⏰ 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). (4)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
packages/ui/src/components/Button.tsx (1)
21-22: LGTM!The outline variant styling refinement looks good. The updated classes provide clear hover and disabled states using consistent gray-scale tokens.
apps/web/app/(site)/Navbar.tsx (3)
17-17: LGTM!The Image import is correctly added to support the GitHub button icon.
134-135: LGTM!The responsive breakpoint changes from
mdtolgare consistently applied across the navbar positioning, navigation visibility, and mobile menu button. This creates a cohesive responsive layout strategy.Also applies to: 149-149, 254-254
172-174: LGTM!The minor layout adjustments in the dropdown items are appropriate refinements.
apps/web/components/ui/MobileMenu.tsx (1)
60-60: No action needed: MobileMenu visibility is properly controlled
The Navbar usesshowMobileMenustate (line 117) with{showMobileMenu && <MobileMenu …/>}(lines 290–292), the hamburger toggle is hidden atlg(className="flex lg:hidden"on line 254), and the desktop nav is shown atlg(className="hidden lg:flex"on lines 149 & 209).
| <Button | ||
| variant="outline" | ||
| icon={ | ||
| <Image | ||
| src="/github.svg" | ||
| alt="Github" | ||
| width={16} | ||
| height={16} | ||
| /> | ||
| } | ||
| target="_blank" | ||
| href="https://github.com/CapSoftware/Cap" | ||
| size="sm" | ||
| className="w-full font-medium sm:w-auto" | ||
| > | ||
| Github | ||
| </Button> |
There was a problem hiding this comment.
Remove invalid target prop to fix type error.
The target="_blank" prop on line 220 is not valid for the Button component. While Button renders as an anchor when href is provided, ButtonProps only extends ButtonHTMLAttributes<HTMLButtonElement>, not AnchorHTMLAttributes<HTMLAnchorElement>.
Apply this diff to remove the invalid prop:
<Button
variant="outline"
icon={
<Image
src="/github.svg"
alt="Github"
width={16}
height={16}
/>
}
- target="_blank"
href="https://github.com/CapSoftware/Cap"
size="sm"
className="w-full font-medium sm:w-auto"
>
Github
</Button>Alternatively, if opening in a new tab is required, the Button component's type definition should be updated to conditionally include anchor attributes when href is present.
📝 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.
| <Button | |
| variant="outline" | |
| icon={ | |
| <Image | |
| src="/github.svg" | |
| alt="Github" | |
| width={16} | |
| height={16} | |
| /> | |
| } | |
| target="_blank" | |
| href="https://github.com/CapSoftware/Cap" | |
| size="sm" | |
| className="w-full font-medium sm:w-auto" | |
| > | |
| Github | |
| </Button> | |
| <Button | |
| variant="outline" | |
| icon={ | |
| <Image | |
| src="/github.svg" | |
| alt="Github" | |
| width={16} | |
| height={16} | |
| /> | |
| } | |
| href="https://github.com/CapSoftware/Cap" | |
| size="sm" | |
| className="w-full font-medium sm:w-auto" | |
| > | |
| Github | |
| </Button> |
🧰 Tools
🪛 GitHub Check: Typecheck
[failure] 220-220:
Type '{ children: string; variant: "outline"; icon: Element; target: string; href: string; size: "sm"; className: string; }' is not assignable to type 'IntrinsicAttributes & ButtonProps & RefAttributes'.
🤖 Prompt for AI Agents
In apps/web/app/(site)/Navbar.tsx around lines 210 to 226, the Button usage
includes an invalid target="_blank" prop (line ~220) because ButtonProps extend
button attributes, not anchor attributes; remove the target="_blank" prop from
this Button invocation to fix the type error; if opening in a new tab is
required instead, either render a plain <a> with the href and target/rel
attributes or update the Button component's type definition to conditionally
include AnchorHTMLAttributes when href is provided and ensure rel="noopener
noreferrer" is added when using target="_blank".
Summary by CodeRabbit
New Features
Style
Refactor
Chores