diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index 146fbacd..9ace1aaf 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -434,9 +434,12 @@ html.mobile-init .file-browser-panel { height: 12px; } - /* Hide header settings gear and lifecycle log on mobile - settings moved to toolbar */ + /* Hide header settings gear and lifecycle log on mobile - settings moved to toolbar. + Also hide the attachment-history button: the header is kept minimal on phones and + the drawer is a desktop-oriented panel. */ .btn-icon-header.btn-settings, - .btn-icon-header.btn-lifecycle-log { + .btn-icon-header.btn-lifecycle-log, + .btn-icon-header.btn-attachments-history { display: none !important; } diff --git a/test/mobile-header-buttons-policy.test.ts b/test/mobile-header-buttons-policy.test.ts new file mode 100644 index 00000000..133014a2 --- /dev/null +++ b/test/mobile-header-buttons-policy.test.ts @@ -0,0 +1,143 @@ +// Port: none (pure static analysis — runs in CI, no browser/server). +// +// Regression guard for "a header button leaks onto the cramped mobile header". +// This exact class of bug shipped twice (the plan-usage chip, then the COD-39 +// attachments-history button): a new control was added to the header, looked +// fine on desktop, and nobody noticed it cluttering the phone header. The mobile +// Playwright suite that *would* catch it (test/mobile/**) is EXCLUDED from CI, so +// it never gated. This test is intentionally a pure parser of index.html + +// mobile.css so it runs in the normal CI sweep with zero browser dependencies. +// +// Policy: every header button that is VISIBLE BY DEFAULT on desktop must have an +// explicit decision for phones — either it's hidden via an @media (max-width: +// 430px) display:none rule in mobile.css, or it's added to MOBILE_VISIBLE_ALLOWLIST +// below with a reason. A new default-visible header button with neither fails this +// test, forcing the author to decide its mobile behavior. +// +// The real-browser counterpart (actual computed visibility on an emulated phone) +// lives in test/mobile/header-buttons.test.ts. +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { join } from 'node:path'; +import postcss from 'postcss'; + +const HERE = fileURLToPath(new URL('.', import.meta.url)); +const PUBLIC = join(HERE, '../src/web/public'); + +// Canonical phone width used to decide whether a media query applies on a phone. +// Matches the device the browser-based test emulates (iPhone 14 Pro = 393px CSS). +const PHONE_WIDTH = 393; + +// Header buttons intentionally kept VISIBLE in the phone header. Empty today: the +// mobile header is deliberately minimal and essential controls (settings, case) +// live in the toolbar. Add a class here ONLY with a justifying comment. +const MOBILE_VISIBLE_ALLOWLIST = new Set([]); + +// Buttons we expect to STAY hidden on phones — an explicit lock so a future edit +// that removes a hide rule fails loudly (not silently). +const KNOWN_PHONE_HIDDEN = ['btn-attachments-history', 'btn-settings', 'btn-lifecycle-log']; + +function attrOf(openTag: string, name: string): string { + const m = openTag.match(new RegExp(`${name}="([^"]*)"`)); + return m ? m[1] : ''; +} + +/** Does a media query's width range include a phone-width viewport? */ +function appliesToPhone(params: string): boolean { + const max = params.match(/max-width:\s*(\d+)px/); + const min = params.match(/min-width:\s*(\d+)px/); + if (max && PHONE_WIDTH > Number(max[1])) return false; // phone is wider than the cap + if (min && PHONE_WIDTH < Number(min[1])) return false; // phone is narrower than the floor + return Boolean(max || min); // must actually be a width-bounded query +} + +function loadHeaderButtons(): { classes: string[]; id: string; distinguishing: string[] }[] { + const html = readFileSync(join(PUBLIC, 'index.html'), 'utf-8'); + // The header controls live in
… . + const region = html.match(/
]*>([\s\S]*?)<\/header>/); + expect(region, '#headerRight region not found in index.html — update the selector in this test').toBeTruthy(); + const headerHtml = region![1]; + + return [...headerHtml.matchAll(/]*)>/g)] + .map((m) => { + const open = m[1]; + const classes = attrOf(open, 'class').split(/\s+/).filter(Boolean); + return { + classes, + id: attrOf(open, 'id'), + style: attrOf(open, 'style'), + distinguishing: classes.filter( + (c) => c.startsWith('btn-') && c !== 'btn-icon-header' && c !== 'btn-sm' && !c.endsWith('--hidden') + ), + }; + }) + .filter((b) => b.classes.includes('btn-icon-header')) + .filter((b) => !b.classes.includes('btn-sm')) // font A-/A+ controls — separate sub-group + .filter((b) => !/display:\s*none/i.test(b.style)) // JS-gated (solo-redock, retired bell) + .filter((b) => !b.classes.some((c) => c.endsWith('--hidden'))); // opt-in, hidden by default +} + +function loadPhoneHiddenClasses(): Set { + const css = readFileSync(join(PUBLIC, 'mobile.css'), 'utf-8'); + const hidden = new Set(); + postcss.parse(css).walkAtRules('media', (atRule) => { + if (!appliesToPhone(atRule.params)) return; + atRule.walkRules((rule) => { + let hides = false; + rule.walkDecls('display', (decl) => { + if (decl.value.replace(/!important/i, '').trim() === 'none') hides = true; + }); + if (!hides) return; + for (const token of rule.selector.match(/\.btn-[a-z0-9-]+/gi) || []) { + hidden.add(token.slice(1).toLowerCase()); + } + }); + }); + return hidden; +} + +describe('Mobile header button policy (static guard)', () => { + const buttons = loadHeaderButtons(); + const phoneHidden = loadPhoneHiddenClasses(); + + it('finds the default-visible header buttons (sanity)', () => { + // If this drops to 0 the parser/markup drifted — fix the parser, don't delete the test. + expect(buttons.length).toBeGreaterThan(0); + }); + + it('every default-visible header button has a mobile-visibility decision', () => { + for (const btn of buttons) { + expect( + btn.distinguishing.length, + `Header button (id=${btn.id || '?'}, class="${btn.classes.join(' ')}") has no distinguishing ` + + `btn-* class to target on mobile. Give it one so its phone visibility can be controlled.` + ).toBeGreaterThan(0); + + const hidden = btn.distinguishing.some((c) => phoneHidden.has(c.toLowerCase())); + const allowed = btn.distinguishing.some((c) => MOBILE_VISIBLE_ALLOWLIST.has(c)); + + expect( + hidden || allowed, + `Header button .${btn.distinguishing.join('.')} (id=${btn.id || '?'}) is VISIBLE BY DEFAULT but has ` + + `no mobile-visibility decision.\n` + + ` → To hide it on phones: add it to the @media (max-width: 430px) "display: none" block in ` + + `src/web/public/mobile.css (next to .btn-settings / .btn-lifecycle-log).\n` + + ` → To keep it visible on phones: add '${btn.distinguishing[0]}' to MOBILE_VISIBLE_ALLOWLIST in ` + + `this test, with a reason.\n` + + `This guard exists because the plan-usage chip and the attachments button both leaked onto the ` + + `mobile header unnoticed.` + ).toBe(true); + } + }); + + it('locks the known phone-hidden header buttons', () => { + for (const cls of KNOWN_PHONE_HIDDEN) { + expect( + phoneHidden.has(cls), + `${cls} must stay hidden on phones — restore its rule in the @media (max-width: 430px) ` + + `display:none block in src/web/public/mobile.css.` + ).toBe(true); + } + }); +}); diff --git a/test/mobile/header-buttons.test.ts b/test/mobile/header-buttons.test.ts new file mode 100644 index 00000000..753c25a7 --- /dev/null +++ b/test/mobile/header-buttons.test.ts @@ -0,0 +1,54 @@ +// Port 3208 - Mobile header button visibility (real-browser E2E). +// +// Companion to the CI static guard (test/mobile-header-buttons-policy.test.ts). +// This one renders the real app in an emulated phone vs. a desktop-class tablet +// and asserts the ACTUAL computed visibility — catching CSS/layout regressions +// the static parser can't see. Regression history: the COD-39 attachments button +// (and earlier the plan-usage chip) shipped visible on the cramped phone header. +import { describe, it, beforeAll, afterAll } from 'vitest'; +import type { WebServer } from '../../src/web/server.js'; +import { createTestServer, stopTestServer } from './helpers/server.js'; +import { createDevicePage, closeAllBrowsers } from './helpers/browser.js'; +import { assertHidden, assertVisible } from './helpers/assertions.js'; +import { REPRESENTATIVE_DEVICES } from './devices.js'; +import { PORTS, WAIT } from './helpers/constants.js'; + +const PORT = PORTS.HEADER_BUTTONS; +const BASE_URL = `http://localhost:${PORT}`; + +// Secondary header buttons that must NOT clutter the minimal phone header +// (settings/case controls live in the mobile toolbar instead). +const PHONE_HIDDEN = [ + '#attachmentsHistoryBtn', // COD-39 — the reported regression + '.btn-icon-header.btn-settings', + '.btn-icon-header.btn-lifecycle-log', +]; + +describe('Mobile header button visibility (E2E)', () => { + let server: WebServer; + + beforeAll(async () => { + server = await createTestServer(PORT); + }); + + afterAll(async () => { + await closeAllBrowsers(); + await stopTestServer(server); + }); + + it('hides secondary header buttons on a standard phone (iPhone 14 Pro, 393px)', async () => { + const { page } = await createDevicePage(REPRESENTATIVE_DEVICES['standard-phone'], BASE_URL); + await page.waitForTimeout(WAIT.PAGE_SETTLE); + for (const sel of PHONE_HIDDEN) { + await assertHidden(page, sel); + } + }); + + it('keeps the attachments button visible on a desktop-class tablet', async () => { + // assertHidden also passes when an element is ABSENT, so prove the selector is + // real by asserting the same button IS visible where the phone rule doesn't apply. + const { page } = await createDevicePage(REPRESENTATIVE_DEVICES['large-tablet'], BASE_URL); + await page.waitForTimeout(WAIT.PAGE_SETTLE); + await assertVisible(page, '#attachmentsHistoryBtn'); + }); +}); diff --git a/test/mobile/helpers/constants.ts b/test/mobile/helpers/constants.ts index 5628bd49..b2dda4d0 100644 --- a/test/mobile/helpers/constants.ts +++ b/test/mobile/helpers/constants.ts @@ -8,6 +8,7 @@ export const PORTS = { DEVICE_MATRIX: 3205, VISUAL_REGRESSION: 3206, ACCESSIBILITY: 3207, + HEADER_BUTTONS: 3208, } as const; // CSS Selectors @@ -64,8 +65,8 @@ export const BREAKPOINTS = { // Keyboard constants (match app.js KeyboardHandler) export const KEYBOARD = { - SHOW_THRESHOLD: 150, // heightDiff > 150px triggers show - HIDE_THRESHOLD: 100, // heightDiff < 100px triggers hide + SHOW_THRESHOLD: 150, // heightDiff > 150px triggers show + HIDE_THRESHOLD: 100, // heightDiff < 100px triggers hide TYPICAL_IOS_HEIGHT: 336, FOCUSIN_DELAY: 400, ANIMATION_DELAY: 150, @@ -87,7 +88,7 @@ export const SWIPE = { export const SUBAGENT = { MOBILE_CARD_HEIGHT: 110, MOBILE_CARD_GAP: 4, - MOBILE_CARD_STRIDE: 114, // height + gap + MOBILE_CARD_STRIDE: 114, // height + gap TOOLBAR_OFFSET: 40, DEFAULT_HEADER_HEIGHT: 36, } as const;