Open
Conversation
Member
|
this broke the version dropdown |
Collaborator
Author
Fixed and I added a unit test. |
There was a problem hiding this comment.
Pull request overview
This PR refactors navigation/layout to rely on sticky headers and dialog-based mobile menus instead of dynamically measuring header heights, alongside adding browser-based Vitest UI coverage for the new nav components.
Changes:
- Introduces new Primary/Secondary/Tertiary navbar components and migrates routes/layouts to use them.
- Reworks sidebar/mobile drawer behavior using
dialog(and updates related CSS). - Adds Vitest browser setup/config and new UI/screenshot tests for nav and version selection.
Reviewed changes
Copilot reviewed 45 out of 52 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.setup.mjs | Loads global CSS for Vitest browser runs. |
| vitest.config.mjs | New Vitest browser (Playwright) configuration. |
| vitest.config.js | Removes old Vitest config in favor of .mjs. |
| vite.config.mjs | Reorders plugins and removes unused reactRefreshHost config. |
| styles/main.css | Updates Tailwind sources and adds dialog/popover + scroll-lock styling. |
| src/layouts/SidebarLayout.resi | Adjusts SidebarLayout public API and exports breadcrumbs module. |
| src/layouts/SidebarLayout.res | Removes height-based offsets and old mobile navbar block; simplifies layout. |
| src/layouts/MainLayout.res | Removes extra top margin now that header is sticky. |
| src/layouts/LandingPageLayout.res | Fixes malformed Playground URL query string. |
| src/layouts/DocsLayout.res | Removes breadcrumb/editHref plumbing from DocsLayout. |
| src/layouts/CommunityLayout.res | Removes breadcrumbs prop usage to match new navbar approach. |
| src/layouts/ApiOverviewLayout.resi | Exposes categories for route-side rendering. |
| src/layouts/ApiOverviewLayout.res | Drops breadcrumbs usage and relies on tertiary navbar instead. |
| src/layouts/ApiLayout.resi | Removes optional breadcrumbs prop from API layout signature. |
| src/layouts/ApiLayout.res | Removes breadcrumbs prop usage when rendering SidebarLayout. |
| src/components/VersionSelect.res | Uses useId to generate unique popover IDs per instance. |
| src/components/Search.res | Tweaks search button/icon classes. |
| src/components/NavbarUtils.res | Adds shared navbar helpers + dialog open/close/toggle utilities. |
| src/components/NavbarTertiary.res | Adds tertiary sticky navbar with dialog-based mobile drawer. |
| src/components/NavbarSecondary.res | Adds secondary sticky navbar for docs-related sections. |
| src/components/NavbarPrimary.resi | Exposes primary navbar component interface. |
| src/components/NavbarPrimary.res | Adds primary sticky navbar + mobile overlay toggle. |
| src/components/NavbarMobileOverlay.res | Adds dialog-based mobile overlay navigation menu. |
| src/components/BreadCrumbs.res | Adds simple pathname-derived breadcrumbs component. |
| src/common/Util.res | Removes debug logging. |
| src/bindings/Vitest.res | Expands vitest-browser bindings (viewport, locators, screenshots). |
| src/bindings/ReactRouter.res | Adds Link onClick + aria-label support and BrowserRouter binding. |
| src/SyntaxLookup.res | Removes extra top spacing to match sticky nav behavior. |
| src/Playground.res | Removes top margin now that header is sticky. |
| src/Packages.res | Removes wrapper spacing container to match new header stacking. |
| src/Blog.res | Removes wrapper spacing container to match new header stacking. |
| src/ApiDocs.res | Simplifies sidebar tree for desktop and removes mobile sidebar toggling logic. |
| package.json | Changes CI test script to always update screenshots. |
| functions/ogimage/[[path]]/index.png.res | Removes debug logging. |
| app/routes/SyntaxLookupRoute.res | Adds secondary navbar to syntax lookup route. |
| app/routes/MdxRoute.res | Adds secondary/tertiary navbars and mobile sidebar content for docs/API MDX. |
| app/routes/DocsOverview.res | Removes extra top margin to match sticky nav behavior. |
| app/routes/ApiRoute.res | Adds secondary/tertiary navbars and sidebar content for API route. |
| app/routes.res | Precomputes mdx routes list for cleaner route definition. |
| app/root.res | Removes previous overlay/scroll-lock provider and renders NavbarPrimary globally. |
| tests/VersionSelect_.test.res | Adds browser tests for VersionSelect popover behavior. |
| tests/NavbarTertiary_.test.res | Adds responsive + screenshot tests for tertiary navbar/drawer. |
| tests/NavbarPrimary_.test.res | Adds responsive + screenshot tests for primary navbar/mobile overlay. |
| tests/Example.test.res | Removes example test file. |
| .github/workflows/pull-request.yml | Auto-commits updated Vitest screenshots after CI test run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
|
@jderochervlk I've opened a new pull request, #1198, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Initial plan * Add onClick handlers to close mobile overlay for all navigation links Co-authored-by: jderochervlk <60623931+jderochervlk@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jderochervlk <60623931+jderochervlk@users.noreply.github.com>
Cloudflare deploymentDeployement ID: 57f18918-65f2-4572-824c-a6eec8303b7f ⛅️ wrangler 4.63.0 (update available 4.67.0) ✨ Uploading _redirects |
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.
This refactors the navbar to not be so brittile and depend on checking height for layouts. The main change is using the modern html
dialogcomponent for the mobile menus and switching the header to usestickyto stay in place. Now the content body doesn't need to add margin or padding to account for any headers, they just show up above and can stack easily.There will be some more refactors soon to split up the mdx route, but one thing at a time.
I changed the structure of the "tertiary navbar" which has the bread crumbs and edit button for doc pages. Instead of having it inset into the content block it now sits above so the sidebar can be a child of it. This makes it easier to manage outside of the content as part of the layout. This also makes it consistent across all screen sizes.
Before
After
AI summary
This pull request introduces significant improvements to the navigation and documentation layout, particularly around the API and documentation pages. It adds new responsive navigation bar tests, refactors and enhances the tertiary navbar and sidebar for API and docs routes, and automates the management of Vitest screenshot updates. Additionally, it updates the routing and configuration for better scalability and maintainability.Navigation and Layout Enhancements:
NavbarSecondaryandNavbarTertiarycomponents across API and documentation routes, providing consistent breadcrumbs, sidebar content, and edit links for improved user navigation and editing experience. The sidebar now adapts based on route and viewport, supporting mobile drawer toggling and version selection. (app/routes/ApiRoute.res,app/routes/MdxRoute.res,app/routes/SyntaxLookupRoute.res, [1] [2] [3] [4] [5] [6]app/root.res, [1] [2]Testing and Automation Improvements:
NavbarPrimaryandNavbarTertiary, covering desktop, tablet, and mobile scenarios, including screenshot assertions for visual regression testing. (__tests__/NavbarPrimary_.test.res,__tests__/NavbarTertiary_.test.res, [1] [2].github/workflows/pull-request.yml, [1] [2]Routing and Configuration Updates:
mdxRoutesand streamlining their inclusion, and added support for a new "guide" section in the markdown and routing configuration. (app/routes.res,react-router.config.mjs, [1] [2] [3]package.json,rescript.json, [1] [2]Component and Internal Refactoring:
ApiDocs.SidebarTreeandRightSidebarcomponents to remove unused props and streamline sidebar rendering logic. (src/ApiDocs.res, [1] [2]Miscellaneous:
functions/ogimage/[[path]]/index.png.res, functions/ogimage/[[path]]/index.png.resL19-L20)__tests__/Example.test.res, tests/Example.test.resL1-L28)