Skip to content

feat: mobile nav popover, offline indicator, Varlock test env - #70

Merged
fringe4life merged 3 commits into
mainfrom
feat/mobile-nav-offline-varlock-tests
Aug 24, 2026
Merged

feat: mobile nav popover, offline indicator, Varlock test env#70
fringe4life merged 3 commits into
mainfrom
feat/mobile-nav-offline-varlock-tests

Conversation

@fringe4life

@fringe4life fringe4life commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Summary

🎨 Mobile navbar popover menu with avatar or hamburger trigger below sm
🧩 Navbar split into focused modules (desktop/mobile nav, auth slots, shell styles)
📡 Offline indicator with container-responsive detail; wordmark logo from xs (480px)
🔧 Varlock test env via .env.test and preload shim — no Bitwarden fetch in tests
🐛 Disabled NavLink prefetch by default; dismiss popover on nav link click via hidePopover()
🚀 Vercel Bun 1.4 config for bun.lock lockfileVersion 2
🧪 Navbar component tests + mobile menu e2e; expanded sign-in component and e2e auth coverage
📝 Synced README badges, nav/offline docs, and Varlock test notes
📦 Regenerated bun.lock and bumped Next, Varlock, Panda, nuqs deps

Closes #64
Closes #67
Closes #68
Closes #69
Closes #71
Closes #72

Test plan

  • bun run test — 67 tests pass without Bitwarden/network
  • bun run type — passes via pre-commit hook
  • bun run check — Biome/Ultracite clean on staged files
  • Vercel deploy — Bun 1.4 install + build succeed
  • Manual: mobile viewport — popover opens from avatar (signed in) or hamburger (signed out)
  • Manual: nav link click closes popover
  • Manual: toggle offline in DevTools — offline pill with responsive detail
  • Manual: network tab — nav links no longer prefetch by default

Made with Cursor

🎨 Mobile navbar popover menu with avatar or hamburger trigger
📡 Restyled offline indicator with pulse offline and solid online states
🔧 Varlock test env via .env.test and preload shim, no Bitwarden fetch
🐛 Disabled NavLink prefetch by default to reduce network noise
🧪 Expanded sign-in component and e2e auth test coverage
📝 Synced README badges, nav/offline docs, and Varlock test notes
📦 Regenerated bun.lock and bumped Next, Varlock, Panda, nuqs deps

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 47d3d1c.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Mobile nav popover, restyled offline indicator, and Varlock test env

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Adds a native popover mobile nav menu (avatar/hamburger trigger) for viewports below md.
• Restyles the offline indicator and adjusts its visibility/ARIA behavior.
• Adds .env.test + Varlock preload shim so tests avoid Bitwarden/network.
• Sets NavLink prefetch off by default, with explicit opt-in where needed.
• Expands sign-in component and Playwright auth error coverage; updates README and deps.
Diagram

graph TD
  Navbar["Navbar"] --> Popover["Mobile popover"] --> NavLink["NavLink"]
  Popover --> AuthState["Auth state"]
  Navbar --> OfflineBanner["Offline banner"] --> UseOffline{{"next/offline"}}
  BunTests["Bun tests"] --> EnvTest[(".env.test")]
  subgraph Legend
    direction LR
    _cmp["Component"] ~~~ _db[("Env file")] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a headless UI menu/dialog for mobile nav
  • ➕ Provides consistent cross-browser behavior without relying on Popover/anchor-position support
  • ➕ Built-in focus trapping and keyboard navigation patterns
  • ➖ Adds dependency/JS weight and styling overhead
  • ➖ Harder to leverage native discrete transitions and anchor positioning
2. Keep prefetch enabled and gate via heuristics
  • ➕ Preserves perceived nav speed for common routes
  • ➕ Can tune to only prefetch on idle/fast networks
  • ➖ More complexity and still risks network noise in dev/tests
  • ➖ Harder to reason about and debug than explicit opt-in

Recommendation: The current approach is good: native popover + anchor positioning keeps dependencies low and matches platform semantics. The main caveat is browser support; if you need wider support (older Safari), consider swapping just the mobile menu to a headless UI dialog/menu while keeping the rest of the structure intact. Prefetch default-off is a clear, debuggable choice; continue opting in on the few routes where it materially helps.

Files changed (26) +908 / -479

Enhancement (4) +446 / -66
navbar.tsxIntroduce mobile popover navigation and reorganize desktop nav +384/-50

Introduce mobile popover navigation and reorganize desktop nav

• Adds a below-'md' popover menu using native Popover API + anchor positioning with an animated hamburger trigger and auth-driven account row. Keeps desktop nav as inline links and integrates the offline banner in the right-side cluster.

src/components/navbar/navbar.tsx

