mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-05-23 06:25:07 +00:00
fix(nav): floor Priority+ overflow at high-priority links — fixes nav vanishing on non-high routes (#1311) (#1312)
Red commit: `5f366b71` — CI: pending (will link once first run starts). Fixes #1311 ## The bug `applyNavPriority` in `public/app.js` had no floor on the iterative overflow loop: ```js let i = 0; while (!fits() && i < overflowQueue.length) { overflowQueue[i].classList.add('is-overflow'); i++; } ``` The `overflowQueue` is built non-high-first then high-priority tail. When `fits()` kept returning `false` — because the active-route pill renders wider than other links — the loop walked past the non-high tail and started dropping high-priority links too. On a non-high active route (`/#/perf`, `/#/audio-lab`, `/#/analytics`, `/#/observers`) at ~1101–1200px, this nuked Home/Packets/Map/Live/Nodes and left the user with brand + "More ▾" + the active pill. ## Repro (master) 1. `go build ./cmd/server` and serve against the e2e fixture 2. Visit `http://localhost:13581/#/perf` at 1101px viewport 3. Inline strip shows only "More ▾" + the ⚡ Perf pill — Home/Packets/Map/Live/Nodes are all gone 4. New E2E (`test-nav-priority-1311-e2e.js`) reproduces this: 4/16 cases fail at 1101px on master. ## The fix Two-line floor in the loop guard: break when the next queue item is a high-priority link. ```js while (!fits() && i < overflowQueue.length) { if (overflowQueue[i].dataset.priority === 'high') break; overflowQueue[i].classList.add('is-overflow'); i++; } ``` The `>=2` More-menu floor (#1139) gets the same guard — never promote a high-priority link just to hit the floor. A degenerate 1-item dropdown is a smaller paper-cut than nuking primary nav. ## TDD trail - **RED commit `5f366b71`**: `test-nav-priority-1311-e2e.js` lands first. Asserts (`assert.deepStrictEqual`) all 5 high-priority hrefs are visible inline at 900/1024/1101/1200px on /#/perf, /#/audio-lab, /#/analytics, /#/observers (16 cases). Fails 4/16 against master. - **GREEN commit `6d1a5542`**: floor added; 16/16 pass. Existing nav suite still green: - `test-nav-priority-1102-e2e.js`: 5/5 ✅ - `test-nav-more-floor-1139-e2e.js`: 10/10 ✅ - `test-nav-fluid-1055-e2e.js`: 20/20 ✅ - **Mutation guard**: stash the floor → test fails 4/16 again on the same cases. Browser verified: chromium 136 against local Go server with `test-fixtures/e2e-fixture.db` at 900/1024/1101/1200px on each non-high route. E2E assertion added: `test-nav-priority-1311-e2e.js:107` (`assert.deepStrictEqual`). ## Constraints respected - Existing 5/5 inline behavior on /#/home (active route IS high-priority) — preserved by 1102 suite ✅ - `<=1100` branch — unchanged (already data-priority-aware) ✅ - `>=2` More-menu floor (#1139) — preserved + extended with the same high-pri guard ✅ - All colors via CSS vars ✅ - PII preflight clean ✅ --------- Co-authored-by: CoreScope Bot <bot@corescope>
This commit is contained in:
Kpa-clawbot
@@ -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
|
||||
|
||||
+19
-1
@@ -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 {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user