refactor: refactoring navbars by jderochervlk · Pull Request #1195 · rescript-lang/rescript-lang.org

jderochervlk · 2026-02-16T14:17:17Z

This refactors the navbar to not be so brittile and depend on checking height for layouts. The main change is using the modern html dialog component for the mobile menus and switching the header to use sticky to 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.

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:

Introduced and integrated NavbarSecondary and NavbarTertiary components 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]
Refactored the main app layout to simplify the root structure and ensure the new navbars are rendered appropriately. (app/root.res, [1] [2]

Testing and Automation Improvements:

Added comprehensive, responsive UI tests for NavbarPrimary and NavbarTertiary, covering desktop, tablet, and mobile scenarios, including screenshot assertions for visual regression testing. (__tests__/NavbarPrimary_.test.res, __tests__/NavbarTertiary_.test.res, [1] [2]
Automated the process of committing and pushing updated Vitest screenshots after test runs, ensuring that visual changes are consistently tracked in CI. (.github/workflows/pull-request.yml, [1] [2]

Routing and Configuration Updates:

Improved route definitions by precomputing mdxRoutes and 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]
Updated the test script to always update screenshots on CI runs, and adjusted test/dev dependencies configuration for clarity. (package.json, rescript.json, [1] [2]

Component and Internal Refactoring:

Simplified the ApiDocs.SidebarTree and RightSidebar components to remove unused props and streamline sidebar rendering logic. (src/ApiDocs.res, [1] [2]

Miscellaneous:

Removed unused debug logging from the Open Graph image function. (functions/ogimage/[[path]]/index.png.res, functions/ogimage/[[path]]/index.png.resL19-L20)
Removed an obsolete example test file. (__tests__/Example.test.res, tests/Example.test.resL1-L28)

fhammerschmidt · 2026-02-17T08:39:47Z

this broke the version dropdown

jderochervlk · 2026-02-17T14:31:29Z

this broke the version dropdown

Fixed and I added a unit test.

Copilot

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.

package.json

.github/workflows/pull-request.yml

Copilot · 2026-02-18T14:47:20Z

src/components/NavbarMobileOverlay.res

+      <li className=base>
+        <Link
+          prefetch={#intent}
+          to=#"/blog"
+          className={linkOrActiveLinkSubroute(~target=#"/blog", ~route)}
+        >
+          {React.string("Blog")}
+        </Link>
+      </li>
+      <li className=base>
+        <Link
+          prefetch={#intent}
+          to=#"/community/overview"
+          className={linkOrActiveLink(~target=#"/community/overview", ~route)}
+        >
+          {React.string("Community")}
+        </Link>
+      </li>
+      <li className=base>
+        <Link
+          prefetch={#intent}
+          to=#"/packages"
+          className={linkOrActiveLink(~target=#"/packages", ~route)}
+        >
+          {React.string("Packages")}
+        </Link>
+      </li>


Only the “Playground” link closes the mobile overlay (onClick=toggleMobileOverlay), but the other in-app links don’t. Because the overlay component is mounted globally (in NavbarPrimary), navigation won’t automatically unmount/close the dialog, so the overlay can remain open after route changes. Add an onClick handler to close the overlay for all navigation links (and consider doing the same for external links).

@copilot open a new pull request to apply changes based on this feedback

src/components/BreadCrumbs.res

Copilot · 2026-02-18T14:47:21Z

src/layouts/SidebarLayout.resi

-  ~editHref: string=?,
  // (Sidebar, toggleSidebar) ... for toggling sidebar in mobile view
-  ~sidebarState: (bool, (bool => bool) => unit),
+  ~sidebarState: (bool, (bool => bool) => unit)=?,


This interface marks sidebarState as an optional labeled argument (=?), but the implementation uses a default value (~sidebarState=(false, _ => ())) and treats it as a non-option tuple. This mismatch is likely to cause a signature/type mismatch. Make the .resi match the implementation (remove =? and keep the tuple type), or change the implementation to accept an option<...> and handle None.

Suggested change

~sidebarState: (bool, (bool => bool) => unit)=?,

~sidebarState: (bool, (bool => bool) => unit),

Copilot · 2026-02-18T14:47:21Z

__tests__/NavbarPrimary_.test.res

+
+  let leftContent = await screen->getByTestId("navbar-primary-left-content")
+
+  await element(leftContent->getByText("Docs"))->toBeVisible


getByText / getByLabelText are defined in src/bindings/Vitest.res as returning promise<element>, but these calls aren’t awaited before passing into element(...). This will either fail to type-check or behave incorrectly at runtime. Also, the aria-label in the component is "GitHub" (capital H), while the test uses "Github", so the query will fail even after awaiting.

Copilot · 2026-02-18T14:47:21Z

__tests__/NavbarPrimary_.test.res

+
+  let rightContent = await screen->getByTestId("navbar-primary-right-content")
+
+  await element(rightContent->getByLabelText("Github"))->toBeVisible


getByText / getByLabelText are defined in src/bindings/Vitest.res as returning promise<element>, but these calls aren’t awaited before passing into element(...). This will either fail to type-check or behave incorrectly at runtime. Also, the aria-label in the component is "GitHub" (capital H), while the test uses "Github", so the query will fail even after awaiting.

Copilot · 2026-02-18T14:47:22Z

vitest.config.mjs

@@ -0,0 +1,23 @@
+import { defineConfig } from "vitest/config";
+import { playwright, defineBrowserCommand } from "@vitest/browser-playwright";


defineBrowserCommand is imported but not used anywhere in this (new) config file. Drop the unused import to keep the config minimal and avoid lint/typecheck noise.

Suggested change

import { playwright, defineBrowserCommand } from "@vitest/browser-playwright";

import { playwright } from "@vitest/browser-playwright";

src/components/Search.res

Copilot · 2026-02-18T14:47:23Z

src/components/NavbarTertiary.res

+      ${isDocRoute(~route=pathname) ? "top-28" : "top-16"}`}
+    >
+      <div className="flex items-center h-full w-full max-w-1280 m-auto">
+        <button className="md:hidden mr-3" onClick={toggleMobileTertiaryDrawer}>


The mobile drawer toggle button lacks an accessible name (no ariaLabel / aria-label), so screen readers won’t know what it does. Add an aria label (and consider setting type_="button" to avoid accidental form submission if this navbar is ever rendered inside a form).

Suggested change

<button className="md:hidden mr-3" onClick={toggleMobileTertiaryDrawer}>

<button

className="md:hidden mr-3"

onClick={toggleMobileTertiaryDrawer}

ariaLabel="Toggle navigation menu"

type_="button">

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-02-18T18:50:29Z

@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>

github-actions · 2026-02-18T18:52:20Z

Cloudflare deployment

Deployement ID: 743feab0-38ab-4c17-ae5f-4b7ca37a1e40
Deployment Environment: preview

⛅️ wrangler 4.63.0 (update available 4.66.0)
─────────────────────────────────────────────
✨ Compiled Worker successfully
Uploading... (6810/7224)
Uploading... (6948/7224)
Uploading... (7086/7224)
Uploading... (7224/7224)
✨ Success! Uploaded 414 files (6810 already uploaded) (3.68 sec)

✨ Uploading _redirects
✨ Uploading Functions bundle
🌎 Deploying...
✨ Deployment complete! Take a peek over at https://743feab0.rescript-lang.pages.dev
✨ Deployment alias URL: https://vlk-refactor-navigation.rescript-lang.pages.dev

jderochervlk added 17 commits February 8, 2026 14:09

refactor: refactoring navbars

51dc3f8

making progress on the nav

be70e99

scrolling works again

0808cbc

refactoring

949389f

mobile overlay works

bd7b348

tests are passing

482b405

done with mobile overlay

80b836d

screenshots

3eb6f20

removing top padding

53682b0

fix playground

876486a

remove unused open

57dae95

fix issue with homepage reloading nonstop

b7de7a6

restor mdx route

86f806b

remove dummy page

09e42d4

fixing docs

f642c1c

fix syntax lookup

6e0bc9d

remove build errors and warnings

c671d16

github-actions bot deployed to preview February 16, 2026 22:29 View deployment

jderochervlk and others added 2 commits February 16, 2026 17:36

commit screenshot changes

e0bf40d

chore: update vitest screenshots [skip ci]

21a333c

github-actions bot deployed to preview February 16, 2026 22:38 View deployment

jderochervlk added 2 commits February 16, 2026 17:55

remove unused values

f4ec369

remove unused file

f66a9f1

github-actions bot deployed to preview February 16, 2026 22:57 View deployment

tidying up

5e5c0e1

jderochervlk marked this pull request as ready for review February 16, 2026 23:04

jderochervlk requested review from aspeddro, fhammerschmidt and shulhi February 16, 2026 23:04

github-actions bot deployed to preview February 16, 2026 23:05 View deployment

fix version dropdown

76414e8

github-actions bot deployed to preview February 17, 2026 14:29 View deployment

jderochervlk requested a review from Copilot February 18, 2026 14:32

Copilot AI reviewed Feb 18, 2026

View reviewed changes

Update src/components/Search.res

95b07eb

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Feb 18, 2026

Fix mobile overlay not closing on navigation link clicks #1198

Draft

Update src/components/BreadCrumbs.res

a15986e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-actions bot deployed to preview February 18, 2026 18:52 View deployment

	~sidebarState: (bool, (bool => bool) => unit)=?,
	~sidebarState: (bool, (bool => bool) => unit),


		let leftContent = await screen->getByTestId("navbar-primary-left-content")

		await element(leftContent->getByText("Docs"))->toBeVisible


		let rightContent = await screen->getByTestId("navbar-primary-right-content")

		await element(rightContent->getByLabelText("Github"))->toBeVisible

		@@ -0,0 +1,23 @@
		import { defineConfig } from "vitest/config";
		import { playwright, defineBrowserCommand } from "@vitest/browser-playwright";

	import { playwright, defineBrowserCommand } from "@vitest/browser-playwright";
	import { playwright } from "@vitest/browser-playwright";

Uh oh!

Conversation

jderochervlk commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhammerschmidt commented Feb 17, 2026

Uh oh!

jderochervlk commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

jderochervlk Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 18, 2026

Uh oh!

github-actions bot commented Feb 18, 2026

Cloudflare deployment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

jderochervlk commented Feb 16, 2026 •

edited

Loading