mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(mobile): hide attachments-history button on phones + regression guards
The COD-39 attachment-history header button was visible on the cramped phone header. Hide it on phones alongside the settings gear and lifecycle log (the mobile header is intentionally minimal — those controls live in the toolbar). One-line addition to the existing @media (max-width: 430px) display:none block in mobile.css. This is the second time a header control leaked onto mobile (the plan-usage chip was the first), so add two regression guards: - test/mobile-header-buttons-policy.test.ts — a pure static analysis of index.html + mobile.css (no browser), so it runs in the normal CI sweep (the test/mobile/** Playwright suite is EXCLUDED from CI and never gated this). It enumerates every default-visible header button and fails when one has no phone-visibility decision — either a mobile.css hide rule or an explicit MOBILE_VISIBLE_ALLOWLIST entry. A new header button now forces that decision. Verified it fails on the pre-fix state and passes after. - test/mobile/header-buttons.test.ts — real-browser E2E in the mobile suite: asserts the attachments/settings/lifecycle buttons are hidden on an emulated iPhone 14 Pro and the attachments button is visible on a desktop-class tablet. tsc + lint + prettier + both new tests green. Only CSS + tests changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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<string>([]);
|
||||
|
||||
// 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 <div class="header-right" id="headerRight"> … </header>.
|
||||
const region = html.match(/<div class="header-right"[^>]*>([\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(/<button\b([^>]*)>/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<string> {
|
||||
const css = readFileSync(join(PUBLIC, 'mobile.css'), 'utf-8');
|
||||
const hidden = new Set<string>();
|
||||
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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user