feat/header-dropdown-menu-update#2088
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughRefactors header and footer into smaller internal components, adds SidebarNavigation and getDocsBySection for docs/handbook sidebars, introduces HandbookDrawerContext and useHandbookDrawer, wires mobile docs/handbook drawers with scroll refs, and removes the FAQ route. Public exports remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Header
participant DrawerCtx as Drawer Contexts
participant SidebarNav as SidebarNavigation
participant DocsSvc as Docs/Handbook Data (getDocsBySection / allHandbooks)
participant Router
User->>Header: open mobile menu / handbook toggle
Header->>DrawerCtx: setIsOpen(true) for docs or handbook
Header->>SidebarNav: render with scrollContainerRef and linkTo
SidebarNav->>DocsSvc: request sections data
DocsSvc-->>SidebarNav: return sections
SidebarNav->>DrawerCtx: onLinkClick -> setIsOpen(false)
SidebarNav->>Router: navigate to selected doc
Router-->>User: route/content updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/components/footer.tsx (1)
4-8: Redundant conditional: both branches return the same value.The
isBlogOrDocscheck has no effect since both theifandelsebranches return"max-w-6xl". Either simplify by removing the conditional or correct the intended logic if different widths were meant for different routes.function getMaxWidthClass(pathname: string): string { - const isBlogOrDocs = - pathname.startsWith("/blog") || pathname.startsWith("/docs"); - return isBlogOrDocs ? "max-w-6xl" : "max-w-6xl"; + return "max-w-6xl"; }apps/web/src/components/header.tsx (1)
22-26: Dead code: both branches return the same value.The conditional returns
"max-w-6xl"regardless ofisBlogOrDocs. Either the condition is unnecessary or one branch should use a different value.-function getMaxWidthClass(pathname: string): string { - const isBlogOrDocs = - pathname.startsWith("/blog") || pathname.startsWith("/docs"); - return isBlogOrDocs ? "max-w-6xl" : "max-w-6xl"; -} +function getMaxWidthClass(_pathname: string): string { + return "max-w-6xl"; +}Or if different widths were intended, correct the second value.
apps/web/src/routes/_view/docs/route.tsx (1)
47-84:DocsNavigationis duplicated.This component is defined identically in both
apps/web/src/routes/_view/docs/route.tsx(here) andapps/web/src/routes/_view/route.tsx. Extract to a shared location.Create a shared component file:
// apps/web/src/components/docs-navigation.tsx export function DocsNavigation({ sections, currentSlug, onLinkClick }: {...}) { // ... shared implementation }Then import in both route files.
♻️ Duplicate comments (2)
apps/web/src/routes/_view/route.tsx (2)
80-80: Same memoization concern applies here.
getDocsBySection()is called on every render. Consider memoizing as suggested indocs/route.tsx.
99-136: DuplicateDocsNavigationcomponent.This is the same duplication issue flagged in
docs/route.tsx. Extract to a shared component.
🧹 Nitpick comments (3)
apps/web/src/components/footer.tsx (1)
155-162: Inconsistent link type for FAQ.The FAQ link uses a plain
<a>tag for/docs/faq, which appears to be an internal route. Other internal routes in this file use theLinkcomponent from@tanstack/react-routerfor proper SPA navigation. If this is intentional (e.g., triggering a full page load), consider adding a comment explaining why.<li> - <a - href="/docs/faq" + <Link + to="/docs/faq" className="text-sm text-neutral-600 hover:text-stone-600 transition-colors no-underline hover:underline hover:decoration-dotted" > FAQ - </a> + </Link> </li>apps/web/src/components/header.tsx (1)
198-245: Consider extracting shared list rendering logic.
ProductsListandFeaturesList(lines 198-246) are nearly identical, differing only in data source and header text. Similarly for their mobile counterparts (lines 474-530). Consider a single reusable component:function ItemList({ title, items, onClose }: { title: string; items: typeof productsList; onClose: () => void; }) { return ( <div> <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> {title} </div> {items.map((link) => ( <Link key={link.to} to={link.to} onClick={onClose} className="py-2 text-sm text-neutral-700 flex items-center justify-between hover:underline decoration-dotted" > <span>{link.label}</span> {link.badge && ( <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> {link.badge} </span> )} </Link> ))} </div> ); }Then use
<ItemList title="Products" items={productsList} onClose={onClose} />.apps/web/src/routes/_view/docs/route.tsx (1)
36-36: Consider memoizinggetDocsBySection()result.
getDocsBySection()iterates overallDocs, groups, sorts, and filters on every render. SinceallDocsis static content, the result could be computed once outside the component or memoized.Option 1 - Compute once at module level in
-structure.ts:// In -structure.ts const _cachedSections = null as ReturnType<typeof getDocsBySection> | null; export function getDocsBySection() { if (_cachedSections) return _cachedSections; // ... existing logic }Option 2 - Use
useMemoin component:const { sections } = useMemo(() => getDocsBySection(), []);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/src/routeTree.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (6)
apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/header.tsx(1 hunks)apps/web/src/routes/_view/docs/-structure.ts(2 hunks)apps/web/src/routes/_view/docs/route.tsx(2 hunks)apps/web/src/routes/_view/faq.tsx(0 hunks)apps/web/src/routes/_view/route.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/routes/_view/faq.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/src/routes/_view/docs/-structure.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/docs/-structure.tsapps/web/src/components/footer.tsxapps/web/src/components/header.tsxapps/web/src/routes/_view/route.tsxapps/web/src/routes/_view/docs/route.tsx
🧬 Code graph analysis (3)
apps/web/src/components/header.tsx (2)
apps/web/src/hooks/use-docs-drawer.ts (1)
useDocsDrawer(12-14)apps/web/src/hooks/use-platform.ts (1)
getPlatformCTA(35-43)
apps/web/src/routes/_view/route.tsx (1)
apps/web/src/routes/_view/docs/-structure.ts (1)
getDocsBySection(13-52)
apps/web/src/routes/_view/docs/route.tsx (1)
apps/web/src/routes/_view/docs/-structure.ts (1)
getDocsBySection(13-52)
⏰ 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: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (11)
apps/web/src/components/footer.tsx (7)
10-27: LGTM!The refactored
Footercomponent has a clean composition usingBrandSectionandLinksGrid, improving readability and maintainability.
29-68: LGTM!Clean extraction with inline prop types as per coding guidelines.
70-80: LGTM!Simple grid composition that organizes link sections clearly.
82-146: LGTM!Proper use of
Linkfor internal routes and<a>with appropriate security attributes for external links.
212-262: LGTM!Consistent use of
Linkcomponent for all internal routes.
264-291: LGTM!The
search={{ id: undefined }}pattern on the file-transcription link appears intentional to clear any existing search params.
293-358: LGTM!The mix of relative paths (
/x,/discord,/youtube) withtarget="_blank"appears to be using server-side redirects, while other links use full URLs directly. Both patterns work correctly.apps/web/src/components/header.tsx (1)
61-91: Good modular refactor of the header component.The componentization into
LeftNav,DesktopNav,MobileNav, andMobileMenuimproves readability and maintainability. State coordination between docs drawer and mobile menu (closing one when opening the other) is handled correctly.apps/web/src/routes/_view/docs/-structure.ts (2)
13-52: Clean extraction of docs section grouping logic.The function correctly groups docs by section, sorts by order, and returns sections in the predefined structure order. The type predicate in the filter is well-done.
3-11: ThedocsStructureconfiguration is correct and requires no changes.The FAQ documentation files exist (apps/web/content/docs/faq/*.mdx), are properly integrated with the
section: "FAQ"metadata, and are referenced in the sitemap and robots.txt. The"faq"entry indocsStructure.sectionsand thedefaultPages.faqentry are accurate and necessary for the documentation navigation system to function correctly.apps/web/src/routes/_view/route.tsx (1)
32-64: Good integration of the docs drawer with the centralized section logic.The
MobileDocsDrawercorrectly integrates with the newgetDocsBySection()helper and maintains proper state management through context.
Ensure the sidebar automatically scrolls the active documentation link into view when the docs route initializes. Added a ref for the active link and a useEffect that calls scrollIntoView centered when currentSlug changes so the current section is visible on load or navigation.
Ensure only the sidebar scrolls when navigating docs so the whole page no longer jumps. Pass a scrollContainerRef into DocsNavigation, compute the active link's position relative to that container, and set container.scrollTop instead of calling scrollIntoView. This keeps scrolling confined to the sidebar and centers the active item within it.
Make the table-of-contents scroll happen immediately after page updates by wrapping the scroll calculation and assignment in requestAnimationFrame. This removes the visible delay that occurred when calculating getBoundingClientRect and setting scrollTop synchronously, ensuring the active link is centered promptly after the page settles.
The scrollContainerRef prop was typed as React.RefObject<HTMLDivElement>, but the code provides a RefObject that may contain null. Update the prop type to React.RefObject<HTMLDivElement | null> to match TypeScript's expected nullable ref type and fix TS2322. This ensures the component accepts standard React refs that can be null when unmounted or before mounting.
ComputelessComputer
force-pushed
the
feat/header-dropdown-menu-update
branch
from
ebe5922 to
91f04f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/header.tsx (1)
22-26: Redundant ternary: both branches return the same value.The
isBlogOrDocscondition evaluates but returns"max-w-6xl"in both branches, making the conditional logic ineffective. Either this is incomplete (different max-width classes were intended) or the condition should be removed.function getMaxWidthClass(pathname: string): string { - const isBlogOrDocs = - pathname.startsWith("/blog") || pathname.startsWith("/docs"); - return isBlogOrDocs ? "max-w-6xl" : "max-w-6xl"; + return "max-w-6xl"; }
♻️ Duplicate comments (1)
apps/web/src/components/header.tsx (1)
480-536: Same duplication pattern as desktop lists.The same refactoring opportunity applies here:
MobileProductsListandMobileFeaturesListcould share a commonMobileCategoryListcomponent. This would complement the earlier suggestion for desktop lists.
🧹 Nitpick comments (5)
apps/web/src/routes/_view/docs/route.tsx (2)
67-87: Auto-scroll implementation works correctly.The effect properly centers the active link when the route changes. The use of
requestAnimationFrameensures accurate measurements after layout.Consider adding smooth scrolling for better UX:
- container.scrollTop = scrollTop; + container.scrollTo({ top: scrollTop, behavior: 'smooth' });
97-115: Consider usingcnutility for className logic.The conditional className logic works but could be cleaner using the
cnutility, aligning with the project's coding guidelines.Apply this diff:
+import { cn } from "@hypr/utils"; {section.docs.map((doc) => { const isActive = currentSlug === doc.slug; return ( <Link key={doc.slug} to="/docs/$" params={{ _splat: doc.slug }} onClick={onLinkClick} ref={isActive ? activeLinkRef : undefined} - className={`block pl-5 pr-3 py-1.5 text-sm rounded-sm transition-colors ${ - isActive - ? "bg-neutral-100 text-stone-600 font-medium" - : "text-neutral-600 hover:text-stone-600 hover:bg-neutral-50" - }`} + className={cn([ + "block pl-5 pr-3 py-1.5 text-sm rounded-sm transition-colors", + isActive + ? "bg-neutral-100 text-stone-600 font-medium" + : "text-neutral-600 hover:text-stone-600 hover:bg-neutral-50", + ])} > {doc.title} </Link> ); })}As per coding guidelines: "If there are many classNames with conditional logic, use
cn(import from@hypr/utils)."apps/web/src/routes/_view/route.tsx (1)
122-126: Consider usingcnutility for conditional className.The className contains conditional logic with a ternary operator. According to the coding guidelines, consider using the
cnutility (from@hypr/utils) to improve readability and consistency.Apply this diff:
+import { cn } from "@hypr/utils";- className={`block pl-5 pr-3 py-1.5 text-sm rounded-sm transition-colors ${ - currentSlug === doc.slug - ? "bg-neutral-100 text-stone-600 font-medium" - : "text-neutral-600 hover:text-stone-600 hover:bg-neutral-50" - }`} + className={cn([ + "block pl-5 pr-3 py-1.5 text-sm rounded-sm transition-colors", + currentSlug === doc.slug + ? "bg-neutral-100 text-stone-600 font-medium" + : "text-neutral-600 hover:text-stone-600 hover:bg-neutral-50", + ])}As per coding guidelines, use array syntax and split by logical grouping when using
cn.apps/web/src/components/header.tsx (2)
198-246: Consider extracting a shared list component.
ProductsListandFeaturesListare nearly identical, differing only in the data source and heading. A sharedCategoryListcomponent acceptingtitleanditemsprops would reduce duplication.function CategoryList({ title, items, onClose }: { title: string; items: typeof productsList; onClose: () => void; }) { return ( <div> <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> {title} </div> {items.map((link) => ( <Link key={link.to} to={link.to} onClick={onClose} className="py-2 text-sm text-neutral-700 flex items-center justify-between hover:underline decoration-dotted" > <span>{link.label}</span> {link.badge && ( <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> {link.badge} </span> )} </Link> ))} </div> ); }
340-343: Consider usingcnfor conditional class logic.Per coding guidelines, use
cnfrom@hypr/utilswhen there are classNames with conditional logic. This improves readability:+import { cn } from "@hypr/utils"; - const baseClass = mobile - ? "px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all" - : "px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md hover:shadow-lg hover:scale-[102%] active:scale-[98%] transition-all"; + const baseClass = cn([ + "px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all", + !mobile && "hover:shadow-lg hover:scale-[102%]", + ]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/src/routeTree.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (6)
apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/header.tsx(1 hunks)apps/web/src/routes/_view/docs/-structure.ts(2 hunks)apps/web/src/routes/_view/docs/route.tsx(4 hunks)apps/web/src/routes/_view/faq.tsx(0 hunks)apps/web/src/routes/_view/route.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/routes/_view/faq.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/footer.tsx
- apps/web/src/routes/_view/docs/-structure.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/route.tsxapps/web/src/routes/_view/docs/route.tsxapps/web/src/components/header.tsx
🧬 Code graph analysis (3)
apps/web/src/routes/_view/route.tsx (1)
apps/web/src/routes/_view/docs/-structure.ts (1)
getDocsBySection(13-52)
apps/web/src/routes/_view/docs/route.tsx (1)
apps/web/src/routes/_view/docs/-structure.ts (1)
getDocsBySection(13-52)
apps/web/src/components/header.tsx (2)
apps/web/src/hooks/use-docs-drawer.ts (1)
useDocsDrawer(12-14)apps/web/src/hooks/use-platform.ts (1)
getPlatformCTA(35-43)
⏰ 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: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (6)
apps/web/src/routes/_view/docs/route.tsx (3)
8-8: LGTM! Clean extraction of section grouping logic.The refactoring successfully extracts the section grouping logic into a reusable helper function, improving maintainability.
Also applies to: 10-10, 37-37
38-38: LGTM! Proper ref setup for scroll container.The scrollContainerRef is correctly created, typed, and passed to enable programmatic scrolling.
Also applies to: 42-49
56-66: LGTM! Type signature is correct.The scrollContainerRef parameter is properly typed and optional, allowing flexible usage of DocsNavigation.
apps/web/src/components/header.tsx (3)
44-91: Clean modular composition.The Header component has been well-refactored into logical sub-components (LeftNav, DesktopNav, MobileNav, MobileMenu) with clear prop passing. State management is appropriate for this UI pattern.
123-154: Good accessibility and null-safety.The DocsDrawerButton properly handles the null case for
docsDrawerand includes appropriatearia-labelfor screen readers. The interaction logic to close the mobile menu when opening the docs drawer prevents confusing overlapping UI states.
298-329: Good accessibility attributes on mobile menu toggle.The menu toggle button includes
aria-labelandaria-expandedattributes, which properly communicate the menu state to assistive technologies.
Add a mobile drawer for the company handbook sidebar so the left navigation is available on small screens. Introduce a floating MobileMenuButton, a BottomSheet-based MobileMenu, and a shared HandbookNavigation component that handles scrolling to the active link. Refactor LeftSidebar to pass a scrollContainerRef into the navigation and wire up state (isMobileMenuOpen) in the route component to open/close the mobile drawer.
Replace in-page BottomSheet/mobile menu navigation with a shared SidebarNavigation component used by docs and company handbook routes. This removes redundant navigation implementations, avoids the bottom sheet UI, and centralizes scrolling/active-link logic into apps/web/src/components/sidebar-navigation.tsx so both routes can reuse it and simplify their route files.
Add support for a handbook drawer alongside the existing docs drawer so the left drawer icon and header controls appear on /company-handbook pages. Introduces a HandbookDrawerContext and useHandbookDrawer hook, wires handbook drawer state through the view route, and updates Header/LeftNav to render a shared DrawerButton that toggles either the docs or handbook drawer depending on the current route. Also add a MobileHandbookDrawer implementation and ensure mobile menu closing logic hides the other drawer when opening the menu.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/_view/route.tsx (1)
3-3: Remove unusedLinkimport to fix build failure.The pipeline is failing with
TS6133: 'Link' is declared but its value is never read.import { createFileRoute, - Link, Outlet, useMatchRoute, useRouterState, } from "@tanstack/react-router";
♻️ Duplicate comments (2)
apps/web/src/routes/_view/route.tsx (1)
99-100: Add memoization togetDocsBySection()call.This call lacks memoization, causing unnecessary recomputation on every render. The
MobileHandbookDrawerbelow usesuseMemofor the same pattern—apply it consistently here.- const { sections } = getDocsBySection(); + const { sections } = useMemo(() => getDocsBySection(), []);apps/web/src/components/header.tsx (1)
589-605: Still missing menu-close handler on mobile “Get started” CTAInside
MobileMenuCTAs, the “Get started” link (/join-waitlist) does not callsetIsMenuOpen(false), unlike the other CTAs below it. This means the mobile menu will remain open after tapping “Get started”, which is inconsistent with the rest of the overlay.Suggested fix:
<Link to="/join-waitlist" + onClick={() => setIsMenuOpen(false)} className="block w-full px-4 py-3 text-center text-sm text-neutral-700 border border-neutral-200 rounded-lg hover:bg-neutral-50 transition-colors" > Get started </Link>
🧹 Nitpick comments (4)
apps/web/src/routes/_view/docs/route.tsx (1)
33-34: Add memoization to prevent unnecessary recomputation.
getDocsBySection()is called directly in the component body. Since this processes staticallDocsdata, the result is deterministic and recomputing on every render is wasteful. TheMobileHandbookDrawerin this same PR usesuseMemofor similar logic—apply the same pattern here for consistency.-import { useRef } from "react"; +import { useMemo, useRef } from "react";- const { sections } = getDocsBySection(); + const { sections } = useMemo(() => getDocsBySection(), []);apps/web/src/routes/_view/company-handbook/route.tsx (1)
34-70: Consider extracting agetHandbooksBySectionhelper for consistency.This section grouping logic duplicates the pattern in
getDocsBySection(). Extracting it to a helper function in-structure.tswould improve consistency and reduce duplication.Example in
apps/web/src/routes/_view/company-handbook/-structure.ts:export function getHandbooksBySection() { const sectionGroups: Record<string, { title: string; docs: (typeof allHandbooks)[0][] }> = {}; // ... same grouping logic return { sections }; }apps/web/src/routes/_view/route.tsx (1)
135-171: Duplicated section grouping logic—consider extracting a helper.This handbook section grouping logic is duplicated in
company-handbook/route.tsx. Consider extracting to agetHandbooksBySection()helper in-structure.tsfor DRY compliance.apps/web/src/components/header.tsx (1)
211-290: Add ARIA state to Product dropdown for better accessibilityThe desktop
ProductDropdownand itsProductsList/FeaturesListlook good structurally, but the trigger button currently lacks ARIA state. Consider:
- Adding
aria-haspopup="menu"andaria-expanded={isProductOpen}to theProductbutton.- Optionally using list semantics (
<ul>/<li>) orrole="menu"/role="menuitem"on the dropdown content.This will make the hover/toggle behavior more discoverable to assistive tech without changing visuals.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/src/components/header.tsx(3 hunks)apps/web/src/components/sidebar-navigation.tsx(1 hunks)apps/web/src/hooks/use-handbook-drawer.ts(1 hunks)apps/web/src/routes/_view/company-handbook/route.tsx(2 hunks)apps/web/src/routes/_view/docs/route.tsx(2 hunks)apps/web/src/routes/_view/route.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/components/sidebar-navigation.tsxapps/web/src/routes/_view/docs/route.tsxapps/web/src/routes/_view/route.tsxapps/web/src/routes/_view/company-handbook/route.tsxapps/web/src/components/header.tsxapps/web/src/hooks/use-handbook-drawer.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/src/hooks/use-handbook-drawer.ts
🧬 Code graph analysis (3)
apps/web/src/routes/_view/docs/route.tsx (2)
apps/web/src/routes/_view/docs/-structure.ts (1)
getDocsBySection(13-52)apps/web/src/components/sidebar-navigation.tsx (1)
SidebarNavigation(4-71)
apps/web/src/routes/_view/company-handbook/route.tsx (1)
apps/web/src/components/sidebar-navigation.tsx (1)
SidebarNavigation(4-71)
apps/web/src/components/header.tsx (3)
apps/web/src/hooks/use-docs-drawer.ts (1)
useDocsDrawer(12-14)apps/web/src/hooks/use-handbook-drawer.ts (1)
useHandbookDrawer(11-13)apps/web/src/hooks/use-platform.ts (1)
getPlatformCTA(35-43)
🪛 GitHub Actions: web_ci
apps/web/src/routes/_view/route.tsx
[error] 3-3: TypeScript error TS6133: 'Link' is declared but its value is never read. (Occurred during: tsc --project tsconfig.json --noEmit)
⏰ 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: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (8)
apps/web/src/routes/_view/docs/route.tsx (1)
36-50: LGTM!The sidebar structure with scroll container ref, sticky positioning, and SidebarNavigation integration is well implemented.
apps/web/src/routes/_view/company-handbook/route.tsx (1)
74-88: LGTM!The sidebar structure with scroll container ref and SidebarNavigation integration follows the established pattern correctly.
apps/web/src/components/sidebar-navigation.tsx (1)
1-71: LGTM!Well-structured reusable component with:
- Appropriate generic constraint for flexibility
- Inlined props (per coding guidelines)
- Proper scroll-to-active behavior using
requestAnimationFrame- Clean separation of concerns
apps/web/src/hooks/use-handbook-drawer.ts (1)
1-13: LGTM!Clean context/hook pattern consistent with the existing
DocsDrawerContextapproach. The interface is appropriately inlined since it's not shared.apps/web/src/routes/_view/route.tsx (1)
54-79: LGTM!The HandbookDrawerContext.Provider integration and conditional rendering of both docs and handbook drawers is well structured.
apps/web/src/components/header.tsx (3)
45-82: Header decomposition and docs/handbook drawer wiring look solidThe refactor of
HeaderintoLeftNav/DesktopNav/MobileNavplus the newisHandbookPage+handbookDrawerwiring mirrors the existing docs drawer behavior cleanly. The guards inDrawerButton(isDocsPage && docsDrawer,isHandbookPage && handbookDrawer) and thesetIsMenuOpen(false)calls when opening a drawer should prevent overlapping mobile overlays without introducing new state bugs. No changes needed here.Also applies to: 100-198
342-380: Mobile menu toggle correctly coordinates with docs/handbook drawersThe
MobileNavmenu button logic—closingdocsDrawer/handbookDrawerwhen opening the main mobile menu and then togglingisMenuOpen—avoids conflicting overlays on small screens. The ARIA label/expanded attributes are also wired correctly. Looks good as-is.
410-587: Mobile menu overlay and link-close behavior are consistent
MobileMenuand its subcomponents (MobileMenuLinks,MobileProductSection,MobileProductsList,MobileFeaturesList) handle open/close state cleanly: background click closes the menu, and all mobile nav/product/feature links inside the overlay callsetIsMenuOpen(false)on navigation. This matches expected mobile behavior and should feel consistent.
Remove unused imports from route.tsx (Link and allDocs) to fix TypeScript "declared but its value is never read" errors. Also remove the invalid "/faq" entry from sitemap.ts which caused a TypeScript type error due to an unknown route key. These changes were needed to clean up unused code and align the sitemap with the defined route types so the project compiles without TS6133 and TS2353 errors.
Make the drawer background in the docs and company-handbook view routes match the header's translucent blurred style and add left safe-area padding. This ensures a consistent visual appearance across views and prevents content from being obscured on devices with display insets. - Change bg-white to bg-white/80 and add backdrop-blur-sm for both routes. - Add inline style paddingLeft: "env(safe-area-inset-left)" to respect device safe areas.
Add the "scrollbar-hide" utility class to the main scroll containers in the view route so scrollbars are visually hidden while preserving scroll behavior. This change improves the UI by removing visible scrollbars in content areas for a cleaner look. Files changed: - apps/web/src/routes/_view/route.tsx: added "scrollbar-hide" to two divs that use overflow-y-auto.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/routes/_view/route.tsx (1)
98-98: Apply memoization to prevent unnecessary recomputation.The call to
getDocsBySection()lacks memoization despite:
- A previous review comment indicating this should be addressed
useMemobeing imported on line 8- The identical logic in
MobileHandbookDrawer(line 140) being properly memoizedSince
getDocsBySection()processes arrays (grouping, sorting, filtering), this recomputation on every render is wasteful.Apply this diff:
- const { sections } = getDocsBySection(); + const { sections } = useMemo(() => getDocsBySection(), []);
🧹 Nitpick comments (2)
apps/web/src/routes/_view/route.tsx (2)
140-176: Extract duplicated section-grouping logic to a shared utility.The section-grouping logic here is nearly identical to
getDocsBySection()fromdocs/-structure.ts:
- Both group documents by section
- Both sort by order
- Both filter based on structure
This duplication increases maintenance burden.
Consider extracting to a shared utility like:
function groupDocsBySection<T extends { section: string; slug: string; isIndex?: boolean; order: number }>( docs: T[], structure: { sections: string[] }, matchFn: (sectionId: string, groupTitle: string) => boolean ) { const sectionGroups: Record<string, { title: string; docs: T[] }> = {}; docs.forEach((doc) => { if (doc.slug === "index" || doc.isIndex) return; const sectionName = doc.section; if (!sectionGroups[sectionName]) { sectionGroups[sectionName] = { title: sectionName, docs: [] }; } sectionGroups[sectionName].docs.push(doc); }); Object.values(sectionGroups).forEach((group) => { group.docs.sort((a, b) => a.order - b.order); }); const sections = structure.sections .map((sectionId) => Object.values(sectionGroups).find((group) => matchFn(sectionId, group.title)) ) .filter(Boolean); return { sections }; }Then both drawers can call this utility with their specific matching logic.
102-109: Consider extracting shared drawer container styling.Both
MobileDocsDrawerandMobileHandbookDraweruse identical container styling (top positioning, dimensions, backdrop blur, transitions, safe area insets).Extract to a shared constant or component to reduce duplication:
const MOBILE_DRAWER_CLASSES = "fixed top-[69px] left-0 h-[calc(100vh-69px)] w-72 bg-white/80 backdrop-blur-sm border-r border-neutral-100 shadow-2xl shadow-neutral-900/20 z-50 md:hidden transition-transform duration-300 ease-in-out"; const MOBILE_DRAWER_STYLE = { paddingLeft: "env(safe-area-inset-left)", };Also applies to: 182-187
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/routes/_view/route.tsx(6 hunks)apps/web/src/utils/sitemap.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/utils/sitemap.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/route.tsx
🧬 Code graph analysis (1)
apps/web/src/routes/_view/route.tsx (3)
apps/web/src/hooks/use-handbook-drawer.ts (1)
HandbookDrawerContext(8-9)apps/web/src/routes/_view/docs/-structure.ts (1)
getDocsBySection(13-52)apps/web/src/routes/_view/company-handbook/-structure.ts (1)
handbookStructure(1-16)
⏰ 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). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote
- GitHub Check: fmt
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci
🔇 Additional comments (2)
apps/web/src/routes/_view/route.tsx (2)
37-41: LGTM! Handbook drawer integration follows existing patterns.The handbook page detection and drawer state management mirror the existing docs drawer implementation correctly. The context provider nesting and conditional rendering are properly structured.
Also applies to: 53-78
169-171: The section name matching logic at lines 169–171 works correctly. The capitalization approach matches the actualdoc.sectionvalues in the handbook files:
handbookStructure.sectionscontains lowercase hyphenated IDs:"about","how-we-work","who-we-want","go-to-market","communication"- Applying
charAt(0).toUpperCase() + slice(1)produces:"About","How-we-work","Who-we-want","Go-to-market","Communication"- These match exactly with the
sectionvalues defined in handbook frontmatterNo mismatch occurs; sections are properly found and displayed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/header.tsx (1)
23-27: Simplify function - both branches return identical value.The conditional logic is redundant since both branches return
"max-w-6xl". Either simplify to a constant or remove the unused parameter.-function getMaxWidthClass(pathname: string): string { - const isBlogOrDocs = - pathname.startsWith("/blog") || pathname.startsWith("/docs"); - return isBlogOrDocs ? "max-w-6xl" : "max-w-6xl"; -} +function getMaxWidthClass(_pathname: string): string { + return "max-w-6xl"; +}Or if different widths are planned:
function getMaxWidthClass(pathname: string): string { const isBlogOrDocs = pathname.startsWith("/blog") || pathname.startsWith("/docs"); - return isBlogOrDocs ? "max-w-6xl" : "max-w-6xl"; + return isBlogOrDocs ? "max-w-6xl" : "max-w-7xl"; // adjust as needed }
🧹 Nitpick comments (3)
apps/web/src/components/header.tsx (3)
136-198: Consolidate duplicated drawer button logic.The docs and handbook drawer blocks are nearly identical. Consider extracting shared logic to reduce duplication.
function DrawerButton({ isDocsPage, isHandbookPage, docsDrawer, handbookDrawer, setIsMenuOpen, }: { isDocsPage: boolean; isHandbookPage: boolean; docsDrawer: ReturnType<typeof useDocsDrawer>; handbookDrawer: ReturnType<typeof useHandbookDrawer>; setIsMenuOpen: (open: boolean) => void; }) { + const drawer = isDocsPage ? docsDrawer : isHandbookPage ? handbookDrawer : null; + const label = isDocsPage ? "docs" : "handbook"; + - if (isDocsPage && docsDrawer) { + if (drawer) { return ( <button onClick={() => { - if (!docsDrawer.isOpen) { + if (!drawer.isOpen) { setIsMenuOpen(false); } - docsDrawer.setIsOpen(!docsDrawer.isOpen); + drawer.setIsOpen(!drawer.isOpen); }} className="cursor-pointer md:hidden px-3 h-8 flex items-center text-sm bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 rounded-full shadow-sm hover:shadow-md hover:scale-[102%] active:scale-[98%] transition-all" aria-label={ - docsDrawer.isOpen ? "Close docs navigation" : "Open docs navigation" + drawer.isOpen ? `Close ${label} navigation` : `Open ${label} navigation` } > - {docsDrawer.isOpen ? ( + {drawer.isOpen ? ( <PanelLeftClose className="text-neutral-600" size={16} /> ) : ( <PanelLeft className="text-neutral-600" size={16} /> )} </button> ); } - - if (isHandbookPage && handbookDrawer) { - return ( - <button - onClick={() => { - if (!handbookDrawer.isOpen) { - setIsMenuOpen(false); - } - handbookDrawer.setIsOpen(!handbookDrawer.isOpen); - }} - className="cursor-pointer md:hidden px-3 h-8 flex items-center text-sm bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 rounded-full shadow-sm hover:shadow-md hover:scale-[102%] active:scale-[98%] transition-all" - aria-label={ - handbookDrawer.isOpen - ? "Close handbook navigation" - : "Open handbook navigation" - } - > - {handbookDrawer.isOpen ? ( - <PanelLeftClose className="text-neutral-600" size={16} /> - ) : ( - <PanelLeft className="text-neutral-600" size={16} /> - )} - </button> - ); - } return null; }
242-290: Abstract duplicated list components.
ProductsListandFeaturesListshare identical rendering logic. Consider creating a single reusable component.+function LinkList({ + title, + items, + onClose +}: { + title: string; + items: typeof productsList; + onClose: () => void; +}) { + return ( + <div> + <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> + {title} + </div> + {items.map((link) => ( + <Link + key={link.to} + to={link.to} + onClick={onClose} + className="py-2 text-sm text-neutral-700 flex items-center justify-between hover:underline decoration-dotted" + > + <span>{link.label}</span> + {link.badge && ( + <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> + {link.badge} + </span> + )} + </Link> + ))} + </div> + ); +} + -function ProductsList({ onClose }: { onClose: () => void }) { - return ( - <div> - <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> - Products - </div> - {productsList.map((link) => ( - <Link - key={link.to} - to={link.to} - onClick={onClose} - className="py-2 text-sm text-neutral-700 flex items-center justify-between hover:underline decoration-dotted" - > - <span>{link.label}</span> - {link.badge && ( - <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> - {link.badge} - </span> - )} - </Link> - ))} - </div> - ); -} +function ProductsList({ onClose }: { onClose: () => void }) { + return <LinkList title="Products" items={productsList} onClose={onClose} />; +} -function FeaturesList({ onClose }: { onClose: () => void }) { - return ( - <div> - <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> - Features - </div> - {featuresList.map((link) => ( - <Link - key={link.to} - to={link.to} - onClick={onClose} - className="py-2 text-sm text-neutral-700 flex items-center justify-between hover:underline decoration-dotted" - > - <span>{link.label}</span> - {link.badge && ( - <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> - {link.badge} - </span> - )} - </Link> - ))} - </div> - ); -} +function FeaturesList({ onClose }: { onClose: () => void }) { + return <LinkList title="Features" items={featuresList} onClose={onClose} />; +}
531-587: Abstract mobile list duplication (similar to desktop lists).
MobileProductsListandMobileFeaturesListduplicate the same rendering logic. Like the desktop versions, these can share a common abstraction.+function MobileLinkList({ + title, + items, + setIsMenuOpen +}: { + title: string; + items: typeof productsList; + setIsMenuOpen: (open: boolean) => void; +}) { + return ( + <div> + <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> + {title} + </div> + {items.map((link) => ( + <Link + key={link.to} + to={link.to} + onClick={() => setIsMenuOpen(false)} + className="text-sm text-neutral-600 hover:text-neutral-900 transition-colors flex items-center justify-between py-1" + > + <span>{link.label}</span> + {link.badge && ( + <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> + {link.badge} + </span> + )} + </Link> + ))} + </div> + ); +} + -function MobileProductsList({ - setIsMenuOpen, -}: { - setIsMenuOpen: (open: boolean) => void; -}) { - return ( - <div> - <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> - Products - </div> - {productsList.map((link) => ( - <Link - key={link.to} - to={link.to} - onClick={() => setIsMenuOpen(false)} - className="text-sm text-neutral-600 hover:text-neutral-900 transition-colors flex items-center justify-between py-1" - > - <span>{link.label}</span> - {link.badge && ( - <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> - {link.badge} - </span> - )} - </Link> - ))} - </div> - ); -} +function MobileProductsList({ setIsMenuOpen }: { setIsMenuOpen: (open: boolean) => void }) { + return <MobileLinkList title="Products" items={productsList} setIsMenuOpen={setIsMenuOpen} />; +} -function MobileFeaturesList({ - setIsMenuOpen, -}: { - setIsMenuOpen: (open: boolean) => void; -}) { - return ( - <div> - <div className="text-xs font-semibold text-neutral-400 uppercase tracking-wider mb-2"> - Features - </div> - {featuresList.map((link) => ( - <Link - key={link.to} - to={link.to} - onClick={() => setIsMenuOpen(false)} - className="text-sm text-neutral-600 hover:text-neutral-900 transition-colors flex items-center justify-between py-1" - > - <span>{link.label}</span> - {link.badge && ( - <span className="text-[10px] bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 px-2 py-0.5 rounded-full"> - {link.badge} - </span> - )} - </Link> - ))} - </div> - ); -} +function MobileFeaturesList({ setIsMenuOpen }: { setIsMenuOpen: (open: boolean) => void }) { + return <MobileLinkList title="Features" items={featuresList} setIsMenuOpen={setIsMenuOpen} />; +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/components/header.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/components/header.tsx
⏰ 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). (8)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci
- GitHub Check: fmt
🔇 Additional comments (4)
apps/web/src/components/header.tsx (4)
45-98: LGTM - Clean component composition.The refactor successfully modularizes the header into focused subcomponents with clear separation between desktop and mobile navigation.
100-631: Well-structured component refactor.The modular breakdown into focused components improves maintainability and follows React composition patterns effectively. Props are appropriately inlined per coding guidelines, and the drawer coordination logic correctly prevents overlapping menus.
395-401: Verify hardcoded download URL.The download link uses a hardcoded path
/download/apple-siliconbutusePlatform()andgetPlatformCTA()suggest platform detection exists. Ensure this URL is correct for all platforms or should be made dynamic. Same issue exists at line 609.
158-158: Verify custom gradient class name or fix to standard Tailwind syntax.The className uses
bg-linear-to-twhich is not standard Tailwind CSS syntax. The correct standard gradient syntax isbg-gradient-to-t. Either confirm this is a custom utility class defined in your Tailwind config, or update it to use the standard gradient class. This pattern appears throughout the file (lines 158, 181, 257, 282, 372, 392, 550, 580, 612, 624).
1 participant
Overview