Skip to content

CANON-041: fix nav-sheet max-height 60vh → 60dvh on mobile#54

Open
VaultSparkStudios wants to merge 1 commit into
mainfrom
routine/canon-041-dvh-navsheet
Open

CANON-041: fix nav-sheet max-height 60vh → 60dvh on mobile#54
VaultSparkStudios wants to merge 1 commit into
mainfrom
routine/canon-041-dvh-navsheet

Conversation

@VaultSparkStudios

Copy link
Copy Markdown
Owner

What

The nav-sheet bottom-sheet (active for ~50% of mobile visitors via the canary ramp) injected max-height:60vh. On iOS Safari vh is calculated from the large viewport (URL bar retracted), so when the address bar is visible the sheet can overflow behind it — violating CANON-041's explicit 100dvh (NOT 100vh) rule.

Files changed:

  • assets/nav-sheet.js — source
  • assets/nav-sheet.shell-e821c7fa64.js — shell copy served by all pages that reference it

Change: max-height:60vh;max-height:60dvh; — the dvh declaration follows the vh fallback so browsers without dvh support (pre-iOS 15.4, pre-Chrome 108) fall back gracefully while modern browsers use the correct dynamic viewport height.

Why dvh matters here

60vh on a bottom sheet that anchors to bottom:0 means on a phone with the address bar visible, the sheet can extend 60% of the large viewport — partially obscuring the drag-handle or top items behind the address bar. 60dvh tracks the current visible height, so the sheet always fits within what the user can actually see.

To verify on mobile

  1. Open any page on ?nav=sheet (or clear vs-nav-style from localStorage and reload until the canary activates)
  2. Open the nav sheet with the hamburger
  3. On iOS Safari with the address bar visible — confirm the sheet fits within the visible area and does not extend under the browser chrome
  4. Drag the handle downward >80px — confirm it closes cleanly
  5. Confirm desktop layout is completely unaffected (sheet is hidden at ≥981px)

Note: This PR pairs with PR #12 (routine/canon-041-dvh-palette) which fixes the same vhdvh issue in the command palette. Both can be merged independently.


Generated by Claude Code

The nav-sheet (active for ~50% of mobile users via canary) injected
max-height:60vh. On iOS Safari 60vh is calculated from the large
viewport (URL bar hidden), so when the address bar is visible the sheet
can extend behind it. Adding max-height:60dvh after the 60vh declaration
lets supporting browsers (iOS 15.4+, Chrome 108+) use the true dynamic
viewport height while older engines fall back gracefully.

Same one-line fix applied to both the source file and its shell copy
(nav-sheet.shell-e821c7fa64.js) so both serve identical styles.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1968552212

ℹ️ 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 1.4rem;color:var(--text);display:flex;flex-direction:column;box-shadow:0 -18px 60px rgba(0,0,0,0.6)}' +

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Regenerate the shell asset fingerprint

Because this changes a fingerprinted shell asset in place, the file name and assets/shell-manifest.json still advertise e821c7fa64 while the actual content now hashes to bfa740b8d6. I checked node scripts/build-shell-assets.mjs --check, and it fails with shell asset drift across the manifest, sw.js, and every HTML reference; rerun the shell asset build so the hashed file, manifest, service-worker cache name, and page references rotate together.

Useful? React with 👍 / 👎.

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.

1 participant