offline-indicator.tsxRedesign OfflineBanner UI and accessibility attributes +60/-14

Redesign OfflineBanner UI and accessibility attributes

• Reworks the offline indicator into a styled pill with iconography and supporting text, and adds 'role="status"' with polite ARIA live announcements.

src/components/offline-indicator.tsx

sign-in-nav-link.tsxOpt sign-in link into prefetch +1/-0

Opt sign-in link into prefetch

• Sets 'prefetch' explicitly on the sign-in link now that NavLink defaults to prefetch off.

src/features/auth/components/sign-in-nav-link.tsx

search-models.tsTune cache lifetime for model search results +1/-2

Tune cache lifetime for model search results

• Adjusts caching to use 'cacheLife("hours")' (and sets it earlier) for model search responses.

src/features/models/dal/search-models.ts

Bug fix (4) +9 / -5
submit-button.tsxFix loading spinner animation props +3/-1

Fix loading spinner animation props

• Replaces Panda 'animation' shorthand with explicit 'animationName'/'animationDuration'/'animationIterationCount' for the submit spinner.

src/components/form/submit-button.tsx

nav-link.tsxDefault NavLink prefetch to false +1/-1

Default NavLink prefetch to false

• Changes the default 'prefetch' behavior to opt-in, reducing background prefetch traffic and test noise.

src/components/nav-link/nav-link.tsx

search-input.tsxFix search loading spinner animation props +3/-1

Fix search loading spinner animation props

• Updates the spinner animation config to use explicit Panda animation properties.

src/components/search-input/search-input.tsx

pagination-offset-transition.tsxFix pagination exit animation naming +2/-2

Fix pagination exit animation naming

• Moves from 'animation' shorthand to 'animationName' for the exit view transitions.

src/features/pagination/components/pagination-offset-transition.tsx

Refactor (1) +1 / -1
model-back-link.tsxMinor NavLink prefetch prop cleanup +1/-1

Minor NavLink prefetch prop cleanup

• Uses 'prefetch' shorthand instead of 'prefetch={true}' (no behavior change).

src/features/models/back-link/model-back-link.tsx

Tests (5) +74 / -6
field-errors.tsxExpose field errors via stable test ids +4/-1

Expose field errors via stable test ids

• Adds 'data-testid="field-error-{name}"' to the rendered field error span.

src/components/form/field-errors.tsx

form-error.tsxExpose form error via stable test id +1/-0

Expose form error via stable test id

• Adds 'data-testid="form-error"' to the form error container for easier component/e2e assertions.

src/components/form/form-error.tsx

signin-page.spec.tsxAdd coverage for form error clearing and test ids +38/-3

Add coverage for form error clearing and test ids

• Updates assertions to use 'form-error'/'field-error-*' test ids and adds a case ensuring auth errors clear after correcting credentials.

tests/components/auth/signin-page.spec.tsx

login-logout.spec.tsClean up login/logout e2e test comments +0/-2

Clean up login/logout e2e test comments

• Removes outdated commentary; behavior remains the same.

tests/e2e/auth/login-logout.spec.ts

signin-errors.spec.tsAdd e2e regression for auth error then success +31/-0

Add e2e regression for auth error then success

• Adds a Playwright flow verifying invalid credentials show a form error and that retrying with valid credentials succeeds and navigates to /3d-models.

tests/e2e/auth/signin-errors.spec.ts

Documentation (2) +26 / -22
README.mdDocument mobile nav, offline indicator, and Varlock test setup +23/-22

Document mobile nav, offline indicator, and Varlock test setup

• Updates tech-stack badges/versions and adds notes about mobile popover navigation, offline indicator styling/behavior, and '.env.test' usage in tests.

README.md

doctor.config.tsAdd Varlock key generation note +3/-0

Add Varlock key generation note

• Adds a comment showing how to generate/export a Varlock env key for local runs.

doctor.config.ts

Other (10) +352 / -379
.env.schemaMake Varlock environment explicit and scope encryption +4/-3

Make Varlock environment explicit and scope encryption

• Updates schema directives to use '@currentEnv=$VARLOCK_ENV' and the newer TS typegen directive. Limits '@encryptInjectedEnv' to production/preview so tests can use plain fixtures.

.env.schema

.env.testAdd committed VARLOCK_ENV=test fixtures +9/-0

Add committed VARLOCK_ENV=test fixtures

• Adds a '.env.test' file with static OAuth/auth/DB values so tests run without Bitwarden/network access.

.env.test

.gitignoreUnignore .env.test +1/-0

Unignore .env.test

• Keeps '.env*' ignored while explicitly allowing '.env.test' to be committed.

.gitignore

