From d1e6c733dce2a0ce93a410d89440421f3be08668 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Tue, 5 May 2026 08:19:51 -0700 Subject: [PATCH] fix(nav): apply Priority+ at all widths (#1055) (#1097) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Make the top-nav use the **Priority+ pattern at all widths** (not just 768–1279px), so the nav-right cluster never gets pushed off-screen or visually overlapped by the link strip. Fixes #1055. ## What changed **`public/style.css`** — nav section only (clearly fenced): - Removed the upper bound on the Priority+ media query (`max-width: 1279px`). The rule now applies at any viewport `>= 768px`. Above that breakpoint, only `data-priority="high"` links render inline; the rest collapse into the existing `More ▾` overflow menu. - Swapped nav-only hardcoded spacing/type to the fluid `clamp()` tokens shipped in #1054: - `.top-nav` padding → `var(--gutter)` - `.nav-left` gap → `var(--space-lg)` - `.nav-brand` gap → `var(--space-sm)`, font-size → `var(--fs-md)` - `.nav-links` gap → `var(--space-xs)` - `.nav-link` padding → `clamp(8px, 0.6vw + 4px, 14px)`, font-size → `var(--fs-sm)` - `.nav-right` gap → `var(--space-sm)` - Mobile (<768px) hamburger layout, the More-menu markup, and the JS that builds the menu in `public/app.js` are unchanged — they already supported this pattern. `public/index.html` did not need changes — the `data-priority="high"` markup, `nav-more-wrap`, `navMoreBtn`, and `navMoreMenu` are already in place from earlier work. ## Why the bug existed The previous Priority+ rule was scoped `@media (min-width: 768px) and (max-width: 1279px)`. From 1280px–~1599px the full 11-link strip rendered but didn't fit alongside `.nav-stats` + `.nav-right`. The parent `overflow: hidden` masked the symptom, but the rightmost links physically rendered underneath `.nav-right` and were unreachable. ## E2E assertion added New `test-nav-fluid-1055-e2e.js` — Playwright multi-viewport test (768/1024/1280/1440/1920) that asserts: 1. `.nav-right.right` ≤ `document.documentElement.clientWidth` (no horizontal overflow) 2. Last visible `.nav-link.right` ≤ `.nav-right.left` (no overlap underneath the right cluster) 3. `.top-nav.scrollWidth` ≤ `.top-nav.clientWidth` (no scrolled-off content) Wired into the `e2e-test` job in `.github/workflows/deploy.yml`. **TDD evidence:** - Red commit `466221a`: test passes 3/5 (1024/768/1920) — fails at 1280 (253px overlap) and 1440 (93px overlap). - Green commit `1aa939a`: test passes 5/5. ## Acceptance criteria (from #1055) - [x] Priority+ at ALL widths (not just mobile). - [x] No nav link overflow at 1080px (or any tested width). - [x] Overflow menu accessible via keyboard + touch (existing `navMoreBtn` aria-haspopup wiring; verified by existing app.js handlers). - [x] Active route still highlighted when in overflow (existing logic in `app.js` adds `.active` to the cloned link in `navMoreMenu`). - [x] Tested at 768/1024/1280/1440/1920 — visible link count adapts (5 priority links + More menu at all desktop widths; full 11 inline only on hamburger mobile when expanded). --------- Co-authored-by: bot Co-authored-by: clawbot Co-authored-by: meshcore-bot --- .github/workflows/deploy.yml | 1 + public/style.css | 59 +++++++++---- test-nav-fluid-1055-e2e.js | 156 +++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 16 deletions(-) create mode 100644 test-nav-fluid-1055-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 4d0aa0eb..c633333b 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -216,6 +216,7 @@ jobs: BASE_URL=http://localhost:13581 node test-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-channel-issue-1087-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-map-modal-fluid-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-fluid-1055-e2e.js 2>&1 | tee -a e2e-output.txt - name: Collect frontend coverage (parallel) if: success() && github.event_name == 'push' diff --git a/public/style.css b/public/style.css index 3a332bcf..8cb9d3af 100644 --- a/public/style.css +++ b/public/style.css @@ -370,16 +370,24 @@ input[type="week"] { .sort-help:focus .sort-help-tip, .sort-help:focus-within .sort-help-tip { display: block; } -/* === Nav === */ +/* === Nav (Issue #1055 — Priority+ at all widths) === + The top-nav uses Priority+ at ALL widths: only links marked + data-priority="high" render inline; everything else collapses into + the "More ▾" menu. This eliminates the historic 1080–1279px overflow + window AND the 1280–1599px overlap window where the full link strip + ran underneath .nav-right. Spacing and type use the #1054 fluid + clamp() tokens so the bar scales smoothly across viewport widths. + The hamburger and stacked mobile layout still take over at <768px. */ .top-nav { display: flex; align-items: center; justify-content: space-between; - background: linear-gradient(135deg, var(--nav-bg) 0%, var(--nav-bg2) 100%); color: var(--nav-text); padding: 0 20px; height: 52px; + background: linear-gradient(135deg, var(--nav-bg) 0%, var(--nav-bg2) 100%); color: var(--nav-text); + padding: 0 var(--gutter); height: 52px; position: sticky; top: 0; z-index: 1100; box-shadow: 0 2px 8px rgba(0,0,0,.3); flex-wrap: nowrap; overflow: hidden; min-width: 0; } -.nav-left { display: flex; align-items: center; gap: 24px; min-width: 0; flex-shrink: 1; overflow: hidden; } -.nav-brand { display: flex; align-items: center; gap: 8px; text-decoration: none; color: var(--nav-text); font-weight: 700; font-size: 16px; } +.nav-left { display: flex; align-items: center; gap: var(--space-lg); min-width: 0; flex-shrink: 1; overflow: hidden; } +.nav-brand { display: flex; align-items: center; gap: var(--space-sm); text-decoration: none; color: var(--nav-text); font-weight: 700; font-size: var(--fs-md); } .brand-icon { font-size: 20px; } .live-dot { width: 8px; height: 8px; border-radius: 50%; background: var(--text-muted); @@ -392,9 +400,10 @@ input[type="week"] { } .live-dot.connected { background: var(--status-green); animation: pulse-ring 2s ease-out infinite; } -.nav-links { display: flex; align-items: center; gap: 4px; } +.nav-links { display: flex; align-items: center; gap: var(--space-xs); } .nav-link { - color: var(--nav-text-muted); text-decoration: none; padding: 14px 12px; font-size: 14px; + color: var(--nav-text-muted); text-decoration: none; + padding: 14px clamp(8px, 0.6vw + 4px, 14px); font-size: var(--fs-sm); border-bottom: 2px solid transparent; transition: all .15s; background: none; border-top: none; border-left: none; border-right: none; cursor: pointer; font-family: var(--font); @@ -407,7 +416,7 @@ input[type="week"] { background: var(--nav-active-bg); border-radius: 6px; margin: 4px 0; - padding: 10px 12px; + padding: 10px clamp(8px, 0.6vw + 4px, 14px); } .nav-dropdown { position: relative; } @@ -422,7 +431,7 @@ input[type="week"] { } .dropdown-item:hover { background: var(--accent); color: #fff; } -.nav-right { display: flex; align-items: center; gap: 8px; flex-shrink: 0; } +.nav-right { display: flex; align-items: center; gap: var(--space-sm); flex-shrink: 0; } .nav-btn { background: none; border: 1px solid var(--border); color: var(--nav-text-muted); padding: 6px 12px; border-radius: 6px; cursor: pointer; font-size: 14px; transition: all .15s; @@ -1267,18 +1276,36 @@ button.ch-item:hover .ch-icon-btn { opacity: 1; } .map-controls { width: 180px; font-size: 12px; } } -/* === Responsive — Tablet/Mid Priority+ nav (768–1279px) === - At <1280px the full nav-links + nav-stats + nav-right buttons can't fit - on one row (they total ~1540px on the home page). Collapse non-priority - links into the "More ▾" menu so .nav-right stays inside the viewport. */ -@media (min-width: 768px) and (max-width: 1279px) { - .nav-links { display: flex !important; flex-direction: row; gap: 2px; } +/* === Nav Priority+ — applies at ALL desktop widths (Issue #1055) === + Above the mobile hamburger breakpoint, only data-priority="high" links + render inline; the rest collapse into the "More ▾" overflow menu. + This guarantees nav-right cannot be pushed off-screen or overlapped by + the link strip at any common viewport (768/1024/1280/1440/1920+). */ +@media (min-width: 768px) { + .nav-links { display: flex !important; flex-direction: row; gap: var(--space-xs); } .nav-links a:not([data-priority="high"]) { display: none; } .nav-more-wrap { display: flex; align-items: center; } .hamburger { display: none; } - .nav-link { padding: 14px 8px; font-size: 13px; } + .nav-link { padding: 14px clamp(8px, 0.6vw + 4px, 14px); font-size: var(--fs-sm); } .nav-links a[data-priority="high"] { order: -1; } - .nav-link.active { background: var(--nav-active-bg); border-radius: 6px; margin: 4px 0; padding: 10px 8px; } + .nav-link.active { background: var(--nav-active-bg); border-radius: 6px; margin: 4px 0; padding: 10px clamp(8px, 0.6vw + 4px, 14px); } +} + +/* === Nav narrow-desktop tightening (Issue #1055 — 1024px overlap fix) === + Between 768px and ~1100px, even with Priority+ collapsed to 5 high + links + "More ▾" + 4 nav-right buttons + nav-stats, the row doesn't + fit and the rightmost visible link overlaps nav-right by ~20px. + Hide nav-stats and tighten nav-link padding/gap in this band; + nav-stats reappears at ≥1101px where there is room for it. + NOTE: this block MUST follow the (min-width: 768px) Priority+ block + above so its tighter padding/gap rules win the cascade by source + order at 1024px (same specificity). */ +@media (min-width: 768px) and (max-width: 1100px) { + .nav-stats { display: none; } + .nav-links { gap: 0; } + .nav-link { padding: 14px 6px; } + .nav-link.active { padding: 10px 6px; } + .nav-right { gap: 4px; } } /* === Responsive — Hamburger nav (<768px) === */ diff --git a/test-nav-fluid-1055-e2e.js b/test-nav-fluid-1055-e2e.js new file mode 100644 index 00000000..64976715 --- /dev/null +++ b/test-nav-fluid-1055-e2e.js @@ -0,0 +1,156 @@ +#!/usr/bin/env node +/* Issue #1055 — Nav fluid Priority+ adaptation at all widths. + * + * Asserts the top-nav never overflows the viewport at common widths: + * the right edge of `.nav-right` MUST be ≤ document.documentElement.clientWidth. + * + * Pre-fix behavior: the Priority+ collapse rule was scoped to + * `(min-width: 768px) and (max-width: 1279px)`, so at 1280/1440/1920 the + * full link strip + nav-stats + nav-right buttons could push past the + * viewport's right edge (no collapse happened above 1279px). + * + * Post-fix: Priority+ collapses at all widths >=768px when needed. + * + * Run against a CoreScope server (defaults to localhost:13581 with the + * E2E fixture DB, matching the playwright job in .github/workflows/deploy.yml). + * + * CI gating: when CHROMIUM_REQUIRE=1 (set by the GH Actions workflow) a + * missing/broken Chromium is a HARD FAIL — no soft-skip. Locally the + * test is allowed to skip so devs without Playwright browsers installed + * can still run other tests. + */ +'use strict'; + +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +// Common widths the nav must stay clean at. 1280/1440 are the historic +// failure window: the Priority+ rule used to stop at 1279px but the full +// link strip + nav-right buttons don't fit on one row until ~1600px+. +const VIEWPORTS = [768, 1024, 1280, 1440, 1920]; +// Routes asserted at every viewport. The pre-#1097 version only checked +// /#/home, but the bug reproduces on every top-level page since they +// all share the same .top-nav. Cover the four primary routes. +const ROUTES = ['/#/home', '/#/packets', '/#/nodes', '/#/map']; +const HEIGHT = 900; +// Whitespace tolerance (px) for the overflow/overlap assertions. +// Browsers occasionally hand back layout coordinates with sub-pixel +// rounding noise (≈0.1–0.4px) even when the box model is clean. We +// allow up to 0.5px so the test doesn't false-fail on rounding while +// still catching real overlaps (the bug this guards against was +// ~20px). Tighter than 0.5 caused intermittent CI flakes; looser +// would risk masking 1px regressions. +const SUBPIXEL_TOL = 0.5; + +async function main() { + const requireChromium = process.env.CHROMIUM_REQUIRE === '1' || + process.env.NAV_FLUID_REQUIRE === '1'; + let browser; + try { + browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + } catch (err) { + if (requireChromium) { + console.error(`test-nav-fluid-1055-e2e.js: FAIL — Chromium required (CHROMIUM_REQUIRE=1) but unavailable: ${err.message}`); + process.exit(1); + } + console.log(`test-nav-fluid-1055-e2e.js: SKIP (Chromium unavailable: ${err.message.split('\n')[0]})`); + process.exit(0); + } + + let failures = 0; + let passes = 0; + const context = await browser.newContext(); + const page = await context.newPage(); + page.setDefaultTimeout(15000); + + for (const route of ROUTES) { + for (const w of VIEWPORTS) { + await page.setViewportSize({ width: w, height: HEIGHT }); + await page.goto(`${BASE}${route}`, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('.top-nav .nav-right'); + // Wait for fonts (which affect text measurement) AND for the nav + // layout to settle: the .nav-right bounding box must hold steady + // for two consecutive animation frames at the same coordinates. + // This replaces a magic 150ms sleep with a deterministic gate + // that asserts what we actually care about (layout has stopped + // moving) before measuring. + await page.evaluate(() => document.fonts && document.fonts.ready ? document.fonts.ready : null); + await page.waitForFunction(() => { + const el = document.querySelector('.top-nav .nav-right'); + if (!el) return false; + const r1 = el.getBoundingClientRect(); + return new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => { + const r2 = el.getBoundingClientRect(); + resolve(r1.right === r2.right && r1.left === r2.left && r1.top === r2.top); + })); + }); + }, null, { timeout: 5000 }); + + const data = await page.evaluate(() => { + const navRight = document.querySelector('.top-nav .nav-right'); + const navLeft = document.querySelector('.top-nav .nav-left'); + const topNav = document.querySelector('.top-nav'); + const more = document.querySelector('.nav-more-wrap'); + const moreCs = more ? getComputedStyle(more) : null; + const links = Array.from(document.querySelectorAll('.nav-links .nav-link')); + const visible = links.filter(a => getComputedStyle(a).display !== 'none'); + const lastVisible = visible[visible.length - 1] || null; + return { + clientW: document.documentElement.clientWidth, + navScroll: topNav.scrollWidth, + navClient: topNav.clientWidth, + navRight: navRight.getBoundingClientRect().right, + navRightL: navRight.getBoundingClientRect().left, + navLeftR: navLeft.getBoundingClientRect().right, + lastLinkR: lastVisible ? lastVisible.getBoundingClientRect().right : -1, + moreVisible: moreCs ? moreCs.display !== 'none' : false, + visibleLinks: visible.length, + totalLinks: links.length, + }; + }); + + const tag = `${route} @ ${w}px`; + const reasons = []; + // 1. .nav-right must not extend past the viewport's right edge. + if (data.navRight > data.clientW + SUBPIXEL_TOL) { + reasons.push(`nav-right.right=${data.navRight.toFixed(1)} > clientWidth=${data.clientW} ` + + `(excess ${(data.navRight - data.clientW).toFixed(1)}px)`); + } + // 2. The visible link strip must not overlap .nav-right (parent overflow:hidden + // masks this visually but it still hides the rightmost links — the actual bug). + if (data.lastLinkR > data.navRightL + SUBPIXEL_TOL) { + reasons.push(`last visible link right=${data.lastLinkR.toFixed(1)} > nav-right.left=${data.navRightL.toFixed(1)} ` + + `(${(data.lastLinkR - data.navRightL).toFixed(1)}px overlap)`); + } + // 3. The nav row itself must not require horizontal scrolling. + if (data.navScroll > data.navClient + SUBPIXEL_TOL) { + reasons.push(`top-nav scrollWidth=${data.navScroll} > clientWidth=${data.navClient}`); + } + + if (reasons.length === 0) { + passes++; + console.log(` ✅ ${tag}: clean (visible links ${data.visibleLinks}/${data.totalLinks}, more=${data.moreVisible})`); + } else { + failures++; + console.log(` ❌ ${tag}: ${reasons.join(' | ')} ` + + `(visible links ${data.visibleLinks}/${data.totalLinks}, more=${data.moreVisible})`); + } + } + } + + await browser.close(); + + const total = ROUTES.length * VIEWPORTS.length; + console.log(`\ntest-nav-fluid-1055-e2e.js: ${failures === 0 ? 'OK' : 'FAIL'} — ${passes}/${total} passed`); + process.exit(failures === 0 ? 0 : 1); +} + +main().catch((err) => { + console.error('test-nav-fluid-1055-e2e.js: fatal', err); + process.exit(1); +});