mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-06-01 04:14:19 +00:00
f0a7ed758f
## What
Pins the active-route `.nav-link` inline at any viewport ≥768px so
Priority+ never shoves it into the More dropdown. Fixes the operator's
screenshot of `/#/perf` at ~1080px where the navbar showed only the
active "Perf" pill missing — and an inverse failure where the active
pill was the only thing **in** the dropdown.
This is the 20th regression of nav Priority+. Single-loop fix only; no
algorithm redesign (per issue out-of-scope).
## Root cause
`public/app.js` `applyNavPriority()` had two places that ignored the
active state:
1. **≤1100 narrow-desktop CSS branch (line ~1197):** `if
(a.dataset.priority !== 'high') a.classList.add('is-overflow')` blindly
overflowed every non-high link — including the active pill.
2. **>1100 measurement loop (line ~1267):** `overflowQueue` is `non-high
reversed + high reversed`. The active non-high link enters the queue and
the loop's only break condition is `priority === 'high'`. fits() keeps
returning false (active pill is wider — has the `.active`
background/padding), so the loop walks the entire non-high tail and
orphans the active route in More.
The acceptance criterion "Active-route pill MUST always be visible
inline" was never encoded — #1311's floor only protected
`data-priority="high"`.
## Why prior #1311 / #1148 / #1139 floors didn't catch this
- **#1311** floored at `data-priority="high"` only. `/#/perf` is
`data-priority=""` so it had no protection.
- **#1148 / #1139** floored the *More menu* at ≥2 items but didn't
constrain *which* links could be promoted/dropped.
- **#1106** narrow-desktop CSS branch (≤1100) was written before
active-pill width drift was a known issue.
## Fix
One conceptual rule applied at three points:
1. In `overflowQueue` construction, skip any link with `.active` (treat
active like high-priority — never enqueue).
2. In the ≤1100 CSS branch, skip the active link when assigning
`.is-overflow`.
3. In the >1100 loop, also break on `.active` (defensive — queue already
excludes it).
Approach chosen over "pin active-pill max-width during measurement":
measurement-pinning would silently shrink the pill visually mid-resize,
and width drift from #1378's new `--mc-*` vars made that fragile.
Treating active as a hard inline pin matches the documented contract and
is one greppable invariant.
## TDD red → green
- **Red commit `34d69012`:** added `test-nav-priority-1391-e2e.js`
covering `/#/perf, /#/audio-lab, /#/analytics, /#/observers` at `1024,
1080, 1100, 1101, 1200, 1300px`. Asserts (1) active pill not in
overflow, (2) all 5 high-pri still inline (#1311 guard), (3) every
overflowed link mirrored in More dropdown (no orphans). 0/24 passed
locally on red.
- **Green commit:** same test 24/24 pass. Existing #1311 (20/20), #1139
floor, #1102 contract still green.
## Manual verification
Local fixture server (`./corescope-server -port 13581 -db
test-fixtures/e2e-fixture.db -public public`):
- `/#/perf` @ 1080×800: brand + 5 high-pri inline + "Perf" pill inline +
"More ▾" containing the 5 low-pri links (Channels, Tools, Observers,
Analytics, Audio Lab). ✅
- `/#/perf` @ 1300×800: brand + 5 high-pri + "Perf" inline; More hidden
(only 4 low-pri items overflow). ✅
- `/#/perf` @ 800×800 (narrow): hamburger code path untouched. ✅
- Inverse `/#/home` @ 1080×800 (active IS high-pri): no behaviour
change. ✅
## Preflight
`bash ~/.openclaw/skills/pr-preflight/scripts/run-all.sh origin/master`
— exit 0.
Browser verified: local fixture server + Playwright on Chromium
(`/usr/bin/chromium`).
E2E assertion added: `test-nav-priority-1391-e2e.js:138-148`
(`activeOverflowed === false`).
Fixes #1391
---------
Co-authored-by: openclaw-bot <bot@openclaw.local>
222 lines
10 KiB
JavaScript
222 lines
10 KiB
JavaScript
#!/usr/bin/env node
|
|
/* Issue #1102 — Nav Priority+ at very wide widths and "More" menu correctness.
|
|
*
|
|
* Regression from PR #1097 polish: at all widths >=768px the CSS rule
|
|
* .nav-links a:not([data-priority="high"]) { display: none; }
|
|
* unconditionally hides 6 of 11 links, even at 2560px where there is
|
|
* plenty of room for everything. The "More" menu is built once on load
|
|
* from the same selector, so it correctly shows the hidden links — but
|
|
* the bug here is the SET being hidden is wrong (way too aggressive).
|
|
*
|
|
* Acceptance:
|
|
* - At 2560px: ALL 11 links visible inline AND "More ▾" hidden.
|
|
* - At 1920px: at least 9 links visible (room for most).
|
|
* - At 1080px: 5 high-priority links visible AND More menu contains
|
|
* every link not currently visible inline.
|
|
* - At 768px (just above hamburger threshold): 5 high-priority links
|
|
* visible AND More menu non-empty.
|
|
*
|
|
* #1105 MINOR 7: at 1080/800px we now assert the visible set is *exactly*
|
|
* the 5 high-priority links (Home/Packets/Map/Live/Nodes). A buggy queue
|
|
* that hid Home and showed Lab would still pass the cardinality check.
|
|
*
|
|
* #1105 MINOR 9: also asserts that navigating to a route whose link
|
|
* lives in the More menu lights up #navMoreBtn with .active.
|
|
*/
|
|
'use strict';
|
|
|
|
const { chromium } = require('playwright');
|
|
|
|
const BASE = process.env.BASE_URL || 'http://localhost:13581';
|
|
|
|
// [width, expected behavior]
|
|
// requireExactHighPri: when true, asserts the visible set matches HIGH_PRIORITY_HREFS exactly
|
|
const HIGH_PRIORITY_HREFS = ['#/home', '#/packets', '#/map', '#/live', '#/nodes'];
|
|
const CASES = [
|
|
// viewport, minVisible, moreVisible, requireExactHighPri, label
|
|
{ w: 2560, minVisible: 11, moreVisible: false, requireExactHighPri: false, label: '2560px — all visible' },
|
|
{ w: 1920, minVisible: 9, moreVisible: null, requireExactHighPri: false, label: '1920px — most visible' },
|
|
{ w: 1080, minVisible: 5, moreVisible: true, requireExactHighPri: true, label: '1080px — collapsed' },
|
|
{ w: 800, minVisible: 5, moreVisible: true, requireExactHighPri: true, label: '800px — collapsed' },
|
|
];
|
|
|
|
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-1102-e2e.js: FAIL — Chromium required but unavailable: ${err.message}`);
|
|
process.exit(1);
|
|
}
|
|
console.log(`test-nav-priority-1102-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 c of CASES) {
|
|
await page.setViewportSize({ width: c.w, height: HEIGHT });
|
|
await page.goto(`${BASE}/#/home`, { 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 });
|
|
|
|
const data = await page.evaluate(() => {
|
|
const links = Array.from(document.querySelectorAll('.nav-links .nav-link'));
|
|
const visible = links.filter(a => getComputedStyle(a).display !== 'none');
|
|
const visibleHrefs = visible.map(a => a.getAttribute('href'));
|
|
const allHrefs = links.map(a => a.getAttribute('href'));
|
|
const hiddenInline = allHrefs.filter(h => !visibleHrefs.includes(h));
|
|
const moreWrap = document.querySelector('.nav-more-wrap');
|
|
const moreVisible = moreWrap ? getComputedStyle(moreWrap).display !== 'none' : false;
|
|
const moreMenuLinks = Array.from(document.querySelectorAll('#navMoreMenu .nav-link'))
|
|
.map(a => a.getAttribute('href'));
|
|
return { totalLinks: links.length, visibleCount: visible.length,
|
|
visibleHrefs, hiddenInline, moreVisible, moreMenuLinks };
|
|
});
|
|
|
|
const reasons = [];
|
|
if (data.visibleCount < c.minVisible) {
|
|
reasons.push(`only ${data.visibleCount}/${data.totalLinks} links visible (need >=${c.minVisible})`);
|
|
}
|
|
if (c.moreVisible === true && !data.moreVisible) {
|
|
reasons.push(`"More" button should be visible but is hidden`);
|
|
}
|
|
if (c.moreVisible === false && data.moreVisible) {
|
|
reasons.push(`"More" button should be HIDDEN at ${c.w}px (all links fit) but is visible`);
|
|
}
|
|
// More menu MUST contain every link not currently visible inline.
|
|
if (data.moreVisible) {
|
|
const missing = data.hiddenInline.filter(h => !data.moreMenuLinks.includes(h));
|
|
if (missing.length) {
|
|
reasons.push(`More menu missing hidden links: ${missing.join(', ')} ` +
|
|
`(menu has ${data.moreMenuLinks.length}, expected ${data.hiddenInline.length})`);
|
|
}
|
|
}
|
|
// #1105 MINOR 7: identity, not just cardinality. The 5 visible links
|
|
// at the collapsed widths must be EXACTLY the high-priority set
|
|
// (Home/Packets/Map/Live/Nodes). A buggy queue that hid Home and
|
|
// showed Lab would still pass `visibleCount >= 5`.
|
|
if (c.requireExactHighPri) {
|
|
const missingHighPri = HIGH_PRIORITY_HREFS.filter(h => !data.visibleHrefs.includes(h));
|
|
if (missingHighPri.length) {
|
|
reasons.push(`high-priority link(s) NOT visible inline: ${missingHighPri.join(', ')} ` +
|
|
`(visible=[${data.visibleHrefs.join(', ')}])`);
|
|
}
|
|
const extra = data.visibleHrefs.filter(h => !HIGH_PRIORITY_HREFS.includes(h));
|
|
if (extra.length) {
|
|
reasons.push(`unexpected non-high-priority link(s) visible: ${extra.join(', ')} ` +
|
|
`(expected exactly [${HIGH_PRIORITY_HREFS.join(', ')}])`);
|
|
}
|
|
}
|
|
|
|
const tag = c.label;
|
|
if (reasons.length === 0) {
|
|
passes++;
|
|
console.log(` ✅ ${tag}: visible=${data.visibleCount}/${data.totalLinks} more=${data.moreVisible} menu=${data.moreMenuLinks.length}`);
|
|
} else {
|
|
failures++;
|
|
console.log(` ❌ ${tag}: ${reasons.join(' | ')} ` +
|
|
`(visible=${data.visibleCount}/${data.totalLinks} more=${data.moreVisible} menu=${data.moreMenuLinks.length})`);
|
|
}
|
|
}
|
|
|
|
// #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');
|
|
await page.evaluate(() => document.fonts && document.fonts.ready ? document.fonts.ready : null);
|
|
// Wait for layout to settle.
|
|
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 the hashchange-triggered applyNavPriority a frame to run.
|
|
await page.evaluate(() => new Promise(r => requestAnimationFrame(() => requestAnimationFrame(r))));
|
|
|
|
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 moreMenuHrefs = Array.from(document.querySelectorAll('#navMoreMenu .nav-link'))
|
|
.map(a => a.getAttribute('href'));
|
|
return { inlineHidden, inlineActive, inlineWidth, moreBtnActive, moreMenuHrefs };
|
|
});
|
|
|
|
const mirrorReasons = [];
|
|
// #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.inlineActive) {
|
|
mirrorReasons.push('inline #/observers link missing .active class');
|
|
}
|
|
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-pinned @1080 #/observers: inline + .active set, More mirror off, menu excludes active`);
|
|
} else {
|
|
failures++;
|
|
console.log(` ❌ active-pinned @1080 #/observers: ${mirrorReasons.join(' | ')}`);
|
|
}
|
|
|
|
await browser.close();
|
|
console.log(`\ntest-nav-priority-1102-e2e.js: ${failures === 0 ? 'OK' : 'FAIL'} — ${passes}/${CASES.length + 1} passed`);
|
|
process.exit(failures === 0 ? 0 : 1);
|
|
}
|
|
|
|
main().catch((err) => {
|
|
console.error('test-nav-priority-1102-e2e.js: fatal', err);
|
|
process.exit(1);
|
|
});
|