diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 6bf958b8..85af010f 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -248,6 +248,7 @@ jobs: BASE_URL=http://localhost:13581 node test-observer-iata-1188-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 CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-priority-1102-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-priority-1311-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-more-floor-1139-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-bottom-nav-1061-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-gestures-1062-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/app.js b/public/app.js index e3d2ea46..233c20d9 100644 --- a/public/app.js +++ b/public/app.js @@ -1214,7 +1214,20 @@ window.addEventListener('DOMContentLoaded', () => { return needed <= window.innerWidth; } let i = 0; + // #1311 floor: protect data-priority="high" links from being + // dropped by the greedy fit loop. The bug was that on a non-high + // active route (e.g. /#/perf, /#/audio-lab) at ~1101-1200px, the + // active-route pill renders wider than other links, fits() keeps + // returning false even after every non-high link is in overflow, + // and the loop happily walked into the high-priority tail of + // overflowQueue and dropped Home/Packets/Map/Live/Nodes too — + // leaving the user with just brand + "More ▾" + the active pill. + // High-priority links are inline-pinned by contract; if the strip + // still doesn't fit at that point, that's a layout issue (e.g. + // shrink the active pill, drop nav-stats earlier) — never the + // measurer's call to delete primary navigation. while (!fits() && i < overflowQueue.length) { + if (overflowQueue[i].dataset.priority === 'high') break; overflowQueue[i].classList.add('is-overflow'); i++; } @@ -1228,7 +1241,12 @@ window.addEventListener('DOMContentLoaded', () => { // which is the correct UX) and skip when the queue is exhausted. var overflowedCount = allLinks.filter(a => a.classList.contains('is-overflow')).length; if (overflowedCount === 1) { - if (i < overflowQueue.length) { + // #1311: respect the high-priority floor here too — if the only + // remaining queue item is a high-priority link, do NOT promote + // it just to satisfy the >=2 More-menu floor. A degenerate + // 1-item dropdown is a smaller UX paper-cut than nuking a + // primary nav link. + if (i < overflowQueue.length && overflowQueue[i].dataset.priority !== 'high') { overflowQueue[i].classList.add('is-overflow'); i++; } else { diff --git a/test-nav-priority-1311-e2e.js b/test-nav-priority-1311-e2e.js new file mode 100644 index 00000000..763880ef --- /dev/null +++ b/test-nav-priority-1311-e2e.js @@ -0,0 +1,136 @@ +#!/usr/bin/env node +/* Issue #1311 — Active non-high-priority route causes ALL high-priority + * links to overflow into the More menu at narrow-desktop widths. + * + * Bug: applyNavPriority's iterative overflow loop in public/app.js had no + * floor — when fits() kept returning false (because the active-route pill + * is wider than other links), it walked off the end of the non-high tail + * and started dropping high-priority links too. On /#/perf at ~900-1100px + * in Firefox/Edge Windows, this nuked ALL 5 high-priority links from the + * inline strip, leaving only "More ▾" + the active pill visible. + * + * Acceptance: + * - At 900px / 1024px / 1100px width on /#/perf, /#/audio-lab, + * /#/analytics, /#/observers (active route is non-high-priority): + * ALL 5 high-priority links (#/home, #/packets, #/map, #/live, #/nodes) + * are visible inline (.is-overflow not present, getBoundingClientRect + * width > 0). The More menu may contain low-priority links — that's + * fine — but never high-priority links. + * + * Mutation guard: removing the `priority === 'high'` floor in the loop + * makes this test fail (loop walks into the high-priority tail). + */ +'use strict'; + +const assert = require('node:assert'); +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const HIGH_PRIORITY_HREFS = ['#/home', '#/packets', '#/map', '#/live', '#/nodes']; + +// Active routes that are NOT high-priority. The bug surfaced when the +// active-route pill (which has wider padding/background) is one of these. +const NON_HIGH_ROUTES = ['#/perf', '#/audio-lab', '#/analytics', '#/observers']; + +// Widths in the narrow-desktop band where the bug reproduces. The 768-1100 +// branch in applyNavPriority is data-priority-only (already correct), so +// the regression actually surfaces just above 1100 where the iterative +// loop runs. We include 1101/1200 to cover that band, and 900/1024 for +// belt-and-braces (the CSS branch). +const WIDTHS = [900, 1024, 1101, 1200]; +const HEIGHT = 900; + +async function main() { + 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 (process.env.CHROMIUM_REQUIRE === '1') { + console.error(`test-nav-priority-1311-e2e.js: FAIL — Chromium required but unavailable: ${err.message}`); + process.exit(1); + } + console.log(`test-nav-priority-1311-e2e.js: SKIP (Chromium unavailable: ${err.message.split('\n')[0]})`); + process.exit(0); + } + + let failures = 0; + let passes = 0; + const ctx = await browser.newContext(); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + + for (const w of WIDTHS) { + for (const route of NON_HIGH_ROUTES) { + await page.setViewportSize({ width: w, height: HEIGHT }); + await page.goto(`${BASE}/${route}`, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('.top-nav .nav-links'); + await page.evaluate(() => document.fonts && document.fonts.ready ? document.fonts.ready : null); + // Settle layout (two consecutive frames identical for nav-right). + 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); + })); + }); + }, null, { timeout: 5000 }); + // Give applyNavPriority (wired to hashchange + rAF resize) a frame. + await page.evaluate(() => new Promise(r => requestAnimationFrame(() => requestAnimationFrame(r)))); + + const data = await page.evaluate((HIGH) => { + const links = Array.from(document.querySelectorAll('.nav-links .nav-link')); + const overflowedHighPri = []; + const visibleHighPri = []; + for (const a of links) { + const href = a.getAttribute('href'); + if (!HIGH.includes(href)) continue; + const isOverflow = a.classList.contains('is-overflow'); + const width = a.getBoundingClientRect().width; + if (isOverflow || width === 0) { + overflowedHighPri.push({ href, isOverflow, width }); + } else { + visibleHighPri.push(href); + } + } + return { overflowedHighPri, visibleHighPri }; + }, HIGH_PRIORITY_HREFS); + + const tag = `${w}px @ ${route}`; + // Hard assertion (preflight gate requires `assert` call): every + // high-priority link href is in the visible set, never overflowed. + try { + assert.deepStrictEqual( + [...data.visibleHighPri].sort(), + [...HIGH_PRIORITY_HREFS].sort(), + `${tag}: expected all 5 high-priority links inline, got [${data.visibleHighPri.join(', ')}]` + ); + passes++; + console.log(` ✅ ${tag}: 5/5 high-pri visible inline`); + } catch (e) { + failures++; + const detail = data.overflowedHighPri + .map(o => `${o.href}(overflow=${o.isOverflow},w=${o.width})`) + .join(', '); + console.log(` ❌ ${tag}: high-pri overflowed/hidden: ${detail || '(none)'} ` + + `| visible=[${data.visibleHighPri.join(', ')}]`); + } + } + } + + await browser.close(); + const total = WIDTHS.length * NON_HIGH_ROUTES.length; + console.log(`\ntest-nav-priority-1311-e2e.js: ${failures === 0 ? 'OK' : 'FAIL'} — ${passes}/${total} passed`); + process.exit(failures === 0 ? 0 : 1); +} + +main().catch((err) => { + console.error('test-nav-priority-1311-e2e.js: fatal', err); + process.exit(1); +});