CANON-041: fix nav-sheet mobile dvh, scroll-lock, and safe-area-inset#43
CANON-041: fix nav-sheet mobile dvh, scroll-lock, and safe-area-inset#43VaultSparkStudios wants to merge 1 commit into
Conversation
Three CANON-041.2 violations in the bottom-sheet nav (active for 50% of mobile users via canary ramp): 1. max-height:60vh → 60vh/60dvh (progressive) — on iOS Safari 60vh resolves against the large retracted viewport; 60dvh uses the true visual viewport. 2. Add lockScroll()/unlockScroll() with the iOS-safe position:fixed pattern (matching nav-toggle.js) — openSheet() was leaving body scroll free behind the backdrop, closeSheet() had no unlock path at all. 3. bottom padding: 1.4rem → calc(1.4rem + env(safe-area-inset-bottom,0px)) — home-indicator-model iPhones were clipping the last nav links under the gesture bar. Both nav-sheet.js (source) and nav-sheet.shell-e4e11cde5c.js (deployed asset referenced by all HTML pages) updated identically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTTM53QZATCgD7gnsvBhnP
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3f2126f81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '.vs-nav-sheet-backdrop{position:fixed;inset:0;background:rgba(0,0,0,0.6);z-index:2147483640;opacity:0;pointer-events:none;transition:opacity 240ms ease}' + | ||
| '.vs-nav-sheet-backdrop.open{opacity:1;pointer-events:auto}' + | ||
| '.vs-nav-sheet{position:fixed;left:0;right:0;bottom:0;max-height:60vh;background:var(--mobile-nav-bg,rgba(5,6,14,0.99));border-top:1px solid var(--mobile-nav-border,rgba(255,255,255,0.06));border-radius:18px 18px 0 0;z-index:2147483641;transform:translateY(100%);transition:transform 280ms cubic-bezier(.4,0,.2,1);overflow:auto;padding:0.6rem 1.1rem 1.4rem;color:var(--text);display:flex;flex-direction:column;box-shadow:0 -18px 60px rgba(0,0,0,0.6)}' + | ||
| '.vs-nav-sheet{position:fixed;left:0;right:0;bottom:0;max-height:60vh;max-height:60dvh;background:var(--mobile-nav-bg,rgba(5,6,14,0.99));border-top:1px solid var(--mobile-nav-border,rgba(255,255,255,0.06));border-radius:18px 18px 0 0;z-index:2147483641;transform:translateY(100%);transition:transform 280ms cubic-bezier(.4,0,.2,1);overflow:auto;padding:0.6rem 1.1rem calc(1.4rem + env(safe-area-inset-bottom,0px));color:var(--text);display:flex;flex-direction:column;box-shadow:0 -18px 60px rgba(0,0,0,0.6)}' + |
There was a problem hiding this comment.
Regenerate the fingerprinted nav-sheet asset
Changing assets/nav-sheet.js changes the content hash that scripts/build-shell-assets.mjs uses for the shell filename, but this commit keeps the old nav-sheet.shell-e4e11cde5c.js path and leaves the manifest, service worker, and HTML references stale. I checked node scripts/build-shell-assets.mjs --check, and it now reports drift expecting assets/nav-sheet.shell-e421917c24.js plus updated references, so the build gate will fail and cached visitors can remain on the old sheet code.
Useful? React with 👍 / 👎.
| function openSheet() { | ||
| if (open) return; | ||
| open = true; | ||
| lockScroll(); |
There was a problem hiding this comment.
Unlock the sheet before same-page navigation
With the new body scroll lock, sheet links that do not unload the document stay stuck behind position:fixed; for example, on /vault-member/, tapping the sheet’s /vault-member/#login or #register actions changes only the hash and no handler calls closeSheet(), so the sheet remains open and closing it later restores savedScrollY instead of the target section. Close/unlock the sheet on same-document link clicks before allowing the anchor navigation.
Useful? React with 👍 / 👎.
What
Three CANON-041.2 violations fixed in
nav-sheet.js/nav-sheet.shell-e4e11cde5c.js— the bottom-sheet nav active for ~50% of mobile users (canary ramp).Fix 1 —
60vh→60dvh(progressive enhancement)On iPhone with the address bar visible,
60vhresolves against the large (retracted) viewport, so the sheet extends into obscured screen space.60dvhalways tracks the actual visible area.Fix 2 — Body scroll-lock: add
lockScroll()/unlockScroll()openSheet()applied no scroll lock — page content scrolled freely behind the open backdrop.closeSheet()had no release path at all.Added the same iOS-safe
position:fixedpattern already used innav-toggle.js: savescrollY, setbody { position:fixed; top: -<savedY>px; ... }on open, restore andscrollToon close.Fix 3 —
env(safe-area-inset-bottom)in sheet paddingOn iPhone (home-indicator models), the last nav links were clipped under the gesture bar. The main drawer already used this pattern correctly (
style.cssline 1924).Files changed
assets/nav-sheet.js— sourceassets/nav-sheet.shell-e4e11cde5c.js— deployed asset (all HTML pages reference this hash)How to verify on mobile
?nav=sheetto force the sheet cohort, or wait for canary assignment).Remaining CANON-041 issues (out of scope for this PR)
command-palette.js:96height:100vhon mobile palette →100dvhinvestor-theme.css:22min-height:100vhon.inv-page→100dvhinvestor-admin.js:408min-height:100vhon error screen →100dvhstyle.css:837style.css:2401.pwa-nav-btn~30px tall (PWA standalone mode)style.css:717max-height:min(80vh,540px)nav dropdown →dvhstyle.css:3588max-height:min(72vh,680px)feedback widget →dvhGenerated by Claude Code