diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index dbdb91fe..85664485 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -265,6 +265,7 @@ jobs: 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-priority-1391-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 04904a80..bb264625 100644 --- a/public/app.js +++ b/public/app.js @@ -1133,9 +1133,23 @@ window.addEventListener('DOMContentLoaded', () => { // only signal — if you ever need finer ordering, switch to a numeric // attribute (e.g. data-overflow-order="3") rather than re-shuffling // index in HTML. - const overflowQueue = allLinks.filter(a => a.dataset.priority !== 'high') - .reverse() // right-to-left - .concat(allLinks.filter(a => a.dataset.priority === 'high').reverse()); + // #1391: ALSO exclude the currently-active link from the queue. + // The active pill has wider rendered width (background + padding), + // and acceptance for #1391 requires "Active-route pill MUST always + // be visible inline (never overflowed to More) at any viewport + // ≥768px." The queue is rebuilt on hashchange (applyNavPriority + // is wired to hashchange below), so the exclusion tracks the + // current route automatically. + function buildOverflowQueue() { + var isPinned = function(a) { + return a.dataset.priority === 'high' || a.classList.contains('active'); + }; + return allLinks.filter(a => !isPinned(a)) + .reverse() // right-to-left + .concat(allLinks.filter(a => a.dataset.priority === 'high' && !a.classList.contains('active')).reverse()); + } + var overflowQueue = buildOverflowQueue(); + function rebuildMoreMenu() { navMoreMenu.innerHTML = ''; @@ -1194,7 +1208,14 @@ window.addEventListener('DOMContentLoaded', () => { // owns the decision (and at 2560px nothing overflows). if (window.innerWidth <= 1100) { allLinks.forEach(a => { - if (a.dataset.priority !== 'high') a.classList.add('is-overflow'); + // #1391: never overflow the active-route pill, even in the + // narrow-desktop CSS branch — acceptance requires it stay + // inline at any viewport ≥768px. Without this guard, a + // non-high-priority active route (e.g. /#/perf) would be + // shoved into More alongside the rest. + if (a.dataset.priority !== 'high' && !a.classList.contains('active')) { + a.classList.add('is-overflow'); + } }); rebuildMoreMenu(); return; @@ -1251,6 +1272,11 @@ window.addEventListener('DOMContentLoaded', () => { return needed <= window.innerWidth; } let i = 0; + // #1391: rebuild queue here so it reflects the CURRENT active + // link (hashchange wakes applyNavPriority, but the queue was + // captured at init-time; we need to re-evaluate which link is + // active on every run). Cheap — just filters allLinks twice. + overflowQueue = buildOverflowQueue(); // #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 @@ -1263,8 +1289,13 @@ window.addEventListener('DOMContentLoaded', () => { // 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. + // + // #1391: also break on .active — buildOverflowQueue already + // excludes the active link from the queue, but the break is a + // defensive belt for any future code that re-enqueues it. while (!fits() && i < overflowQueue.length) { if (overflowQueue[i].dataset.priority === 'high') break; + if (overflowQueue[i].classList.contains('active')) break; overflowQueue[i].classList.add('is-overflow'); i++; } @@ -1283,7 +1314,7 @@ window.addEventListener('DOMContentLoaded', () => { // 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') { + if (i < overflowQueue.length && overflowQueue[i].dataset.priority !== 'high' && !overflowQueue[i].classList.contains('active')) { overflowQueue[i].classList.add('is-overflow'); i++; } else { diff --git a/test-nav-priority-1102-e2e.js b/test-nav-priority-1102-e2e.js index 64db40b3..d1e76ffe 100644 --- a/test-nav-priority-1102-e2e.js +++ b/test-nav-priority-1102-e2e.js @@ -143,12 +143,14 @@ async function main() { } } - // #1105 MINOR 9: when at a collapsed width, navigating to a route - // whose link overflows into the More menu must light up #navMoreBtn - // with .active. Verifies rebuildMoreMenu() correctly mirrors the - // active state from the inline (cloned) link to the More button on - // each hashchange (applyNavPriority is wired to hashchange and runs - // after the route handler's class toggles). + // #1105 MINOR 9 (updated by #1391): the active-route pill is now + // PINNED inline at any viewport ≥768px — even if it is not a + // data-priority="high" link. So when we navigate to /#/observers + // (non-high) at 1080px, the observers link MUST stay inline and the + // More menu MUST NOT contain it. The navMoreBtn .active mirror only + // fires when the active route is actually in the dropdown — under + // #1391 that can no longer happen at any width ≥768px, so this test + // verifies the inverse contract. await page.setViewportSize({ width: 1080, height: HEIGHT }); await page.goto(`${BASE}/#/observers`, { waitUntil: 'domcontentloaded' }); await page.waitForSelector('.top-nav .nav-links'); @@ -171,29 +173,41 @@ async function main() { const activeMirror = await page.evaluate(() => { const observersInline = document.querySelector('.nav-links .nav-link[href="#/observers"]'); const inlineHidden = observersInline && observersInline.classList.contains('is-overflow'); + const inlineActive = observersInline && observersInline.classList.contains('active'); + const inlineWidth = observersInline ? observersInline.getBoundingClientRect().width : 0; const moreBtn = document.getElementById('navMoreBtn'); const moreBtnActive = moreBtn ? moreBtn.classList.contains('active') : false; - const moreMenuActiveHrefs = Array.from(document.querySelectorAll('#navMoreMenu .nav-link.active')) + const moreMenuHrefs = Array.from(document.querySelectorAll('#navMoreMenu .nav-link')) .map(a => a.getAttribute('href')); - return { inlineHidden, moreBtnActive, moreMenuActiveHrefs }; + return { inlineHidden, inlineActive, inlineWidth, moreBtnActive, moreMenuHrefs }; }); const mirrorReasons = []; - if (!activeMirror.inlineHidden) { - mirrorReasons.push('precondition: #/observers should be in the More menu at 1080px (not visible inline)'); + // #1391: active link MUST stay inline (not overflowed). + if (activeMirror.inlineHidden) { + mirrorReasons.push('#1391 contract: #/observers is active route — MUST stay inline at 1080px, not in More'); } - if (!activeMirror.moreBtnActive) { - mirrorReasons.push('navMoreBtn missing .active class while #/observers is the active route'); + if (!activeMirror.inlineActive) { + mirrorReasons.push('inline #/observers link missing .active class'); } - if (!activeMirror.moreMenuActiveHrefs.includes('#/observers')) { - mirrorReasons.push(`More-menu clone of #/observers missing .active (active hrefs in menu: [${activeMirror.moreMenuActiveHrefs.join(', ')}])`); + if (activeMirror.inlineWidth === 0) { + mirrorReasons.push('inline #/observers has zero width (clipped)'); + } + // #1391: navMoreBtn should NOT have .active because the active link + // is inline, not in the dropdown. + if (activeMirror.moreBtnActive) { + mirrorReasons.push('navMoreBtn has .active but active route #/observers is inline (mirror should be off)'); + } + // #1391: More menu must NOT contain the active link. + if (activeMirror.moreMenuHrefs.includes('#/observers')) { + mirrorReasons.push(`More menu contains active route #/observers (must be inline only): menu=[${activeMirror.moreMenuHrefs.join(', ')}]`); } if (mirrorReasons.length === 0) { passes++; - console.log(` ✅ active-mirror @1080 #/observers: navMoreBtn.active=true, menu .active=#/observers`); + console.log(` ✅ active-pinned @1080 #/observers: inline + .active set, More mirror off, menu excludes active`); } else { failures++; - console.log(` ❌ active-mirror @1080 #/observers: ${mirrorReasons.join(' | ')}`); + console.log(` ❌ active-pinned @1080 #/observers: ${mirrorReasons.join(' | ')}`); } await browser.close(); diff --git a/test-nav-priority-1391-e2e.js b/test-nav-priority-1391-e2e.js new file mode 100644 index 00000000..27bdd30f --- /dev/null +++ b/test-nav-priority-1391-e2e.js @@ -0,0 +1,183 @@ +#!/usr/bin/env node +/* Issue #1391 — 20th Priority+ nav regression. + * + * Symptom: at viewport ~1080-1200px on a non-high-priority active route + * (e.g. /#/perf, /#/audio-lab), the active-route pill is shoved into the + * More dropdown instead of staying visible inline. Operator screenshot at + * ~1080px on /#/perf showed the navbar with only the "Perf" pill visible + * (or, in the inverse failure mode, NO inline pill at all, with More + * containing only the orphaned active route). + * + * Acceptance (from issue #1391): + * - Active-route pill MUST always be visible inline (never overflowed + * to More) at any viewport ≥768px. + * - If active route is NOT a high-priority link (e.g. /#/perf), the + * high-priority links MUST still be inline ≥768px. + * - Every link in overflow MUST be reachable via the More dropdown + * (the existing #1311/#1139 contract — don't regress). + * + * Mutation guard: removing the "pin active inline" rule in applyNavPriority + * must make this test fail (active link gets overflowed at 1080px on /#/perf). + */ +'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']; + +// Routes whose link is NOT data-priority="high" (verified via +// `grep data-priority public/index.html`). These exercise the +// "active pill is non-high" branch where the bug surfaces. +const NON_HIGH_ROUTES = ['#/perf', '#/audio-lab', '#/analytics', '#/observers']; + +// Operator screenshot was ~1080px. Cover the narrow-desktop CSS branch +// (≤1100) AND the measurement-loop branch (>1100) — bug reproduces in +// both, and the #1311 fix only addressed >1100. +const WIDTHS = [1024, 1080, 1100, 1101, 1200, 1300]; +const HEIGHT = 800; + +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-1391-e2e.js: FAIL — Chromium required but unavailable: ${err.message}`); + process.exit(1); + } + console.log(`test-nav-priority-1391-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 }); + await page.evaluate(() => new Promise(r => requestAnimationFrame(() => requestAnimationFrame(r)))); + + const data = await page.evaluate((route) => { + const links = Array.from(document.querySelectorAll('.nav-links .nav-link')); + let activeHref = null; + let activeOverflowed = false; + let activeWidth = 0; + const visibleHighPri = []; + const overflowedHighPri = []; + for (const a of links) { + const href = a.getAttribute('href'); + const isActive = a.classList.contains('active'); + const isOverflow = a.classList.contains('is-overflow'); + const w = a.getBoundingClientRect().width; + if (isActive) { + activeHref = href; + activeOverflowed = isOverflow; + activeWidth = w; + } + if (a.dataset.priority === 'high') { + if (isOverflow || w === 0) overflowedHighPri.push({ href, isOverflow, w }); + else visibleHighPri.push(href); + } + } + // Open More dropdown and capture its items (clones live in + // .nav-more-menu, the originals stay in .nav-links). + const moreBtn = document.getElementById('navMoreBtn'); + const moreWrap = document.querySelector('.nav-more-wrap'); + const moreMenu = document.getElementById('navMoreMenu'); + const moreVisible = moreWrap && !moreWrap.classList.contains('is-hidden'); + const moreItems = moreMenu + ? Array.from(moreMenu.querySelectorAll('.nav-link')).map(a => a.getAttribute('href')) + : []; + // Every inline-overflowed link must appear in the More dropdown + // (otherwise it's unreachable). + const overflowedHrefs = links + .filter(a => a.classList.contains('is-overflow')) + .map(a => a.getAttribute('href')); + const missingFromMore = overflowedHrefs.filter(h => !moreItems.includes(h)); + return { + activeHref, activeOverflowed, activeWidth, + visibleHighPri, overflowedHighPri, + moreVisible, moreItems, overflowedHrefs, missingFromMore, + }; + }, route); + + const tag = `${w}px @ ${route}`; + const expectedActive = route; + + try { + // (1) Active pill is correctly identified and present inline. + assert.strictEqual( + data.activeHref, expectedActive, + `${tag}: expected active=${expectedActive}, got ${data.activeHref}` + ); + assert.strictEqual( + data.activeOverflowed, false, + `${tag}: active-route pill ${expectedActive} MUST NOT be in overflow ` + + `(was overflowed=${data.activeOverflowed}, width=${data.activeWidth})` + ); + assert.ok( + data.activeWidth > 0, + `${tag}: active-route pill ${expectedActive} must have non-zero width inline ` + + `(got width=${data.activeWidth})` + ); + + // (2) All high-priority links must be inline (regression guard for #1311). + assert.deepStrictEqual( + [...data.visibleHighPri].sort(), + [...HIGH_PRIORITY_HREFS].sort(), + `${tag}: expected all 5 high-pri inline, got [${data.visibleHighPri.join(', ')}] ` + + `overflowed=[${data.overflowedHighPri.map(o => o.href).join(', ')}]` + ); + + // (3) Every overflowed link is reachable via the More dropdown + // (no orphaned overflow links). + assert.deepStrictEqual( + data.missingFromMore, [], + `${tag}: overflowed links missing from More dropdown: [${data.missingFromMore.join(', ')}] ` + + `(more=[${data.moreItems.join(', ')}])` + ); + + passes++; + console.log(` ✅ ${tag}: active inline + ${data.visibleHighPri.length}/5 high-pri inline + ` + + `More has ${data.moreItems.length} item(s)`); + } catch (e) { + failures++; + console.log(` ❌ ${tag}: ${e.message}`); + } + } + } + + await browser.close(); + const total = WIDTHS.length * NON_HIGH_ROUTES.length; + console.log(`\ntest-nav-priority-1391-e2e.js: ${failures === 0 ? 'OK' : 'FAIL'} — ${passes}/${total} passed`); + process.exit(failures === 0 ? 0 : 1); +} + +main().catch((err) => { + console.error('test-nav-priority-1391-e2e.js: fatal', err); + process.exit(1); +});