mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-24 04:14:27 +00:00
## Summary Make the top-nav use the **Priority+ pattern at all widths** (not just 768–1279px), so the nav-right cluster never gets pushed off-screen or visually overlapped by the link strip. Fixes #1055. ## What changed **`public/style.css`** — nav section only (clearly fenced): - Removed the upper bound on the Priority+ media query (`max-width: 1279px`). The rule now applies at any viewport `>= 768px`. Above that breakpoint, only `data-priority="high"` links render inline; the rest collapse into the existing `More ▾` overflow menu. - Swapped nav-only hardcoded spacing/type to the fluid `clamp()` tokens shipped in #1054: - `.top-nav` padding → `var(--gutter)` - `.nav-left` gap → `var(--space-lg)` - `.nav-brand` gap → `var(--space-sm)`, font-size → `var(--fs-md)` - `.nav-links` gap → `var(--space-xs)` - `.nav-link` padding → `clamp(8px, 0.6vw + 4px, 14px)`, font-size → `var(--fs-sm)` - `.nav-right` gap → `var(--space-sm)` - Mobile (<768px) hamburger layout, the More-menu markup, and the JS that builds the menu in `public/app.js` are unchanged — they already supported this pattern. `public/index.html` did not need changes — the `data-priority="high"` markup, `nav-more-wrap`, `navMoreBtn`, and `navMoreMenu` are already in place from earlier work. ## Why the bug existed The previous Priority+ rule was scoped `@media (min-width: 768px) and (max-width: 1279px)`. From 1280px–~1599px the full 11-link strip rendered but didn't fit alongside `.nav-stats` + `.nav-right`. The parent `overflow: hidden` masked the symptom, but the rightmost links physically rendered underneath `.nav-right` and were unreachable. ## E2E assertion added New `test-nav-fluid-1055-e2e.js` — Playwright multi-viewport test (768/1024/1280/1440/1920) that asserts: 1. `.nav-right.right` ≤ `document.documentElement.clientWidth` (no horizontal overflow) 2. Last visible `.nav-link.right` ≤ `.nav-right.left` (no overlap underneath the right cluster) 3. `.top-nav.scrollWidth` ≤ `.top-nav.clientWidth` (no scrolled-off content) Wired into the `e2e-test` job in `.github/workflows/deploy.yml`. **TDD evidence:** - Red commit `466221a`: test passes 3/5 (1024/768/1920) — fails at 1280 (253px overlap) and 1440 (93px overlap). - Green commit `1aa939a`: test passes 5/5. ## Acceptance criteria (from #1055) - [x] Priority+ at ALL widths (not just mobile). - [x] No nav link overflow at 1080px (or any tested width). - [x] Overflow menu accessible via keyboard + touch (existing `navMoreBtn` aria-haspopup wiring; verified by existing app.js handlers). - [x] Active route still highlighted when in overflow (existing logic in `app.js` adds `.active` to the cloned link in `navMoreMenu`). - [x] Tested at 768/1024/1280/1440/1920 — visible link count adapts (5 priority links + More menu at all desktop widths; full 11 inline only on hamburger mobile when expanded). --------- Co-authored-by: bot <bot@corescope> Co-authored-by: clawbot <clawbot@users.noreply.github.com> Co-authored-by: meshcore-bot <bot@meshcore.local>
This commit is contained in:
3 files changed
+200
-16
No files matched your search
@@ -216,6 +216,7 @@ jobs:
|
||||
BASE_URL=http://localhost:13581 node test-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt
|
||||
BASE_URL=http://localhost:13581 node test-channel-issue-1087-e2e.js 2>&1 | tee -a e2e-output.txt
|
||||
BASE_URL=http://localhost:13581 node test-map-modal-fluid-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
|
||||
|
||||
- name: Collect frontend coverage (parallel)
|
||||
if: success() && github.event_name == 'push'
|
||||
|
||||
+43
-16
@@ -370,16 +370,24 @@ input[type="week"] {
|
||||
.sort-help:focus .sort-help-tip,
|
||||
.sort-help:focus-within .sort-help-tip { display: block; }
|
||||
|
||||
/* === Nav === */
|
||||
/* === Nav (Issue #1055 — Priority+ at all widths) ===
|
||||
The top-nav uses Priority+ at ALL widths: only links marked
|
||||
data-priority="high" render inline; everything else collapses into
|
||||
the "More ▾" menu. This eliminates the historic 1080–1279px overflow
|
||||
window AND the 1280–1599px overlap window where the full link strip
|
||||
ran underneath .nav-right. Spacing and type use the #1054 fluid
|
||||
clamp() tokens so the bar scales smoothly across viewport widths.
|
||||
The hamburger and stacked mobile layout still take over at <768px. */
|
||||
.top-nav {
|
||||
display: flex; align-items: center; justify-content: space-between;
|
||||
background: linear-gradient(135deg, var(--nav-bg) 0%, var(--nav-bg2) 100%); color: var(--nav-text); padding: 0 20px; height: 52px;
|
||||
background: linear-gradient(135deg, var(--nav-bg) 0%, var(--nav-bg2) 100%); color: var(--nav-text);
|
||||
padding: 0 var(--gutter); height: 52px;
|
||||
position: sticky; top: 0; z-index: 1100;
|
||||
box-shadow: 0 2px 8px rgba(0,0,0,.3);
|
||||
flex-wrap: nowrap; overflow: hidden; min-width: 0;
|
||||
}
|
||||
.nav-left { display: flex; align-items: center; gap: 24px; min-width: 0; flex-shrink: 1; overflow: hidden; }
|
||||
.nav-brand { display: flex; align-items: center; gap: 8px; text-decoration: none; color: var(--nav-text); font-weight: 700; font-size: 16px; }
|
||||
.nav-left { display: flex; align-items: center; gap: var(--space-lg); min-width: 0; flex-shrink: 1; overflow: hidden; }
|
||||
.nav-brand { display: flex; align-items: center; gap: var(--space-sm); text-decoration: none; color: var(--nav-text); font-weight: 700; font-size: var(--fs-md); }
|
||||
.brand-icon { font-size: 20px; }
|
||||
.live-dot {
|
||||
width: 8px; height: 8px; border-radius: 50%; background: var(--text-muted);
|
||||
@@ -392,9 +400,10 @@ input[type="week"] {
|
||||
}
|
||||
.live-dot.connected { background: var(--status-green); animation: pulse-ring 2s ease-out infinite; }
|
||||
|
||||
.nav-links { display: flex; align-items: center; gap: 4px; }
|
||||
.nav-links { display: flex; align-items: center; gap: var(--space-xs); }
|
||||
.nav-link {
|
||||
color: var(--nav-text-muted); text-decoration: none; padding: 14px 12px; font-size: 14px;
|
||||
color: var(--nav-text-muted); text-decoration: none;
|
||||
padding: 14px clamp(8px, 0.6vw + 4px, 14px); font-size: var(--fs-sm);
|
||||
border-bottom: 2px solid transparent; transition: all .15s;
|
||||
background: none; border-top: none; border-left: none; border-right: none;
|
||||
cursor: pointer; font-family: var(--font);
|
||||
@@ -407,7 +416,7 @@ input[type="week"] {
|
||||
background: var(--nav-active-bg);
|
||||
border-radius: 6px;
|
||||
margin: 4px 0;
|
||||
padding: 10px 12px;
|
||||
padding: 10px clamp(8px, 0.6vw + 4px, 14px);
|
||||
}
|
||||
|
||||
.nav-dropdown { position: relative; }
|
||||
@@ -422,7 +431,7 @@ input[type="week"] {
|
||||
}
|
||||
.dropdown-item:hover { background: var(--accent); color: #fff; }
|
||||
|
||||
.nav-right { display: flex; align-items: center; gap: 8px; flex-shrink: 0; }
|
||||
.nav-right { display: flex; align-items: center; gap: var(--space-sm); flex-shrink: 0; }
|
||||
.nav-btn {
|
||||
background: none; border: 1px solid var(--border); color: var(--nav-text-muted); padding: 6px 12px;
|
||||
border-radius: 6px; cursor: pointer; font-size: 14px; transition: all .15s;
|
||||
@@ -1267,18 +1276,36 @@ button.ch-item:hover .ch-icon-btn { opacity: 1; }
|
||||
.map-controls { width: 180px; font-size: 12px; }
|
||||
}
|
||||
|
||||
/* === Responsive — Tablet/Mid Priority+ nav (768–1279px) ===
|
||||
At <1280px the full nav-links + nav-stats + nav-right buttons can't fit
|
||||
on one row (they total ~1540px on the home page). Collapse non-priority
|
||||
links into the "More ▾" menu so .nav-right stays inside the viewport. */
|
||||
@media (min-width: 768px) and (max-width: 1279px) {
|
||||
.nav-links { display: flex !important; flex-direction: row; gap: 2px; }
|
||||
/* === Nav Priority+ — applies at ALL desktop widths (Issue #1055) ===
|
||||
Above the mobile hamburger breakpoint, only data-priority="high" links
|
||||
render inline; the rest collapse into the "More ▾" overflow menu.
|
||||
This guarantees nav-right cannot be pushed off-screen or overlapped by
|
||||
the link strip at any common viewport (768/1024/1280/1440/1920+). */
|
||||
@media (min-width: 768px) {
|
||||
.nav-links { display: flex !important; flex-direction: row; gap: var(--space-xs); }
|
||||
.nav-links a:not([data-priority="high"]) { display: none; }
|
||||
.nav-more-wrap { display: flex; align-items: center; }
|
||||
.hamburger { display: none; }
|
||||
.nav-link { padding: 14px 8px; font-size: 13px; }
|
||||
.nav-link { padding: 14px clamp(8px, 0.6vw + 4px, 14px); font-size: var(--fs-sm); }
|
||||
.nav-links a[data-priority="high"] { order: -1; }
|
||||
.nav-link.active { background: var(--nav-active-bg); border-radius: 6px; margin: 4px 0; padding: 10px 8px; }
|
||||
.nav-link.active { background: var(--nav-active-bg); border-radius: 6px; margin: 4px 0; padding: 10px clamp(8px, 0.6vw + 4px, 14px); }
|
||||
}
|
||||
|
||||
/* === Nav narrow-desktop tightening (Issue #1055 — 1024px overlap fix) ===
|
||||
Between 768px and ~1100px, even with Priority+ collapsed to 5 high
|
||||
links + "More ▾" + 4 nav-right buttons + nav-stats, the row doesn't
|
||||
fit and the rightmost visible link overlaps nav-right by ~20px.
|
||||
Hide nav-stats and tighten nav-link padding/gap in this band;
|
||||
nav-stats reappears at ≥1101px where there is room for it.
|
||||
NOTE: this block MUST follow the (min-width: 768px) Priority+ block
|
||||
above so its tighter padding/gap rules win the cascade by source
|
||||
order at 1024px (same specificity). */
|
||||
@media (min-width: 768px) and (max-width: 1100px) {
|
||||
.nav-stats { display: none; }
|
||||
.nav-links { gap: 0; }
|
||||
.nav-link { padding: 14px 6px; }
|
||||
.nav-link.active { padding: 10px 6px; }
|
||||
.nav-right { gap: 4px; }
|
||||
}
|
||||
|
||||
/* === Responsive — Hamburger nav (<768px) === */
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
#!/usr/bin/env node
|
||||
/* Issue #1055 — Nav fluid Priority+ adaptation at all widths.
|
||||
*
|
||||
* Asserts the top-nav never overflows the viewport at common widths:
|
||||
* the right edge of `.nav-right` MUST be ≤ document.documentElement.clientWidth.
|
||||
*
|
||||
* Pre-fix behavior: the Priority+ collapse rule was scoped to
|
||||
* `(min-width: 768px) and (max-width: 1279px)`, so at 1280/1440/1920 the
|
||||
* full link strip + nav-stats + nav-right buttons could push past the
|
||||
* viewport's right edge (no collapse happened above 1279px).
|
||||
*
|
||||
* Post-fix: Priority+ collapses at all widths >=768px when needed.
|
||||
*
|
||||
* Run against a CoreScope server (defaults to localhost:13581 with the
|
||||
* E2E fixture DB, matching the playwright job in .github/workflows/deploy.yml).
|
||||
*
|
||||
* CI gating: when CHROMIUM_REQUIRE=1 (set by the GH Actions workflow) a
|
||||
* missing/broken Chromium is a HARD FAIL — no soft-skip. Locally the
|
||||
* test is allowed to skip so devs without Playwright browsers installed
|
||||
* can still run other tests.
|
||||
*/
|
||||
'use strict';
|
||||
|
||||
const { chromium } = require('playwright');
|
||||
|
||||
const BASE = process.env.BASE_URL || 'http://localhost:13581';
|
||||
// Common widths the nav must stay clean at. 1280/1440 are the historic
|
||||
// failure window: the Priority+ rule used to stop at 1279px but the full
|
||||
// link strip + nav-right buttons don't fit on one row until ~1600px+.
|
||||
const VIEWPORTS = [768, 1024, 1280, 1440, 1920];
|
||||
// Routes asserted at every viewport. The pre-#1097 version only checked
|
||||
// /#/home, but the bug reproduces on every top-level page since they
|
||||
// all share the same .top-nav. Cover the four primary routes.
|
||||
const ROUTES = ['/#/home', '/#/packets', '/#/nodes', '/#/map'];
|
||||
const HEIGHT = 900;
|
||||
// Whitespace tolerance (px) for the overflow/overlap assertions.
|
||||
// Browsers occasionally hand back layout coordinates with sub-pixel
|
||||
// rounding noise (≈0.1–0.4px) even when the box model is clean. We
|
||||
// allow up to 0.5px so the test doesn't false-fail on rounding while
|
||||
// still catching real overlaps (the bug this guards against was
|
||||
// ~20px). Tighter than 0.5 caused intermittent CI flakes; looser
|
||||
// would risk masking 1px regressions.
|
||||
const SUBPIXEL_TOL = 0.5;
|
||||
|
||||
async function main() {
|
||||
const requireChromium = process.env.CHROMIUM_REQUIRE === '1' ||
|
||||
process.env.NAV_FLUID_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-nav-fluid-1055-e2e.js: FAIL — Chromium required (CHROMIUM_REQUIRE=1) but unavailable: ${err.message}`);
|
||||
process.exit(1);
|
||||
}
|
||||
console.log(`test-nav-fluid-1055-e2e.js: SKIP (Chromium unavailable: ${err.message.split('\n')[0]})`);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
let failures = 0;
|
||||
let passes = 0;
|
||||
const context = await browser.newContext();
|
||||
const page = await context.newPage();
|
||||
page.setDefaultTimeout(15000);
|
||||
|
||||
for (const route of ROUTES) {
|
||||
for (const w of VIEWPORTS) {
|
||||
await page.setViewportSize({ width: w, height: HEIGHT });
|
||||
await page.goto(`${BASE}${route}`, { waitUntil: 'domcontentloaded' });
|
||||
await page.waitForSelector('.top-nav .nav-right');
|
||||
// Wait for fonts (which affect text measurement) AND for the nav
|
||||
// layout to settle: the .nav-right bounding box must hold steady
|
||||
// for two consecutive animation frames at the same coordinates.
|
||||
// This replaces a magic 150ms sleep with a deterministic gate
|
||||
// that asserts what we actually care about (layout has stopped
|
||||
// moving) before measuring.
|
||||
await page.evaluate(() => document.fonts && document.fonts.ready ? document.fonts.ready : null);
|
||||
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 && r1.top === r2.top);
|
||||
}));
|
||||
});
|
||||
}, null, { timeout: 5000 });
|
||||
|
||||
const data = await page.evaluate(() => {
|
||||
const navRight = document.querySelector('.top-nav .nav-right');
|
||||
const navLeft = document.querySelector('.top-nav .nav-left');
|
||||
const topNav = document.querySelector('.top-nav');
|
||||
const more = document.querySelector('.nav-more-wrap');
|
||||
const moreCs = more ? getComputedStyle(more) : null;
|
||||
const links = Array.from(document.querySelectorAll('.nav-links .nav-link'));
|
||||
const visible = links.filter(a => getComputedStyle(a).display !== 'none');
|
||||
const lastVisible = visible[visible.length - 1] || null;
|
||||
return {
|
||||
clientW: document.documentElement.clientWidth,
|
||||
navScroll: topNav.scrollWidth,
|
||||
navClient: topNav.clientWidth,
|
||||
navRight: navRight.getBoundingClientRect().right,
|
||||
navRightL: navRight.getBoundingClientRect().left,
|
||||
navLeftR: navLeft.getBoundingClientRect().right,
|
||||
lastLinkR: lastVisible ? lastVisible.getBoundingClientRect().right : -1,
|
||||
moreVisible: moreCs ? moreCs.display !== 'none' : false,
|
||||
visibleLinks: visible.length,
|
||||
totalLinks: links.length,
|
||||
};
|
||||
});
|
||||
|
||||
const tag = `${route} @ ${w}px`;
|
||||
const reasons = [];
|
||||
// 1. .nav-right must not extend past the viewport's right edge.
|
||||
if (data.navRight > data.clientW + SUBPIXEL_TOL) {
|
||||
reasons.push(`nav-right.right=${data.navRight.toFixed(1)} > clientWidth=${data.clientW} ` +
|
||||
`(excess ${(data.navRight - data.clientW).toFixed(1)}px)`);
|
||||
}
|
||||
// 2. The visible link strip must not overlap .nav-right (parent overflow:hidden
|
||||
// masks this visually but it still hides the rightmost links — the actual bug).
|
||||
if (data.lastLinkR > data.navRightL + SUBPIXEL_TOL) {
|
||||
reasons.push(`last visible link right=${data.lastLinkR.toFixed(1)} > nav-right.left=${data.navRightL.toFixed(1)} ` +
|
||||
`(${(data.lastLinkR - data.navRightL).toFixed(1)}px overlap)`);
|
||||
}
|
||||
// 3. The nav row itself must not require horizontal scrolling.
|
||||
if (data.navScroll > data.navClient + SUBPIXEL_TOL) {
|
||||
reasons.push(`top-nav scrollWidth=${data.navScroll} > clientWidth=${data.navClient}`);
|
||||
}
|
||||
|
||||
if (reasons.length === 0) {
|
||||
passes++;
|
||||
console.log(` ✅ ${tag}: clean (visible links ${data.visibleLinks}/${data.totalLinks}, more=${data.moreVisible})`);
|
||||
} else {
|
||||
failures++;
|
||||
console.log(` ❌ ${tag}: ${reasons.join(' | ')} ` +
|
||||
`(visible links ${data.visibleLinks}/${data.totalLinks}, more=${data.moreVisible})`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await browser.close();
|
||||
|
||||
const total = ROUTES.length * VIEWPORTS.length;
|
||||
console.log(`\ntest-nav-fluid-1055-e2e.js: ${failures === 0 ? 'OK' : 'FAIL'} — ${passes}/${total} passed`);
|
||||
process.exit(failures === 0 ? 0 : 1);
|
||||
}
|
||||
|
||||
main().catch((err) => {
|
||||
console.error('test-nav-fluid-1055-e2e.js: fatal', err);
|
||||
process.exit(1);
|
||||
});
|
||||
Reference in new issue
Block a user