From 7e492a71a0fd2b4d087bc9a032009e654cceef6a Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Tue, 26 May 2026 11:07:17 -0700 Subject: [PATCH] =?UTF-8?q?fix(#1400):=20root=20cause=20of=20recurring=20n?= =?UTF-8?q?av-vanishing=20=E2=80=94=20min-height:48px=20overflowed=2052px?= =?UTF-8?q?=20top-nav,=20clipped=20link=20strip=20above=20viewport=20(#140?= =?UTF-8?q?1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **RED commit phase** — TDD failing test for #1400. Green fix incoming next push. See full PR body on ready-for-review. Fixes #1400 --------- Co-authored-by: openclaw-bot --- .github/workflows/deploy.yml | 1 + public/style.css | 15 ++- test-issue-1400-nav-vertical-clip.js | 176 +++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 test-issue-1400-nav-vertical-clip.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 85664485..c7b6e7ec 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -266,6 +266,7 @@ jobs: 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-issue-1400-nav-vertical-clip.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/style.css b/public/style.css index b475d958..b260a476 100644 --- a/public/style.css +++ b/public/style.css @@ -289,8 +289,16 @@ a:focus-visible, button:focus-visible, input:focus-visible, select:focus-visible all interactive controls. Targets are achieved with min-height/min-width plus inline-flex centering so existing visual styling (font-size, padding, icon size) is preserved on desktop while the *hit area* grows for touch. - Issue #1060. */ -.nav-link { min-height: 48px; display: inline-flex; align-items: center; } + Issue #1060. + + Issue #1400: the global `min-height: 48px` on `.nav-link` was the root + cause of ~20 recurring nav-vanishing bugs (#1391, #1396, and ~18 earlier + symptoms). 48px links + padding inflated `.nav-links` to 56px tall inside + a 52px `.top-nav` with `overflow:hidden`; Firefox flex-centered the + over-tall item to a negative y, clipping the entire link strip ABOVE the + viewport. Touch-target sizing is preserved for mobile via the + `@media (max-width: 767px)` override added near the hamburger block. */ +.nav-link { display: inline-flex; align-items: center; } /* Generic button surfaces — filter bar, modal buttons, inline .btn usages. inline-flex keeps text/icons centered without changing visible padding much. */ @@ -1797,6 +1805,9 @@ button.ch-item:hover .ch-icon-btn { opacity: 1; } .nav-links a:not([data-priority="high"]) { display: flex; } .nav-links.open { display: flex; } .nav-link { padding: 12px 20px; border-bottom: none; } + /* Issue #1400: restore 48px touch-target on mobile only. Global rule + removed because it overflowed the 52px desktop top-nav. */ + .nav-link { min-height: 48px; } .nav-link.active { background: var(--nav-active-bg); border-radius: 0; margin: 0; padding: 12px 20px; } .nav-left { gap: 12px; } body.nav-open { overflow: hidden; } diff --git a/test-issue-1400-nav-vertical-clip.js b/test-issue-1400-nav-vertical-clip.js new file mode 100644 index 00000000..3d8c5667 --- /dev/null +++ b/test-issue-1400-nav-vertical-clip.js @@ -0,0 +1,176 @@ +#!/usr/bin/env node +/* Issue #1400 — root cause of recurring nav-vanishing class of bugs. + * + * Symptom: at desktop viewports (1024..1711), the `.nav-links` strip + * rendered at NEGATIVE y (operator probe: y=-57, height=56), entirely + * above the visible 0..52 band of `.top-nav` which has `overflow:hidden`. + * + * Root cause: PR #1060 (commit eaf14a61) added a global + * .nav-link { min-height: 48px; display:inline-flex; align-items:center; } + * The 48px link + padding inflated `.nav-links` to 56px tall inside a 52px + * `.top-nav` with `overflow:hidden`. With `align-items: center`, Firefox + * centers the over-tall flex item at a negative y → strip clipped above + * viewport. + * + * Acceptance (from #1400): + * - Desktop: `.nav-links` rect.y >= 0 AND every `.nav-links > a` is + * vertically inside the visible top-nav band (y >= 0 AND y+height <= 60). + * - Mobile (<768px): touch-target preserved — `.nav-link` min-height + * computed style >= 48px (regression guard for #1060). + * + * Mutation guard: re-adding `min-height: 48px` to global `.nav-link` + * must make this test fail with negative y at desktop widths. + */ +'use strict'; + +const assert = require('node:assert'); +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; + +const DESKTOP_WIDTHS = [1024, 1366, 1711]; +const MOBILE_WIDTH = 480; +const HEIGHT = 800; +const TOPNAV_HEIGHT_MAX = 60; // 52px nominal + a few px slack + +async function settleNav(page) { + await page.waitForSelector('.top-nav .nav-links'); + await page.evaluate(() => document.fonts && document.fonts.ready ? document.fonts.ready : null); + await page.waitForFunction(() => { + const el = document.querySelector('.top-nav .nav-links'); + if (!el) return false; + const r1 = el.getBoundingClientRect(); + return new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => { + const r2 = el.getBoundingClientRect(); + resolve(r1.top === r2.top && r1.height === r2.height); + })); + }); + }); +} + +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-issue-1400-nav-vertical-clip.js: FAIL — Chromium required but unavailable: ${err.message}`); + process.exit(1); + } + console.log(`test-issue-1400-nav-vertical-clip.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); + + // === Desktop: vertical clip guard === + for (const w of DESKTOP_WIDTHS) { + await page.setViewportSize({ width: w, height: HEIGHT }); + await page.goto(`${BASE}/#/home`, { waitUntil: 'domcontentloaded' }); + await settleNav(page); + + const probe = await page.evaluate(() => { + const nav = document.querySelector('.top-nav'); + const links = document.querySelector('.nav-links'); + const anchors = Array.from(document.querySelectorAll('.nav-links > a')); + const r = (el) => { + if (!el) return null; + const b = el.getBoundingClientRect(); + return { y: b.y, height: b.height, bottom: b.y + b.height }; + }; + return { + nav: r(nav), + links: r(links), + anchors: anchors.map((a) => ({ href: a.getAttribute('href'), ...r(a) })), + }; + }); + + const tag = `vw=${w}`; + if (!probe.links) { + console.error(`FAIL ${tag}: .nav-links not found`); + failures++; + continue; + } + + try { + assert.ok( + probe.links.y >= 0, + `${tag}: .nav-links y=${probe.links.y} must be >= 0 (issue #1400 root-cause regression: clipped above viewport)`, + ); + assert.ok( + probe.anchors.length > 0, + `${tag}: expected >=1 .nav-links > a, got 0`, + ); + for (const a of probe.anchors) { + assert.ok( + a.y >= 0, + `${tag}: nav-link href=${a.href} y=${a.y} must be >= 0`, + ); + assert.ok( + a.bottom <= TOPNAV_HEIGHT_MAX, + `${tag}: nav-link href=${a.href} bottom=${a.bottom} must be <= ${TOPNAV_HEIGHT_MAX} (overflowing 52px top-nav)`, + ); + } + console.log(`PASS ${tag}: .nav-links y=${probe.links.y.toFixed(1)} h=${probe.links.height.toFixed(1)}; ${probe.anchors.length} anchors all inside top-nav band`); + passes++; + } catch (err) { + console.error(`FAIL ${tag}: ${err.message}`); + console.error(` probe: ${JSON.stringify(probe)}`); + failures++; + } + } + + // === Mobile: touch-target preserved (#1060 regression guard) === + await page.setViewportSize({ width: MOBILE_WIDTH, height: HEIGHT }); + await page.goto(`${BASE}/#/home`, { waitUntil: 'domcontentloaded' }); + // open hamburger so .nav-link is rendered (display:none otherwise on mobile until .open) + await page.evaluate(() => { + const links = document.querySelector('.nav-links'); + if (links) links.classList.add('open'); + }); + await page.waitForTimeout(50); + + const mobileProbe = await page.evaluate(() => { + const anchors = Array.from(document.querySelectorAll('.nav-links > a')); + return anchors.slice(0, 3).map((a) => { + const cs = getComputedStyle(a); + return { href: a.getAttribute('href'), minHeight: parseFloat(cs.minHeight) || 0 }; + }); + }); + + const tag = `vw=${MOBILE_WIDTH}`; + try { + assert.ok(mobileProbe.length > 0, `${tag}: expected mobile nav-links anchors, got 0`); + for (const a of mobileProbe) { + assert.ok( + a.minHeight >= 48, + `${tag}: nav-link href=${a.href} min-height=${a.minHeight} must be >= 48 (touch-target regression of #1060)`, + ); + } + console.log(`PASS ${tag}: mobile .nav-link min-height >= 48 (touch-target preserved per #1060)`); + passes++; + } catch (err) { + console.error(`FAIL ${tag}: ${err.message}`); + console.error(` probe: ${JSON.stringify(mobileProbe)}`); + failures++; + } + + await browser.close(); + + console.log(`\ntest-issue-1400-nav-vertical-clip.js: ${passes} passed, ${failures} failed`); + if (failures > 0) process.exit(1); +} + +main().catch((err) => { + console.error('test-issue-1400-nav-vertical-clip.js: ERROR', err); + process.exit(1); +});