mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-08-27 22:35:21 +00:00
fix(nav): surface Coverage route in mobile nav when enabled (#1783)
Fixes #1782. ## Problem When `clientRxCoverage` is enabled, the **Coverage** route (`#/rx-coverage`) is reachable from the desktop top-nav but **unreachable on mobile** — neither the bottom-nav "More" sheet (phones, ≤768px) nor the edge-swipe drawer (touch tablets, >768px) lists it. ## Root cause `public/roles.js` injects the Coverage link **only into the desktop top-nav** (`.nav-links`), gated on `window.MC_CLIENT_RX_COVERAGE`. The two mobile nav surfaces build their long-tail lists from **independent hardcoded arrays** that omitted `rx-coverage`: - `public/bottom-nav.js` → `MORE_ROUTES` - `public/nav-drawer.js` → `ROUTES` Both even carry `!! MANUAL SYNC REQUIRED !!` comments. Because the link is injected into the DOM (not these arrays) and is config-gated, it never reached mobile. ## Fix Both surfaces now insert the Coverage entry **right after Analytics** (matching the desktop top-nav insertion point) when `window.MC_CLIENT_RX_COVERAGE` is true. The check is evaluated at **lazy build time** (first sheet/drawer open), by which point `MeshConfigReady` has resolved the flag. Default-off behaviour is unchanged, so the default nav still matches the existing nav-overflow tests. ## Testing Adds `test-rx-coverage-mobile-nav-e2e.js`, which: - skips cleanly when Chromium is unavailable (`CHROMIUM_REQUIRE=1` makes it a hard fail) or when `clientRxCoverage` is disabled — mirroring `test-node-reach-coverage-e2e.js`; - at 360px asserts Coverage is present in the bottom-nav More sheet, ordered after Analytics, and that tapping it navigates to `#/rx-coverage`; - at 1024px asserts Coverage is present in the edge-swipe drawer, ordered after Analytics. Verified locally against a server built from this branch with `clientRxCoverage` enabled (migrated `test-fixtures/e2e-fixture.db`): - new test: **3/3 pass**; reverting the two source files makes it **fail 3/3** (true regression test); - existing nav e2e suites still green: `test-nav-drawer-1064-e2e.js` (11/11), `test-bottom-nav-1061-e2e.js` (31/31), `test-nav-more-floor-1139-e2e.js` (10/10). ## Notes - No perf impact: the route list is built once, lazily, on first sheet/drawer open. - The hardcoded `MORE_ROUTES` / `ROUTES` arrays remain the source of truth for the always-on routes; this only conditionally appends the one opt-in route, consistent with how `roles.js` already gates the desktop link. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
5c0de8fb41
commit
55e203a9c8
+15
-1
@@ -81,6 +81,20 @@
|
||||
{ route: 'audio-lab', hash: '#/audio-lab', label: 'Audio Lab', ph: 'music-note' },
|
||||
];
|
||||
|
||||
// The Coverage route is opt-in (config `clientRxCoverage`). roles.js injects
|
||||
// its link into the desktop top-nav only, so it must be added here too or it
|
||||
// is unreachable on phones. Evaluated at lazy build time (first sheet open),
|
||||
// by which point MeshConfigReady has resolved window.MC_CLIENT_RX_COVERAGE.
|
||||
var COVERAGE_ROUTE = { route: 'rx-coverage', hash: '#/rx-coverage', label: 'Coverage', ph: 'broadcast' };
|
||||
|
||||
function moreRoutes() {
|
||||
if (!window.MC_CLIENT_RX_COVERAGE) return MORE_ROUTES;
|
||||
var out = MORE_ROUTES.slice();
|
||||
var after = out.findIndex(function (r) { return r.route === 'analytics'; }) + 1;
|
||||
out.splice(after, 0, COVERAGE_ROUTE);
|
||||
return out;
|
||||
}
|
||||
|
||||
var SHEET_ID = 'bottomNavMoreSheet';
|
||||
|
||||
function currentRoute() {
|
||||
@@ -200,7 +214,7 @@
|
||||
sheet.setAttribute('aria-label', 'More navigation');
|
||||
sheet.hidden = true;
|
||||
|
||||
MORE_ROUTES.forEach(function (r) {
|
||||
moreRoutes().forEach(function (r) {
|
||||
var a = document.createElement('a');
|
||||
a.className = 'bottom-nav-sheet-item';
|
||||
a.setAttribute('href', r.hash);
|
||||
|
||||
+15
-1
@@ -65,6 +65,20 @@
|
||||
{ route: 'audio-lab', hash: '#/audio-lab', label: 'Audio Lab', ph: 'music-note' },
|
||||
];
|
||||
|
||||
// Coverage is opt-in (config `clientRxCoverage`). roles.js injects its link
|
||||
// into the desktop top-nav only, so add it here too or it is unreachable via
|
||||
// the drawer on touch tablets (>768px). Evaluated at lazy build time, by which
|
||||
// point MeshConfigReady has resolved window.MC_CLIENT_RX_COVERAGE.
|
||||
var COVERAGE_ROUTE = { route: 'rx-coverage', hash: '#/rx-coverage', label: 'Coverage', ph: 'broadcast' };
|
||||
|
||||
function routes() {
|
||||
if (!window.MC_CLIENT_RX_COVERAGE) return ROUTES;
|
||||
var out = ROUTES.slice();
|
||||
var after = out.findIndex(function (r) { return r.route === 'analytics'; }) + 1;
|
||||
out.splice(after, 0, COVERAGE_ROUTE);
|
||||
return out;
|
||||
}
|
||||
|
||||
function phIconHTML(name) {
|
||||
return '<svg class="ph-icon" aria-hidden="true" focusable="false">' +
|
||||
'<use href="/icons/phosphor-sprite.svg#ph-' + name + '"></use></svg>';
|
||||
@@ -126,7 +140,7 @@
|
||||
|
||||
var list = document.createElement('nav');
|
||||
list.className = 'nav-drawer-list';
|
||||
ROUTES.forEach(function (r) {
|
||||
routes().forEach(function (r) {
|
||||
var a = document.createElement('a');
|
||||
a.className = 'nav-drawer-item';
|
||||
a.setAttribute('href', r.hash);
|
||||
|
||||
@@ -0,0 +1,159 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Coverage route reachability on mobile navigation.
|
||||
*
|
||||
* The "Coverage" route (#/rx-coverage) is opt-in: roles.js injects its link
|
||||
* into the DESKTOP top-nav (.nav-links) only, when window.MC_CLIENT_RX_COVERAGE
|
||||
* is true. The two mobile navigation surfaces build their long-tail route lists
|
||||
* independently:
|
||||
* - public/bottom-nav.js → the "More" sheet (phones, ≤768px)
|
||||
* - public/nav-drawer.js → the edge-swipe drawer (tablets, >768px)
|
||||
* Both used hardcoded arrays that omitted rx-coverage, so when Coverage was
|
||||
* enabled it was present on desktop but UNREACHABLE on mobile. This test pins
|
||||
* that, once enabled, Coverage appears in BOTH mobile surfaces, ordered right
|
||||
* after Analytics (matching the desktop top-nav insertion point).
|
||||
*
|
||||
* Skips cleanly when:
|
||||
* - Chromium is unavailable (unless CHROMIUM_REQUIRE=1 → hard fail), or
|
||||
* - the deployment under test has clientRxCoverage disabled (default off).
|
||||
*
|
||||
* Stable selectors consumed here:
|
||||
* [data-bottom-nav-tab="more"] — More tab (opens the sheet)
|
||||
* [data-bottom-nav-more-route="rx-coverage"] — Coverage row in the sheet
|
||||
* window.__navDrawer.open() — opens the edge-swipe drawer
|
||||
* [data-nav-drawer-item="rx-coverage"] — Coverage row in the drawer
|
||||
*/
|
||||
'use strict';
|
||||
|
||||
const { chromium } = require('playwright');
|
||||
|
||||
const BASE = process.env.BASE_URL || 'http://localhost:3000';
|
||||
|
||||
let passed = 0, failed = 0;
|
||||
async function step(name, fn) {
|
||||
try { await fn(); passed++; console.log(' ✓ ' + name); }
|
||||
catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); }
|
||||
}
|
||||
function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); }
|
||||
|
||||
(async () => {
|
||||
const requireChromium = process.env.CHROMIUM_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-rx-coverage-mobile-nav-e2e.js: FAIL — Chromium required but unavailable: ${err.message}`);
|
||||
process.exit(1);
|
||||
}
|
||||
console.log(`test-rx-coverage-mobile-nav-e2e.js: SKIP (Chromium unavailable: ${err.message.split('\n')[0]})`);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
console.log(`\n=== Coverage mobile-nav reachability E2E against ${BASE} ===`);
|
||||
|
||||
// Coverage is opt-in (config flag, default off). Skip when the deployment
|
||||
// under test has it disabled — mirrors test-node-reach-coverage-e2e.js.
|
||||
const probeCtx = await browser.newContext();
|
||||
const probe = await probeCtx.newPage();
|
||||
let enabled = false;
|
||||
try {
|
||||
const cfg = await (await probe.request.get(BASE + '/api/config/client')).json();
|
||||
enabled = cfg.clientRxCoverage === true;
|
||||
} catch (e) {
|
||||
console.log(`test-rx-coverage-mobile-nav-e2e.js: SKIP (could not read /api/config/client: ${e.message})`);
|
||||
await browser.close();
|
||||
process.exit(0);
|
||||
}
|
||||
await probeCtx.close();
|
||||
if (!enabled) {
|
||||
console.log('test-rx-coverage-mobile-nav-e2e.js: SKIP (clientRxCoverage disabled on this deployment)');
|
||||
await browser.close();
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// ── Phone viewport: bottom-nav "More" sheet ──
|
||||
const phoneCtx = await browser.newContext({ viewport: { width: 360, height: 800 } });
|
||||
const phone = await phoneCtx.newPage();
|
||||
phone.setDefaultTimeout(10000);
|
||||
phone.on('pageerror', (e) => console.error('[pageerror]', e.message));
|
||||
|
||||
await phone.goto(BASE + '/#/packets', { waitUntil: 'domcontentloaded' });
|
||||
await phone.waitForSelector('main#app', { timeout: 8000 });
|
||||
// Wait until roles.js has resolved the config flag (the lazy sheet build
|
||||
// reads window.MC_CLIENT_RX_COVERAGE at open time).
|
||||
await phone.waitForFunction(() => window.MC_CLIENT_RX_COVERAGE === true, null, { timeout: 8000 });
|
||||
|
||||
await step('(a) bottom-nav More sheet contains Coverage, ordered after Analytics', async () => {
|
||||
await phone.click('[data-bottom-nav-tab="more"]');
|
||||
await phone.waitForTimeout(150);
|
||||
const info = await phone.evaluate(() => {
|
||||
const cov = document.querySelector('[data-bottom-nav-more-route="rx-coverage"]');
|
||||
if (!cov) return { present: false };
|
||||
const items = Array.prototype.map.call(
|
||||
document.querySelectorAll('[data-bottom-nav-more-route]'),
|
||||
(a) => a.getAttribute('data-bottom-nav-more-route')
|
||||
);
|
||||
const label = (cov.textContent || '').trim();
|
||||
return {
|
||||
present: true,
|
||||
label,
|
||||
afterAnalytics: items.indexOf('rx-coverage') === items.indexOf('analytics') + 1,
|
||||
items,
|
||||
};
|
||||
});
|
||||
assert(info.present, 'Coverage row missing from bottom-nav More sheet');
|
||||
assert(/Coverage/.test(info.label), `expected label "Coverage", got "${info.label}"`);
|
||||
assert(info.afterAnalytics, `Coverage not directly after Analytics: ${info.items.join(', ')}`);
|
||||
});
|
||||
|
||||
await step('(b) tapping Coverage in the sheet navigates to #/rx-coverage', async () => {
|
||||
await phone.click('[data-bottom-nav-more-route="rx-coverage"]');
|
||||
await phone.waitForTimeout(200);
|
||||
const hash = await phone.evaluate(() => location.hash);
|
||||
assert(hash.indexOf('#/rx-coverage') === 0, `expected hash #/rx-coverage, got ${hash}`);
|
||||
});
|
||||
|
||||
await phoneCtx.close();
|
||||
|
||||
// ── Tablet viewport: edge-swipe drawer (enabled > 768px) ──
|
||||
const tabletCtx = await browser.newContext({ viewport: { width: 1024, height: 800 } });
|
||||
const tablet = await tabletCtx.newPage();
|
||||
tablet.setDefaultTimeout(10000);
|
||||
tablet.on('pageerror', (e) => console.error('[pageerror]', e.message));
|
||||
|
||||
await tablet.goto(BASE + '/#/packets', { waitUntil: 'domcontentloaded' });
|
||||
await tablet.waitForSelector('main#app', { timeout: 8000 });
|
||||
await tablet.waitForFunction(() => window.MC_CLIENT_RX_COVERAGE === true, null, { timeout: 8000 });
|
||||
|
||||
await step('(c) edge-swipe drawer contains Coverage, ordered after Analytics', async () => {
|
||||
await tablet.evaluate(() => window.__navDrawer && window.__navDrawer.open && window.__navDrawer.open());
|
||||
await tablet.waitForTimeout(150);
|
||||
const info = await tablet.evaluate(() => {
|
||||
const cov = document.querySelector('[data-nav-drawer-item="rx-coverage"]');
|
||||
if (!cov) return { present: false };
|
||||
const items = Array.prototype.map.call(
|
||||
document.querySelectorAll('[data-nav-drawer-item]'),
|
||||
(a) => a.getAttribute('data-nav-drawer-item')
|
||||
);
|
||||
return {
|
||||
present: true,
|
||||
label: (cov.textContent || '').trim(),
|
||||
afterAnalytics: items.indexOf('rx-coverage') === items.indexOf('analytics') + 1,
|
||||
items,
|
||||
};
|
||||
});
|
||||
assert(info.present, 'Coverage row missing from edge-swipe drawer');
|
||||
assert(/Coverage/.test(info.label), `expected label "Coverage", got "${info.label}"`);
|
||||
assert(info.afterAnalytics, `Coverage not directly after Analytics: ${info.items.join(', ')}`);
|
||||
});
|
||||
|
||||
await tabletCtx.close();
|
||||
await browser.close();
|
||||
console.log(`\n=== Results: passed ${passed} failed ${failed} ===`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
})().catch((e) => { console.error(e); process.exit(1); });
|
||||
Reference in New Issue
Block a user