fix: sidebar menu collapse/expand behaviour#435
Open
niallobrien wants to merge 7 commits into
Open
Conversation
- Add normalizePath utility to compare paths without trailing slashes - Update menu util functions to use normalized paths - Refactor Sidenav to use new menu matching helpers - Expand menu sections based on active path or section type - Add tests for path normalization and menu matching logic
…avigation-bug-in-docs
- Implement hash-based scrolling with retries for layout transitions - Add context and hooks for managing sidenav hash state - Support sibling branch collapse and expand logic in sidenav - Refactor sidenav item actions for hash anchors and sibling branches - Add tests for hash scroll and sidenav interaction logic
- Add getCanonicalActiveAncestors to resolve a single active menu branch when multiple menu items share a path - Refactor Sidenav to use SidenavActiveMenuContext for ancestor tracking - Update hash scroll logic to handle top scroll and deferred scroll on navigation - Add resolveHashScrollEffect for explicit scroll behavior decisions - Improve sidenav navigation to clear hash without scrolling when appropriate - Update and add tests for new menu and scroll behaviors
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the docs sidebar (and related hash scrolling) to fix incorrect expand/collapse and active-branch behavior, especially when menu entries share paths or use hash-anchor “section” links.
Changes:
- Added path normalization (
normalizePath) and applied it to active menu resolution to avoid trailing-slash mismatches. - Introduced “canonical” active-ancestor resolution to handle duplicate menu paths, plus a dedicated click/chevron interaction resolver for predictable expand/collapse vs navigation behavior.
- Reworked hash scrolling so hash/route transitions scroll correctly (including when clearing hashes) and removed the older navigation scroll-to-top logic.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/url.js | Adds normalizePath helper used for consistent pathname comparisons. |
| src/util/url.test.ts | Unit tests for normalizePath. |
| src/lib/windowLocationHash.ts | Adds a small client hook/store wrapper for subscribing to window.location.hash. |
| src/lib/menu/util.ts | Expands menu utilities (canonical active branch resolution, hash-anchor helpers, expand defaults). |
| src/lib/menu/util.test.ts | Adds tests covering trailing-slash normalization, duplicate-path canonicalization, and hash-anchor expand behavior. |
| src/lib/menu/sidenavInteractions.ts | Centralizes sidenav click/chevron behavior decisions into pure resolver functions. |
| src/lib/menu/sidenavInteractions.test.ts | Tests interaction resolution across hash-anchor and sibling-branch scenarios. |
| src/components/WithQuickNav/WithQuicknav.tsx | Moves hash scrolling responsibility into a hook invoked by the page wrapper. |
| src/components/Sidenav/useSidenavItemActions.ts | New hook to apply resolved interactions + coordinate hash/navigation/expand state. |
| src/components/Sidenav/SidenavSiblingBranches.tsx | New provider to coordinate sibling branch collapse behavior within a list level. |
| src/components/Sidenav/sidenavNavigation.ts | New helper functions for hash-clearing and normalized navigation behavior. |
| src/components/Sidenav/SidenavHashContext.tsx | Adds a hash context to reconcile window.location.hash with “optimistic” UI hash state. |
| src/components/Sidenav/SidenavActiveMenuContext.tsx | Provides canonical active-ancestor state to all sidenav rows. |
| src/components/Sidenav/Sidenav.tsx | Refactors rendering to use stable keys, separate chevron/link interactions, and new contexts. |
| src/components/Sidenav/Sidenav.module.css | Updates styles for the new DOM structure (button + link) and root list padding. |
| src/components/QuickNav/QuickNav.tsx | Removes prior hashchange scroll handling (now handled by useHashScroll). |
| src/components/QuickNav/useHashScroll.ts | New hook implementing route/hash scroll behavior via scheduling utilities. |
| src/components/QuickNav/resolveHashScrollEffect.ts | Pure function to decide which scroll effect to run based on route/hash transition. |
| src/components/QuickNav/resolveHashScrollEffect.test.ts | Unit tests for hash scroll effect resolution. |
| src/components/QuickNav/hashScroll.ts | Adds robust, retrying scroll-to-hash / scroll-to-top scheduling (incl. post-layout retry). |
| src/components/QuickNav/hashScroll.test.ts | Tests scroll scheduling and cancellation behavior. |
| src/app/navigation.tsx | Removes prior global scroll-to-top-on-navigation behavior (delegated to useHashScroll on docs pages). |
| src/app/(guides)/guides/[...slug]/page.tsx | Switches parent-title resolution to canonical active ancestors. |
| src/app/(documentation)/[...slug]/page.tsx | Switches parent-title resolution to canonical active ancestors. |
| src/app/(api)/api/[...slug]/page.tsx | Switches parent-title resolution to canonical active ancestors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add Cloudsmith copyright headers to all relevant files - Simplify and reorder import statements for clarity - Use consistent import style for type-only imports - No functional changes to logic or behavior
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Story
https://linear.app/cloudsmith/issue/WEB-365/sidebar-navigation-bug-in-docs
Description
Fixes sidebar navigation issues by normalising path comparisons, using stable React keys for menu items with duplicate paths, and splitting expand/collapse from navigation.
I then discovered even more issues which needed addressing - primarily with regards to the
Developer ToolsandAbout Cloudsmithmenu items (clicking vs manually navigating to the urls), hash-based scrolling issues and https://docs.cloudsmith.com/developer-tools/vscode resolving to multiple items in the sidebar, so this PR definitely "grew legs" the more I dug into everything.I've reviewed these changes with multiple agents as it grew in complexity and tried to make it as manageable as possible, adding tests where recommended etc..
Screenshots
Compare the actions I highlight here to what's currently live - it's a night & day difference.
docs-site-sidebar.mp4