bun.lockRegenerate lockfile for dependency bumps +287/-337

Regenerate lockfile for dependency bumps

• Regenerates Bun lockfile (lockfileVersion 2) to reflect updated Next/Varlock/Panda/nuqs/Better Auth and tooling versions.

bun.lock

bunfig.tomlPreload Varlock test env shim and relax release-age for Next +6/-2

Preload Varlock test env shim and relax release-age for Next

• Adds 'tests/setup/varlock-test-env.ts' ahead of Varlock auto-load in Bun test preloads and excludes Next/SWC packages from 'minimumReleaseAge' gating.

bunfig.toml

next.config.tsDrop appNewScrollHandler experimental flag +0/-1

Drop appNewScrollHandler experimental flag

• Removes the 'experimental.appNewScrollHandler' config option.

next.config.ts

package.jsonBump deps and ensure tests run with VARLOCK_ENV=test +29/-29

Bump deps and ensure tests run with VARLOCK_ENV=test

• Updates dependency versions (Next canary, Varlock, Panda, nuqs, Better Auth, Biome/Ultracite, etc.). Prefixes all test scripts with 'VARLOCK_ENV=test' and adjusts the dev script to run Next via Bun.

package.json

panda.config.tsKeep keyframes during optimization +1/-1

Keep keyframes during optimization

• Disables 'removeUnusedKeyframes' to prevent animation keyframes from being stripped after the styling changes.

panda.config.ts

env.d.tsRegenerate env typings to include VARLOCK_ENV +13/-6

Regenerate env typings to include VARLOCK_ENV

• Updates generated Varlock env typings to include 'VARLOCK_ENV' and refreshes generated type hash names.

src/env.d.ts

varlock-test-env.tsEnsure VARLOCK_ENV=test before Varlock auto-load +2/-0

Ensure VARLOCK_ENV=test before Varlock auto-load

• Preload shim sets 'process.env.VARLOCK_ENV' to 'test' so Varlock resolves '.env.test' deterministically in Bun tests.

tests/setup/varlock-test-env.ts

@qodo-code-review

qodo-code-review Bot commented Aug 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Hardcoded offline menu status ⊘ Outdated 🐞 Bug ≡ Correctness
Description
The mobile popover header always renders the text “Offline” with a “brand” dot, regardless of actual
connectivity state. This will misrepresent online users as offline because the status is not
connected to useOffline() (unlike OfflineBanner).
Code

src/components/navbar/navbar.tsx[R392-395]

+          <span aria-hidden="true" className={mobilePopoverStatusClassName}>
+            <span className={mobilePopoverStatusDotClassName} />
+            Offline
+          </span>
Evidence
The popover header explicitly renders “Offline” unconditionally, while the actual connectivity-aware
component (OfflineBanner) uses useOffline() and hides itself when online, showing that the menu
header is not wired to real state.

