Conversation
🦋 Changeset detectedLatest commit: a90ad8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds a new exported Header React component in Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Client)
participant HomeLayout as HomeLayout (apps/www)
participant Header as Header (fumadocs-ui)
participant Search as SearchContext
Browser->>HomeLayout: render layout
HomeLayout->>Header: instantiate <Header logo, links, actions>
Header->>Header: mount → attach scroll listener (toggle scrolled state)
Browser->>Header: user scrolls
Header->>Header: update scrolled state → apply backdrop blur
Browser->>Header: click SearchToggle
Header->>Search: useSearchContext.open() (open search panel)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
openspec/changes/eject-the-header/specs/header/spec.md (2)
25-27: Consider more implementation-agnostic language for the scroll detection scenario.The scenario references
scrollY === 0, which is an implementation detail. Spec requirements typically focus on observable user-facing behavior rather than specific API calls or variable checks.♻️ More behavior-focused phrasing
#### Scenario: Transparent at top -- **WHEN** the page scroll position is at the top (scrollY === 0) +- **WHEN** the page has not been scrolled from its initial position - **THEN** the header background MUST be transparent (no visible background fill)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/eject-the-header/specs/header/spec.md` around lines 25 - 27, Update the "Scenario: Transparent at top" requirement to remove the implementation-specific `scrollY === 0` check and describe the observable behavior instead; for example, change the WHEN line to something like "WHEN the page is scrolled to the very top (no vertical scroll offset)" or "WHEN the viewport is at the top of the document" so the header background requirement in the THEN line remains behavior-focused and implementation-agnostic (refer to the "Transparent at top" scenario and replace the `scrollY === 0` text).
36-38: Avoid specifying implementation details in requirements.Line 38 mentions "position: fixed (or equivalent Tailwind class)", which prescribes implementation approach. Specs should describe observable behavior and leave implementation details to the code.
♻️ More implementation-agnostic phrasing
#### Scenario: Fixed layout - **WHEN** `Header` is rendered -- **THEN** it MUST have `position: fixed` (or equivalent Tailwind class) so it stays visible as the user scrolls +- **THEN** it MUST remain visible at the top of the viewport as the user scrolls the page🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/eject-the-header/specs/header/spec.md` around lines 36 - 38, The requirement for the Header spec currently mandates an implementation detail ("position: fixed (or equivalent Tailwind class)"); change it to describe observable behavior only by removing the CSS/Tailwind prescription and stating that the Header component (Header) must remain visible in the viewport as the user scrolls (e.g., "THEN it MUST remain visible at the top of the viewport while the user scrolls"), so tests assert the visible/sticky behavior rather than a specific implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/eject-the-header/design.md`:
- Around line 52-53: The mitigation for mobile nav is vague and must be
concretized before implementing Header (task 1.1): choose either Option A or B
and update design/tasks accordingly—Option A: if the nav.children slot exists,
document exactly how to wire fumadocs-ui's mobile menu into Header (mention
using nav.children slot, expected props/events, and fallback behavior); Option
B: if building a custom mobile menu, add explicit implementation tasks to the
Header component for a hamburger button, accessible mobile drawer/menu,
responsive CSS breakpoints, and a clear fallback for when nav.children is
absent; update the Header task description to reference nav.children and Header
component so implementers know which path to follow.
In `@openspec/changes/eject-the-header/proposal.md`:
- Line 25: Update the phrasing that currently reads
"`apps/www/app/layout.config.tsx` — simplified or replaced" to explicitly state
that `layout.config.tsx` will be kept and simplified (e.g.,
"`apps/www/app/layout.config.tsx` — kept and simplified`"), ensuring consistency
with the design note "Keep `layout.config.tsx` but simplify it." Locate the
mention of `layout.config.tsx` in the proposal text and replace the ambiguous
"or replaced" wording with clear language indicating it will be simplified and
retained.
In `@openspec/changes/eject-the-header/specs/fumadocs-ui/spec.md`:
- Line 1: The top-level header currently reads "## MODIFIED Requirements" but
the "Header Export" addition (and possibly "ArkEnv MDX Components") introduces
new orthogonal capabilities, so change the section header to "## ADDED
Requirements" or split the spec into two sections — one "## ADDED Requirements"
containing the new "Header Export" requirement and another "## MODIFIED
Requirements" only if something is truly changing existing behavior (reference
the "Header Export" and "ArkEnv MDX Components" requirement texts when
moving/splitting them). Ensure each new capability is under an ADDED header and
any genuine behavior changes remain under MODIFIED.
- Around line 3-8: The spec currently documents the existing export
arkenvComponents without a delta header; either remove this Requirement block
from spec.md if it merely describes pre-existing behavior, or mark it as a
change by adding the appropriate delta header (e.g., prefix the block with "##
ADDED") and confirm there is no prior implementation; update the spec to either
delete the Requirement for "ArkEnv MDX Components" or change its heading to "##
ADDED: ArkEnv MDX Components" and keep the bullet points as-is to indicate this
PR introduces the export arkenvComponents (ensure arkenvComponents is the
referenced symbol).
In `@openspec/changes/eject-the-header/tasks.md`:
- Line 17: The nav/logo/links in layout.config.tsx (baseOptions,
baseOptions.nav.title, baseOptions.links) are still consumed by
apps/www/app/(home)/layout.tsx and apps/www/app/docs/layout.tsx, so removing
them in task 3.3 will drop the logo and GitHub link; update tasks 3.1 and 3.2 to
explicitly pass those props into the Header component (e.g., instantiate Header
with logo, links, and actions props sourced from baseOptions or local overrides
in the Home and Docs layouts), then in task 3.3 remove the duplicated nav
options from layout.config.tsx and add a verification step that Header renders
the logo and GitHub link in both layouts.
---
Nitpick comments:
In `@openspec/changes/eject-the-header/specs/header/spec.md`:
- Around line 25-27: Update the "Scenario: Transparent at top" requirement to
remove the implementation-specific `scrollY === 0` check and describe the
observable behavior instead; for example, change the WHEN line to something like
"WHEN the page is scrolled to the very top (no vertical scroll offset)" or "WHEN
the viewport is at the top of the document" so the header background requirement
in the THEN line remains behavior-focused and implementation-agnostic (refer to
the "Transparent at top" scenario and replace the `scrollY === 0` text).
- Around line 36-38: The requirement for the Header spec currently mandates an
implementation detail ("position: fixed (or equivalent Tailwind class)"); change
it to describe observable behavior only by removing the CSS/Tailwind
prescription and stating that the Header component (Header) must remain visible
in the viewport as the user scrolls (e.g., "THEN it MUST remain visible at the
top of the viewport while the user scrolls"), so tests assert the visible/sticky
behavior rather than a specific implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c188a06-89a5-42b1-be1d-a581e9f9ecad
📒 Files selected for processing (6)
openspec/changes/eject-the-header/.openspec.yamlopenspec/changes/eject-the-header/design.mdopenspec/changes/eject-the-header/proposal.mdopenspec/changes/eject-the-header/specs/fumadocs-ui/spec.mdopenspec/changes/eject-the-header/specs/header/spec.mdopenspec/changes/eject-the-header/tasks.md
…grate across layouts
commit: |
📦 Bundle Size Report✅ All size limits passed! |
…in Header component; update dependencies for "next" and related packages
… width customization; update imports
…/hover color handling
… structure in Header component; add ThemeToggle action to home layout
…ble; extend transition properties; adjust light theme background color
… structure, and disable theme switch in docs layout
…nditional rendering
…handle transient undefined states
… update desktop logic, and include GitHub link in layouts
…oved mobile navigation
…ith `Menu` and `Menu` with `PanelLeft` for better clarity
… toggle, improve sidebar drawer positioning, and refine layout spacing
…, update layout logic, and refine icon sizes
…y and structure in mobile menu
…t-in close-button row for a cleaner layout
…stom styles for improved maintainability
…ismisses the drawer and poppers remain visible
…rove visual consistency
…bove header and mobile sidebar, improve mobile responsiveness
…d-foreground` for improved text visibility
… only on `lg` screens
…theme toggle in sidebar
…r theme toggle alignment
…pdate header for improved mobile behavior
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/fumadocs-ui/src/components/header.tsx (1)
10-32: Add JSDoc to the exported header API.
Header,HeaderLink, andHeaderPropsare new package-level exports. A top-level JSDoc block with a short usage example will make the public surface much easier to consume from editor hovers and generated docs.As per coding guidelines, "Use JSDoc comments for public APIs" and "Include examples in JSDoc comments when helpful for public APIs."
Also applies to: 34-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fumadocs-ui/src/components/header.tsx` around lines 10 - 32, Add a top-level JSDoc block documenting the public header API: include a brief description and a short usage example that demonstrates importing and rendering the Header component, and document the exported types Header, HeaderLink, and HeaderProps (noting fields like activeMatch and menuActions). Place the JSDoc immediately above the exported symbols (Header, HeaderLink, HeaderProps) so editor hovers and generated docs show the description and example; ensure each public prop field retains its existing inline comments and that the example uses the common props (logo/logoHref/links/actions/sidebarTrigger) to illustrate typical usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/components/ui/theme-toggle.tsx`:
- Around line 20-34: The module-scoped cache assignment "if (theme !==
undefined) cachedTheme = theme" is mutating shared state during render in
ThemeToggle and must be moved into a post-render effect; update the component to
remove that assignment from the render path and instead set cachedTheme inside a
useEffect that runs when "theme" changes (e.g., useEffect(() => { if (theme !==
undefined) cachedTheme = theme }, [theme])), keeping the existing
hydrated/setMounted initialization logic intact and referencing the same
"theme", "cachedTheme", "ThemeToggle", "useEffect", and "useState" symbols.
In `@packages/fumadocs-ui/css/theme.css`:
- Around line 249-254: Add a blank line before the offending declarations to
satisfy the declaration-empty-line-before rule: in the block with selector group
"#nd-sidebar a, `#nd-sidebar` button" ensure there is an empty line before the
"transition: color 150ms;" declaration (and apply the same blank-line fix to the
other blocks flagged around lines 285–293). Locate the declaration inside the
functions/classes named by those selectors and insert a single empty line
immediately above each flagged declaration so the stylesheet passes Stylelint.
In `@packages/fumadocs-ui/src/components/header.tsx`:
- Around line 21-23: The HeaderProps type marks logo as optional but the Header
component always renders a focusable link around it (creating an empty,
unlabeled tab stop when logo is omitted); either make logo required in
HeaderProps or, preferably, guard the link rendering in the Header component
(check the logo prop before returning the <a>/... link block) and skip rendering
the link when logo is undefined (or provide an accessible fallback such as an
aria-label). Update references to HeaderProps/logo and the Header component's
logo-link rendering to ensure no empty focusable element is produced.
- Around line 90-94: The current active-state check inside links.map (the
isInternal/isActive logic) uses pathname.startsWith(link.activeMatch ??
link.url) which yields false positives for sibling paths; update isActive to
match full path segments instead: compute a canonicalMatch = link.activeMatch ??
link.url and treat link as active if pathname === canonicalMatch or
pathname.startsWith(canonicalMatch + "/") (or if canonicalMatch === "/" handle
root specially). Apply this change in both occurrences of the mapping logic (the
desktop/mobile menu blocks where links.map and isActive are computed) so the nav
only activates when the path equals the route or is a subpath under that exact
segment.
---
Nitpick comments:
In `@packages/fumadocs-ui/src/components/header.tsx`:
- Around line 10-32: Add a top-level JSDoc block documenting the public header
API: include a brief description and a short usage example that demonstrates
importing and rendering the Header component, and document the exported types
Header, HeaderLink, and HeaderProps (noting fields like activeMatch and
menuActions). Place the JSDoc immediately above the exported symbols (Header,
HeaderLink, HeaderProps) so editor hovers and generated docs show the
description and example; ensure each public prop field retains its existing
inline comments and that the example uses the common props
(logo/logoHref/links/actions/sidebarTrigger) to illustrate typical usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a997b8c-0b72-49ae-ab12-a6f750ecd6f6
📒 Files selected for processing (11)
.changeset/heavy-fans-fold.md.idea/compiler.xmlapps/www/app/(home)/layout.tsxapps/www/app/docs/layout.tsxapps/www/app/globals.cssapps/www/app/styles/components/fd.cssapps/www/components/docs/sidebar-trigger.tsxapps/www/components/ui/search-toggle.tsxapps/www/components/ui/theme-toggle.tsxpackages/fumadocs-ui/css/theme.csspackages/fumadocs-ui/src/components/header.tsx
💤 Files with no reviewable changes (2)
- apps/www/app/styles/components/fd.css
- apps/www/app/globals.css
✅ Files skipped from review due to trivial changes (1)
- .idea/compiler.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/www/app/(home)/layout.tsx
- apps/www/components/ui/search-toggle.tsx
| /* Sidebar item base styles */ | ||
| #nd-sidebar a, | ||
| #nd-sidebar button { | ||
| @apply font-medium; | ||
| transition: color 150ms; | ||
| } |
There was a problem hiding this comment.
Fix the two new Stylelint failures.
Lines 253 and 292 trip declaration-empty-line-before, so this stylesheet will fail the current lint config.
🧹 Minimal lint-only fix
`#nd-sidebar` a,
`#nd-sidebar` button {
`@apply` font-medium;
+
transition: color 150ms;
}
@@
`#nd-sidebar-mobile` {
inset-inline-end: auto;
inset-inline-start: 0;
border-inline-start: none;
border-inline-end: 1px solid var(--color-fd-border);
--fd-sidebar-drawer-offset: -100%;
+
z-index: 60;
}Also applies to: 285-293
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 253-253: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fumadocs-ui/css/theme.css` around lines 249 - 254, Add a blank line
before the offending declarations to satisfy the declaration-empty-line-before
rule: in the block with selector group "#nd-sidebar a, `#nd-sidebar` button"
ensure there is an empty line before the "transition: color 150ms;" declaration
(and apply the same blank-line fix to the other blocks flagged around lines
285–293). Locate the declaration inside the functions/classes named by those
selectors and insert a single empty line immediately above each flagged
declaration so the stylesheet passes Stylelint.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/fumadocs-ui/src/components/header.tsx (2)
155-158: Minor style inconsistency with desktop nav.The mobile menu uses string concatenation while the desktop nav (line 97) uses a template literal:
// Desktop (line 97) pathname.startsWith(`${canonicalMatch}/`) // Mobile (line 158) pathname.startsWith(canonicalMatch + "/")Both are functionally equivalent; consider unifying for consistency.
🔧 Optional: Unify to template literal
const isActive = isInternal ? pathname === canonicalMatch || - pathname.startsWith(canonicalMatch + "/") + pathname.startsWith(`${canonicalMatch}/`) : false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fumadocs-ui/src/components/header.tsx` around lines 155 - 158, The mobile nav uses string concatenation when computing active state—update the isActive logic that references canonicalMatch and pathname.startsWith(canonicalMatch + "/") to use a template literal (e.g., pathname.startsWith(`${canonicalMatch}/`) ) so it matches the desktop nav style; change the expression in the component where canonicalMatch and isActive are computed to use the template literal form.
10-32: Consider adding JSDoc to the exportedHeadercomponent.The props are well-documented, but adding a brief JSDoc to the
Headerfunction itself would improve discoverability for consumers of the public API.📝 Optional: Add JSDoc to Header
+/** + * A responsive fixed header component with navigation links, + * optional actions, and a mobile dropdown menu. + */ export function Header({ logo, logoHref = "/",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fumadocs-ui/src/components/header.tsx` around lines 10 - 32, Add a concise JSDoc block immediately above the exported Header React component declaration (the Header function/component) describing its purpose (site header/navigation), summarizing key props and behavior, and referencing the HeaderProps and HeaderLink types (mentioning logo, links, actions, menuActions, menuSocialActions, and sidebarTrigger) so consumers see usage and links in IDE tooltips; keep it brief (one-line summary plus param/returns tag or `@param` reference to HeaderProps) and ensure it is exported with the component for public API discoverability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/fumadocs-ui/src/components/header.tsx`:
- Around line 155-158: The mobile nav uses string concatenation when computing
active state—update the isActive logic that references canonicalMatch and
pathname.startsWith(canonicalMatch + "/") to use a template literal (e.g.,
pathname.startsWith(`${canonicalMatch}/`) ) so it matches the desktop nav style;
change the expression in the component where canonicalMatch and isActive are
computed to use the template literal form.
- Around line 10-32: Add a concise JSDoc block immediately above the exported
Header React component declaration (the Header function/component) describing
its purpose (site header/navigation), summarizing key props and behavior, and
referencing the HeaderProps and HeaderLink types (mentioning logo, links,
actions, menuActions, menuSocialActions, and sidebarTrigger) so consumers see
usage and links in IDE tooltips; keep it brief (one-line summary plus
param/returns tag or `@param` reference to HeaderProps) and ensure it is exported
with the component for public API discoverability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b7e19ca-2a13-4045-8a4e-ba8991b7f357
📒 Files selected for processing (4)
.changeset/heavy-fans-fold.md.changeset/little-phones-eat.mdapps/www/components/ui/theme-toggle.tsxpackages/fumadocs-ui/src/components/header.tsx
✅ Files skipped from review due to trivial changes (1)
- .changeset/little-phones-eat.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/heavy-fans-fold.md
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @arkenv/fumadocs-ui@0.0.4 ### Patch Changes - #### Add `Header` component _[`#828`](#828) [`e1f3183`](e1f3183) [@yamcodes](https://github.com/yamcodes)_ `@arkenv/fumadocs-ui` now exports a `Header` component for building site-wide navigation headers. ```tsx import { Header } from "@arkenv/fumadocs-ui/components"; <Header logo={<MyLogo />} links={[ { text: "Docs", url: "/docs" }, { text: "Blog", url: "/blog" }, ]} actions={[<SearchToggle />, <ThemeToggle />]} menuActions={[<ThemeToggle />]} menuSocialActions={[<GitHubLink />]} sidebarTrigger={<MySidebarTrigger />} />; ``` The header is fixed to the top of the viewport and adapts its appearance as the user scrolls — transparent when at the top of the page, blurred with a semi-transparent background once the user scrolls down. On mobile the header renders a full-screen dropdown menu. Nav links are stacked at the top, an "Appearance" row (label + `menuActions`) sits above a centered row of `menuSocialActions`. An optional `sidebarTrigger` slot renders left of the logo for layouts that have a docs sidebar. - #### Expand `css/theme.css` _[`#828`](#828) [`e1f3183`](e1f3183) [@yamcodes](https://github.com/yamcodes)_ `@arkenv/fumadocs-ui/css/theme.css` now includes a complete set of fumadocs override styles so any app importing the theme gets correct defaults out of the box: nav/header height variables, sidebar drawer positioning (left-side on mobile), z-index stack (header → backdrop → sidebar drawer → search dialog → Radix poppers), search bar colors, external link icons, link underline styles, and heading anchor alignment. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Closes #825
Summary by CodeRabbit
New Features
UI / Styling
Documentation