Conversation
Add full mobile navigation for `docs`. Remove `fixed` from `Header`.
There was a problem hiding this comment.
Pull request overview
This PR implements mobile navigation for the docs site using a drawer pattern and removes the fixed positioning from the header. The changes modernize the navigation system to use Svelte 5 runes and improve mobile user experience.
Changes:
- Added a DaisyUI drawer component for mobile navigation that displays the full navigation tree
- Removed fixed positioning from the Header component to simplify layout
- Refactored Navigation component to support both inline (desktop) and drawer (mobile) modes with proper state management using Svelte 5 runes
- Extracted navigation item rendering logic into a new recursive NavigationItem component
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/src/routes/+layout.svelte | Added drawer component wrapper with mobile navigation, removed fixed header offset |
| docs/src/routes/(content)/+layout.svelte | Updated Navigation component usage with styling props |
| docs/src/lib/layouts/MdLayout.svelte | Reordered imports (linter fix) |
| docs/src/lib/consts.ts | Added DRAWER_ID constant for drawer/label association |
| docs/src/lib/components/TableOfContents.svelte | Moved sticky top positioning from CSS to utility class |
| docs/src/lib/components/NavigationItem.svelte | New recursive component for rendering navigation items with proper active states |
| docs/src/lib/components/Navigation.svelte | Refactored to use Svelte 5 runes, added full/onLinkClick/class props for flexibility |
| docs/src/lib/components/Header.svelte | Replaced dropdown mobile menu with drawer trigger button |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div class="flex-none lg:hidden"> | ||
| <label for={DRAWER_ID} aria-label="open sidebar" class="btn btn-circle btn-ghost"> | ||
| <svg class="h-24 w-24" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16" /> | ||
| </svg> | ||
| </summary> | ||
| <ul | ||
| class="dropdown-content menu mt-md flex w-max max-w-[80vw] flex-col gap-md rounded-md bg-base-100 p-2 py-lg text-lg shadow-lg"> | ||
| {#each navigation as section} | ||
| {@const isActive = isActiveSection(section)} | ||
| {@const firstChild = getFirstChild(section)} | ||
| <li><a href={firstChild} class="flex justify-end">{section.title}</a></li> | ||
| {/each} | ||
| <li class="flex justify-end"><GithubIcon /></li> | ||
| </ul> | ||
| </details> | ||
| </label> | ||
| </div> |
There was a problem hiding this comment.
The wrapping div with class "flex-none lg:hidden" is redundant since the parent already has "md:hidden" which will hide this entire section on medium screens and above. The lg:hidden class serves no additional purpose here.
| <div class="navbar-end gap-md md:hidden"> | ||
| <GithubIcon /> | ||
| <div class="flex-none lg:hidden"> | ||
| <label for={DRAWER_ID} aria-label="open sidebar" class="btn btn-circle btn-ghost"> |
There was a problem hiding this comment.
The aria-label "open sidebar" should have consistent capitalization with the close button's aria-label "Close menu" on line 26 of +layout.svelte. Consider changing this to "Open sidebar" or "Open menu" for consistency.
| <label for={DRAWER_ID} aria-label="open sidebar" class="btn btn-circle btn-ghost"> | |
| <label for={DRAWER_ID} aria-label="Open sidebar" class="btn btn-circle btn-ghost"> |
|
|
||
| {#if filteredHeadings.length > 0} | ||
| <nav class="toc" aria-label="Table of contents"> | ||
| <nav class="toc top-xl" aria-label="Table of contents"> |
There was a problem hiding this comment.
The top position has changed from 8rem to top-xl (which resolves to 2.5rem based on the CSS custom properties). This significantly alters the sticky positioning of the table of contents. Please verify this is intentional, as it will make the TOC stick much closer to the top of the viewport.
Add full mobile navigation for
docs.Remove
fixedfromHeader.