src/components/navbar/navbar.tsx[390-395]
src/components/offline-indicator.tsx[54-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The mobile popover header currently renders a static status indicator that always says “Offline”. This is incorrect when the user is online and contradicts the existence of `OfflineBanner`, which correctly derives state from `useOffline()`.

### Issue Context
- `OfflineBanner` uses `useOffline()` and returns `null` when online.
- The mobile popover header is in `Navbar` (a server component), so it cannot directly call `useOffline()`.

### Fix Focus Areas
- src/components/navbar/navbar.tsx[385-396]
- src/components/offline-indicator.tsx[54-76]

### Suggested approach
- Create a small client component (e.g. `MobileConnectivityStatus`) that uses `useOffline()` and renders either:
 - Offline (with pulse/dot), or
 - Online (solid green dot),
 or render nothing when online if that’s the intended UX.
- Replace the hard-coded `Offline` span in the popover header with this client component.

This keeps `Navbar` server-rendered while ensuring correct, dynamic connectivity status.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Popover stays open on nav 🐞 Bug ≡ Correctness
Description
The new mobile popover menu has no explicit close behavior when a menu item is clicked, so its
open/closed state is left entirely to browser defaults. Because the Navbar is mounted via a
persistent App Router slot, navigating with these links can leave the popover open over the next
page until the user manually dismisses it.
Code

src/components/navbar/navbar.tsx[R400-404]

+              <NavLink
+                borderPosition="left"
+                className={mobileNavigationLinkClassName}
+                href="/3d-models"
+              >
Evidence
The PR adds a persistent mobile popover (popover="auto") containing NavLinks but no close
behavior; the app layout renders the Navbar via the {navbar} slot, so the popover can remain
mounted across route transitions.

src/components/navbar/navbar.tsx[385-431]
src/app/layout.tsx[77-87]
src/app/@navbar/default.tsx[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The mobile navigation popover (`popover="auto"`) is opened via `popoverTarget`, but none of the internal navigation actions explicitly close it. In an App Router layout where the Navbar persists across navigations, this can leave the popover open after clicking a menu link.

### Issue Context
- Navbar is rendered via a parallel route slot (`{navbar}`) and is therefore typically persistent across navigations.
- The popover links are `NavLink` (client) components, but the server `Navbar` component cannot pass event handlers directly.

### Fix Focus Areas
- src/components/navbar/navbar.tsx[385-431]
- src/components/nav-link/nav-link.tsx[34-95]

### Suggested approach
1. Add a serializable boolean prop to `NavLink` (e.g. `closePopoverOnClick?: boolean`).
2. Inside `NavLinkInner` (client), wrap/compose the click handler:
  - call any existing `onClick`
  - when `closePopoverOnClick` is true, find the nearest ancestor popover (e.g. `e.currentTarget.closest('[popover]')`) and call `hidePopover()` when available.
3. Set `closePopoverOnClick` on the mobile popover `NavLink`s in `Navbar`.

This avoids passing functions from a server component while ensuring deterministic popover closure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 8 rules

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/components/navbar/navbar.tsx Outdated
Comment on lines +400 to +404
<NavLink
borderPosition="left"
className={mobileNavigationLinkClassName}
href="/3d-models"
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Popover stays open on nav 🐞 Bug ≡ Correctness

The new mobile popover menu has no explicit close behavior when a menu item is clicked, so its
open/closed state is left entirely to browser defaults. Because the Navbar is mounted via a
persistent App Router slot, navigating with these links can leave the popover open over the next
page until the user manually dismisses it.
Agent Prompt
### Issue description
The mobile navigation popover (`popover="auto"`) is opened via `popoverTarget`, but none of the internal navigation actions explicitly close it. In an App Router layout where the Navbar persists across navigations, this can leave the popover open after clicking a menu link.

### Issue Context
- Navbar is rendered via a parallel route slot (`{navbar}`) and is therefore typically persistent across navigations.
- The popover links are `NavLink` (client) components, but the server `Navbar` component cannot pass event handlers directly.

### Fix Focus Areas
- src/components/navbar/navbar.tsx[385-431]
- src/components/nav-link/nav-link.tsx[34-95]

### Suggested approach
1. Add a serializable boolean prop to `NavLink` (e.g. `closePopoverOnClick?: boolean`).
2. Inside `NavLinkInner` (client), wrap/compose the click handler:
   - call any existing `onClick`
   - when `closePopoverOnClick` is true, find the nearest ancestor popover (e.g. `e.currentTarget.closest('[popover]')`) and call `hidePopover()` when available.
3. Set `closePopoverOnClick` on the mobile popover `NavLink`s in `Navbar`.

This avoids passing functions from a server component while ensuring deterministic popover closure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/components/navbar/navbar.tsx Outdated
Coinnich and others added 2 commits August 24, 2026 17:30
Extract desktop/mobile nav, auth slots, and shell styles from monolithic navbar.
Dismiss mobile popover on nav link click via hidePopover(). Move offline UI
into src/components/offline/ with mobile connectivity status in popover.

Closes #71

Co-authored-by: Cursor <cursoragent@cursor.com>
Pin Bun 1.4 on Vercel for lockfileVersion 2 installs. Collapse offline UI
back to offline-indicator with container-responsive detail. Logo wordmark
from xs breakpoint; drop popover mobile connectivity chip. Add xs/xxs Panda
breakpoints and ignore .kombai/canvas.

Closes #72

Co-authored-by: Cursor <cursoragent@cursor.com>
@fringe4life

Copy link
Copy Markdown
Owner Author

Qodo it does close via the NavLink, which closes the closest popover? The sign out is left uncovered as it has pending state as it's async by nature

@qodo-code-review

Copy link
Copy Markdown

Qodo it does close via the NavLink, which closes the closest popover? The sign out is left uncovered as it has pending state as it's async by nature

You're right: MobileNavLink passes closePopoverOnClick, and NavLink calls hidePopover() on the closest ancestor popover. That covers the About/3D Models/sign-in navigation links and matches the added test.

The sign-out action is different: it is an async AuthButtons action, not a NavLink, so it does not receive that close behavior while pending. If the intended requirement includes closing after sign-out, the auth action would need to explicitly hide the containing popover (or the popover should close when the action starts). Otherwise, finding 2's concern about navigation links is addressed, though the finding itself remains active until explicitly resolved.

@fringe4life
fringe4life merged commit 5edcc8c into main Aug 24, 2026
2 checks passed
@fringe4life
fringe4life deleted the feat/mobile-nav-offline-varlock-tests branch August 24, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant