Skip to content

Declare Sprint 3 theme tokens and match navbar exclusions by prefix - #305

Merged
shaply merged 2 commits into
mainfrom
claude/hopeful-bohr-3evtc1-theme-navbar
Sep 30, 2026
Merged

shaply merged 2 commits into
mainfrom
claude/hopeful-bohr-3evtc1-theme-navbar

Conversation

@shaply

@shaply shaply commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Part of #295: the theme tokens and the navbar prefix matching. Schema and contracts are in #304. This PR doesn't depend on it, and neither needs a migration from the other.

Theme tokens (styles/globals.css)

These tokens are declared with today's values, so #299 and #301 can use the classes from day one. Nothing uses them yet, so nothing looks different. #300 makes them follow each org.

Token Class Today's value
--color-page-bg bg-page-bg #ffffff (the body's default)
--color-page-text text-page-text var(--color-grey-text-strong) (the app's main text color)
--color-brand-foreground text-brand-foreground #ffffff (the text-white on today's bg-brand-text buttons)
--font-display font-display var(--font-heading)
--radius-control rounded-control 0.25rem (Tailwind's rounded, as used on the auth pages and BogButton)

I compiled globals.css with the Tailwind CLI to check that all five classes are generated and that the variables they reference are emitted.

Navbar prefix matching (lib/navbar.ts)

NO_NAVBAR_PAGES entries ending in / now match every path under them. /forms/ matches /forms/abc but not /forms or /formsabc. Other entries still match exactly. NavbarWrapper uses the new isNoNavbarPage, so #299 (/forms/) and #301 (/accept-invitation/) only need to add their entry.

Testing

  • New: matchesPage (prefix vs. exact, /formsabc doesn't match /forms/) and a NavbarWrapper component test.
  • Full unit suite (53 files, 573 tests), pnpm lint, pnpm format, and tsc --noEmit pass locally.

Checklist:

🤖 Generated with Claude Code

https://claude.ai/code/session_017RYaE2zBcm7SfFBJdydeuR


Generated by Claude Code

Two Sprint 3 pages need a navbar-free subtree (/forms/ and
/accept-invitation/), so NO_NAVBAR_PAGES entries ending in "/" now match
every path under them. Other entries still match exactly.

Declare --color-page-bg, --color-page-text, --color-brand-foreground,
--font-display, and --radius-control with today's values, so the forms and
auth pages can use their Tailwind classes before per-org theming lands.
Nothing uses them yet, so nothing looks different.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RYaE2zBcm7SfFBJdydeuR
@netlify

netlify Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for servicestart ready!

Name Link
🔨 Latest commit 2297e3a
🔍 Latest deploy log https://app.netlify.com/projects/servicestart/deploys/6abc8542aabc9000087b9147
😎 Deploy Preview https://deploy-preview-305--servicestart.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Disclaimer: This comment was AI-generated and is not necessarily completely accurate. Please take code comments with a grain of salt.

RetriggerConfidence Score: 4/5

[Low risk] Refactors navbar exclusion logic and adds theme token declarations.

The PR appears safe to merge, with a non-blocking gap in component-level coverage of prefix exclusions.

Findings

  1. P2 Prefix behavior lacks component coverage ▶

Summary

The PR adds theme tokens and routes navbar exclusions through a helper that supports trailing-slash prefixes.

  • The prefix matcher has unit coverage, but the new component tests do not exercise prefix behavior.
  • Checklist area to improve: test the new navbar behavior through NavbarWrapper. Review score: 90/100.

Reviews (1) · Last reviewed commit: "Declare Sprint 3 theme tokens and match ..."

Comment thread tests/unit/components/navigation/NavbarWrapper.test.tsx
From review: the component tests only used exact entries, so they would
pass if the wrapper went back to exact matching. Add a prefix entry in
the test and check a nested path and a lookalike path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RYaE2zBcm7SfFBJdydeuR

shaply commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Setup Preview / create-db is red for a reason outside this PR. Migrate and seed pass; the last step, Setup organization, fails when Juno's email/register-domain returns 400. That step fails on most recent PRs too, including #296, which merged with it red. This PR only changes CSS tokens and the navbar matcher. No fix exists yet. I've re-run the job once to confirm it reproduces.


Generated by Claude Code

@shaply
shaply merged commit 4900bfe into main Sep 30, 2026
25 checks passed

This branch was successfully deployed

1 active deployment
Preview — 2297e3ad Deployed Sep 30, 2026 by shaply via drop-db #265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants