From 565ec8044a61173d27a68dcb3fd753d5b216405e Mon Sep 17 00:00:00 2001 From: Openclaw Date: Wed, 23 Sep 2026 16:22:54 +0200 Subject: [PATCH 1/3] fix(css): unify compact touch target sizes --- .github/workflows/deploy.yml | 1 + public/style.css | 15 ++++---- test-all.sh | 1 + test-issue-2052-touch-target-css.js | 54 +++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 test-issue-2052-touch-target-css.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 108d2b970..4b136230d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -112,6 +112,7 @@ jobs: node test-issue-1518-home-url.js node test-channel-decrypt-insecure-context.js node test-live-region-filter.js + node test-issue-2052-touch-target-css.js node test-issue-1136-observer-iata-map.js node test-channel-qr.js node test-channel-qr-wiring.js diff --git a/public/style.css b/public/style.css index 0b6067fb4..a9cf7e7d7 100644 --- a/public/style.css +++ b/public/style.css @@ -518,8 +518,10 @@ a:focus-visible, button:focus-visible, input:focus-visible, select:focus-visible } /* === Touch Targets === */ -/* WCAG 2.5.5 / Apple HIG / Material: 48x48 CSS px minimum touch target for - all interactive controls. Targets are achieved with min-height/min-width +/* WCAG 2.5.5 and Apple HIG use a 44x44 CSS px target; Material uses 48x48. + Most controls in this section use the 48x48 house preference. Compact + .nav-btn and .ch-icon-btn controls retain 44x44 in their component rules. + 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. @@ -558,13 +560,6 @@ a:focus-visible, button:focus-visible, input:focus-visible, select:focus-visible touch-action: manipulation; } -.nav-btn { - min-height: 48px; - min-width: 48px; - touch-action: manipulation; -} - -.ch-icon-btn, .ch-remove-btn, .ch-share-btn { min-height: 48px; @@ -891,6 +886,7 @@ img.brand-logo { 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; min-width: 44px; min-height: 44px; display: inline-flex; align-items: center; justify-content: center; + touch-action: manipulation; } .nav-btn:hover { background: var(--nav-bg2); color: var(--nav-text); } @@ -1870,6 +1866,7 @@ button.ch-item.selected { background: var(--selected-bg); } background: none; border: none; color: var(--text-muted); cursor: pointer; padding: 4px; margin-left: 2px; min-width: 44px; min-height: 44px; box-sizing: border-box; + touch-action: manipulation; opacity: 0.55; transition: opacity 0.15s, color 0.15s; line-height: 1; user-select: none; } diff --git a/test-all.sh b/test-all.sh index 0e63d460f..868f0609e 100755 --- a/test-all.sh +++ b/test-all.sh @@ -57,6 +57,7 @@ node test-issue-1883-redirect-history.js node test-issue-1890-og-url.js node test-traces.js node test-live-multibyte-filter.js +node test-issue-2052-touch-target-css.js # #1418 — route-view v2 (Tufte) coverage node test-issue-1418-raw-hex-extraction.js diff --git a/test-issue-2052-touch-target-css.js b/test-issue-2052-touch-target-css.js new file mode 100644 index 000000000..d0a107fa3 --- /dev/null +++ b/test-issue-2052-touch-target-css.js @@ -0,0 +1,54 @@ +#!/usr/bin/env node +/* Issue #2052 — keep the nav and channel icon touch-target policy + * unambiguous. Both controls render at 44x44 CSS px today, but earlier + * duplicate 48px declarations made the source disagree with the cascade. + * + * This test reads the shipped stylesheet and requires one consistent value + * across each selector's base declarations. Pseudo-class rules such as + * :hover/:active are excluded because they do not define target dimensions. + */ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const css = fs.readFileSync(path.join(__dirname, 'public', 'style.css'), 'utf8') + .replace(/\/\*[\s\S]*?\*\//g, ''); + +function policyDeclarations(selector) { + const declarations = []; + const rulePattern = /([^{}]+)\{([^{}]*)\}/g; + let match; + + while ((match = rulePattern.exec(css)) !== null) { + const selectors = match[1].split(',').map((part) => part.trim()); + if (!selectors.includes(selector)) continue; + + for (const property of ['min-width', 'min-height', 'touch-action']) { + const propertyPattern = new RegExp(`(?:^|;)\\s*${property}\\s*:\\s*([^;]+)`, 'g'); + let propertyMatch; + while ((propertyMatch = propertyPattern.exec(match[2])) !== null) { + declarations.push({ property, value: propertyMatch[1].trim() }); + } + } + } + + return declarations; +} + +for (const selector of ['.nav-btn', '.ch-icon-btn']) { + const declarations = policyDeclarations(selector); + const valuesByProperty = Object.fromEntries(['min-width', 'min-height', 'touch-action'].map((property) => [ + property, + [...new Set(declarations.filter((entry) => entry.property === property).map((entry) => entry.value))], + ])); + assert.deepStrictEqual( + valuesByProperty, + { 'min-width': ['44px'], 'min-height': ['44px'], 'touch-action': ['manipulation'] }, + `${selector} must have one unambiguous 44px touch-target policy; got ${JSON.stringify(declarations)}` + ); + console.log(`PASS ${selector}: one consistent 44x44 minimum`); +} + +console.log('test-issue-2052-touch-target-css.js: OK'); From dba38456d4a14e0ebefe9e4e95b082f3e3e36dff Mon Sep 17 00:00:00 2001 From: Openclaw Date: Thu, 24 Sep 2026 20:12:34 +0200 Subject: [PATCH 2/3] fix(css): drop the dead 32px channel-action rule; test the rendered targets Review found the #2052 policy test was falsely green. It matched only the exact selector ".ch-icon-btn". Inside @media (max-width: 640px), the more specific "#chList .ch-icon-btn { min-width: 32px; min-height: 32px }" set the channel share/remove actions to 32x32. Measured in the real app, that rule never applies. Below 768px channels.js renders the flat .ch-row list (#1367), which has no inline share/remove actions, and it re-renders when the viewport crosses that width. So the rule contradicted the 44x44 policy without ever affecting a control. It is removed, and the comment now states the responsive contract. test-issue-2052-touch-target-css.js now checks every selector anywhere in the stylesheet whose last compound targets .nav-btn, .ch-icon-btn, .ch-share-btn or .ch-remove-btn, including more specific selectors and @media rules. Each min-width/min-height must be at least 44px. It stays a supplement to the browser test. test-issue-2052-touch-target-e2e.js measures the real responsive DOM for a user-added channel (saved key; no injected DOM): - desktop 1440x900: share and remove are at least 44x44, visible, not clipped by any overflow container, hit at their centre, not overlapping each other or the row, and the page has no horizontal overflow. They keep their ARIA/title contract and are reached by Tab with focus-visible. Enter opens the share modal; remove asks for confirmation, which is dismissed, so nothing is removed. - mobile 390x844 touch: the channel renders as a .ch-row with no share/remove actions, every visible tap target is at least 44x44, and tapping the row opens the channel. - tablet 768x1024: characterization only. The actions are clipped by the narrow sidebar there, a separate layout issue also present on master; the step logs the measurement and does not gate. The E2E test is registered once in the Playwright step. This change makes no layout change. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy.yml | 1 + public/style.css | 13 +- test-issue-2052-touch-target-css.js | 94 ++++++---- test-issue-2052-touch-target-e2e.js | 260 ++++++++++++++++++++++++++++ 4 files changed, 329 insertions(+), 39 deletions(-) create mode 100644 test-issue-2052-touch-target-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 4b136230d..19f1ad900 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -479,6 +479,7 @@ jobs: CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1657-analytics-channels-group-sprites-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1224-channels-mobile-ux-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1367-channels-chat-app-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-2052-touch-target-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1236-map-mobile-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1329-map-controls-accordion-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1273-qr-overlay-height-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/style.css b/public/style.css index a9cf7e7d7..7cf997e50 100644 --- a/public/style.css +++ b/public/style.css @@ -2461,19 +2461,16 @@ button.ch-item:hover .ch-icon-btn { opacity: 1; } .ch-header-region .region-pill { padding: 2px 6px; font-size: 11px; } .ch-header-region .region-dropdown-trigger { padding: 3px 6px; font-size: 11px; } - /* Channel list row: name must dominate. The inline share/remove icons - were eating >88px because of their 44x44 touch target — push them - onto a per-row reveal pattern: show a single ⋮ trigger that toggles - the action set. Until then, shrink them to icon-only and let the - name grow via flex:1. */ + /* Channel list row: name must dominate. Below 768px channels.js renders + the flat .ch-row list (#1367), which has no inline share/remove + actions, so there is no size override for .ch-icon-btn here: wherever + those actions render they keep the 44x44 target from the base rule. */ #chList .ch-item { padding: 10px 12px; gap: 10px; } #chList .ch-item-name { flex: 1 1 auto; min-width: 0; } #chList .ch-item-top { gap: 6px; } - #chList .ch-icon-btn { min-width: 32px; min-height: 32px; padding: 2px 4px; } /* #1648 M5: share-btn now renders an inline from channels.js, * so the legacy `font-size: 0` + `::before { content: '' }` shim is - * obsolete. The icon-button class above already sets the touch target; - * the SVG inherits color via currentColor. */ + * obsolete. The SVG inherits color via currentColor. */ #chList .ch-share-btn .ph-icon { font-size: 13px; } #chList .ch-share-btn { font-size: 0; } #chList .ch-share-btn .ph-icon, #chList .ch-share-btn svg { font-size: 13px; } diff --git a/test-issue-2052-touch-target-css.js b/test-issue-2052-touch-target-css.js index d0a107fa3..0f2a677ea 100644 --- a/test-issue-2052-touch-target-css.js +++ b/test-issue-2052-touch-target-css.js @@ -1,11 +1,18 @@ #!/usr/bin/env node -/* Issue #2052 — keep the nav and channel icon touch-target policy - * unambiguous. Both controls render at 44x44 CSS px today, but earlier - * duplicate 48px declarations made the source disagree with the cascade. +/* Issue #2052 — static touch-target policy for the compact controls. * - * This test reads the shipped stylesheet and requires one consistent value - * across each selector's base declarations. Pseudo-class rules such as - * :hover/:active are excluded because they do not define target dimensions. + * Supplement only: test-issue-2052-touch-target-e2e.js measures the rendered + * boxes in a browser and is the real guarantee. + * + * This test reads the shipped stylesheet and, for every rule anywhere in it + * (including inside @media blocks), looks at each selector whose last compound + * targets .nav-btn, .ch-icon-btn, .ch-share-btn or .ch-remove-btn. More + * specific selectors such as `#chList .ch-icon-btn` count too; an earlier + * version matched only the exact selector text and missed a 32px override. + * Every min-width/min-height declared for these controls must be at least + * 44px. Pseudo-class rules (:hover, :active, :focus, ...) are skipped because + * they do not set target dimensions. The base .nav-btn and .ch-icon-btn rules + * must also keep touch-action: manipulation. */ 'use strict'; @@ -16,39 +23,64 @@ const path = require('path'); const css = fs.readFileSync(path.join(__dirname, 'public', 'style.css'), 'utf8') .replace(/\/\*[\s\S]*?\*\//g, ''); -function policyDeclarations(selector) { - const declarations = []; +const CONTROLS = ['.nav-btn', '.ch-icon-btn', '.ch-share-btn', '.ch-remove-btn']; + +// The innermost { } blocks are the style rules, also inside @media. +function rules() { + const out = []; const rulePattern = /([^{}]+)\{([^{}]*)\}/g; let match; - while ((match = rulePattern.exec(css)) !== null) { - const selectors = match[1].split(',').map((part) => part.trim()); - if (!selectors.includes(selector)) continue; - - for (const property of ['min-width', 'min-height', 'touch-action']) { - const propertyPattern = new RegExp(`(?:^|;)\\s*${property}\\s*:\\s*([^;]+)`, 'g'); - let propertyMatch; - while ((propertyMatch = propertyPattern.exec(match[2])) !== null) { - declarations.push({ property, value: propertyMatch[1].trim() }); + out.push({ selectors: match[1].split(',').map((s) => s.trim()), body: match[2] }); + } + return out; +} + +// The compact control a selector targets, or null. Only the last compound +// counts (`.ch-icon-btn .ph-icon` targets the icon, not the button). +function targetedControl(selector) { + const last = selector.split(/[\s>+~]+/).filter(Boolean).pop() || ''; + if (/:/.test(last)) return null; + return CONTROLS.find((c) => new RegExp('\\' + c + '(?![\\w-])').test(last)) || null; +} + +function declared(body, property) { + const values = []; + const pattern = new RegExp(`(?:^|;)\\s*${property}\\s*:\\s*([^;]+)`, 'g'); + let m; + while ((m = pattern.exec(body)) !== null) values.push(m[1].trim()); + return values; +} + +const violations = []; +const seen = new Set(); +let checked = 0; +for (const rule of rules()) { + for (const selector of rule.selectors) { + const control = targetedControl(selector); + if (!control) continue; + seen.add(control); + for (const property of ['min-width', 'min-height']) { + for (const value of declared(rule.body, property)) { + checked++; + const px = /^(\d+(?:\.\d+)?)px$/.exec(value); + if (!px || Number(px[1]) < 44) violations.push(`${selector} { ${property}: ${value} }`); } } } - - return declarations; } -for (const selector of ['.nav-btn', '.ch-icon-btn']) { - const declarations = policyDeclarations(selector); - const valuesByProperty = Object.fromEntries(['min-width', 'min-height', 'touch-action'].map((property) => [ - property, - [...new Set(declarations.filter((entry) => entry.property === property).map((entry) => entry.value))], - ])); - assert.deepStrictEqual( - valuesByProperty, - { 'min-width': ['44px'], 'min-height': ['44px'], 'touch-action': ['manipulation'] }, - `${selector} must have one unambiguous 44px touch-target policy; got ${JSON.stringify(declarations)}` - ); - console.log(`PASS ${selector}: one consistent 44x44 minimum`); +assert.deepStrictEqual(CONTROLS.filter((c) => !seen.has(c)), [], 'every compact control must have at least one rule'); +assert(checked > 0, 'no min-width/min-height declarations found for the compact controls'); +assert.deepStrictEqual(violations, [], 'compact controls must not declare a touch target below 44px:\n ' + violations.join('\n ')); +console.log(`PASS ${checked} min-width/min-height declarations for ${CONTROLS.join(', ')} are all >= 44px`); + +for (const base of ['.nav-btn', '.ch-icon-btn']) { + const values = rules() + .filter((r) => r.selectors.includes(base)) + .flatMap((r) => declared(r.body, 'touch-action')); + assert.deepStrictEqual([...new Set(values)], ['manipulation'], `${base} must keep touch-action: manipulation; got ${JSON.stringify(values)}`); + console.log(`PASS ${base}: touch-action manipulation`); } console.log('test-issue-2052-touch-target-css.js: OK'); diff --git a/test-issue-2052-touch-target-e2e.js b/test-issue-2052-touch-target-e2e.js new file mode 100644 index 000000000..874715e40 --- /dev/null +++ b/test-issue-2052-touch-target-e2e.js @@ -0,0 +1,260 @@ +#!/usr/bin/env node +/* Issue #2052 — rendered touch targets for the channel row actions. + * + * test-issue-2052-touch-target-css.js checks the stylesheet text; this test + * measures what the browser actually renders, in the real responsive DOM: + * + * - Desktop 1440x900 (sectioned .ch-item list): the share and remove actions + * of a user-added channel are at least 44x44 CSS px, visible, not clipped + * by any overflow container, hit at their centre, not overlapping each + * other or the row's other parts, reachable by keyboard with focus-visible, + * keep their ARIA/title contract, and do their job without side effects + * (share opens the modal, remove asks for confirmation, which is dismissed). + * - Mobile 390x844 touch (flat .ch-row list, #1367): the list renders no + * share/remove actions at all, every visible tap target in the channel + * sidebar is at least 44x44, and tapping a row opens the channel. + * - Tablet 768x1024 touch: characterization only. The sidebar is narrower + * than a channel row there, so the actions are clipped. That is a known, + * separate layout issue (also on master); this step logs the measurement + * and does not gate. + * + * The user-added channel comes from a saved key in localStorage, the same + * state a returning user has; no DOM is injected. No sleeps or retries: each + * step waits for the element it needs. + * + * Defaults to localhost:13581 — NEVER point at prod (AGENTS.md). CI sets BASE_URL. + * Run: BASE_URL=http://localhost:13581 node test-issue-2052-touch-target-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const ORIGIN = new URL(BASE).origin; +const CHANNEL = '#tt2052probe'; +const HASH = 'user:' + CHANNEL; +const SHARE = `#chList [data-share-channel="${HASH}"]`; +const REMOVE = `#chList [data-remove-channel="${HASH}"]`; +const MIN = 44; + +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 function openChannels(browser, errors, options) { + const ctx = await browser.newContext(options); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + page.on('pageerror', (e) => errors.push('pageerror: ' + e.message)); + page.on('console', (m) => { + if (m.type() !== 'error') return; + // Map tiles and other CDN resources are outside this app. + const url = (m.location() && m.location().url) || ''; + if (url && !url.startsWith(ORIGIN)) return; + errors.push('console.error: ' + m.text()); + }); + await page.addInitScript(([name]) => { + localStorage.setItem('corescope_channel_keys', JSON.stringify({ [name]: '00112233445566778899aabbccddeeff' })); + window.addEventListener('unhandledrejection', (e) => { + console.error('unhandledrejection: ' + ((e.reason && e.reason.message) || e.reason)); + }); + }, [CHANNEL]); + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + return { ctx, page }; +} + +// Keyboard focus lands on the control, matches :focus-visible, and the +// existing focus rule (.ch-icon-btn:focus { opacity: 1 }) takes effect. The +// opacity has a 0.15s transition, so wait for its end state, not a time. +async function assertFocused(page, selector, name) { + const st = await page.evaluate((sel) => { + const e = document.querySelector(sel); + return { focused: document.activeElement === e, visible: e.matches(':focus-visible') }; + }, selector); + assert(st.focused, name + ' is not focused'); + assert(st.visible, name + ' focus does not match :focus-visible'); + await page.waitForFunction((sel) => getComputedStyle(document.querySelector(sel)).opacity === '1', selector) + .catch((e) => { throw new Error(name + ' never reached the focused opacity of 1: ' + e.message); }); +} + +// Geometry of one control, measured in the page. +function measure(page, selector) { + return page.evaluate((sel) => { + const el = document.querySelector(sel); + if (!el) return null; + const r = el.getBoundingClientRect(); + const cs = getComputedStyle(el); + // How far the box sticks out of the viewport and of every clipping ancestor. + let clippedBy = []; + const outside = (box, name) => { + const over = Math.max(box.left - r.left, r.right - box.right, box.top - r.top, r.bottom - box.bottom); + if (over > 0.5) clippedBy.push(name + ' by ' + over.toFixed(1) + 'px'); + }; + outside({ left: 0, top: 0, right: innerWidth, bottom: innerHeight }, 'viewport'); + for (let a = el.parentElement; a; a = a.parentElement) { + const c = getComputedStyle(a); + if (c.overflowX !== 'visible' || c.overflowY !== 'visible') outside(a.getBoundingClientRect(), a.id || String(a.className).split(' ')[0] || a.tagName); + } + const hitEl = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2); + return { + w: r.width, h: r.height, left: r.left, right: r.right, top: r.top, bottom: r.bottom, + visible: cs.visibility === 'visible' && cs.display !== 'none' && Number(cs.opacity) > 0 && r.width > 0 && r.height > 0, + clippedBy, + hit: !!hitEl && (hitEl === el || el.contains(hitEl)), + hitOn: hitEl ? (hitEl.id || String(hitEl.className) || hitEl.tagName) : 'nothing', + }; + }, selector); +} + +function overlaps(a, b) { + return !(a.right <= b.left || b.right <= a.left || a.bottom <= b.top || b.bottom <= a.top); +} + +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-2052-touch-target-e2e.js: FAIL — Chromium required but unavailable: ' + err.message); + process.exit(1); + } + console.log('test-issue-2052-touch-target-e2e.js: SKIP (Chromium unavailable: ' + err.message.split('\n')[0] + ')'); + process.exit(0); + } + const errors = []; + console.log(`\n=== #2052 channel action touch targets E2E against ${BASE} ===`); + + // ── Desktop: the sectioned list with inline actions ────────────────────── + const desk = await openChannels(browser, errors, { viewport: { width: 1440, height: 900 } }); + const dp = desk.page; + try { + await dp.waitForSelector(REMOVE, { state: 'visible' }); + + await step('desktop 1440x900: share and remove are at least 44x44, visible, unclipped and hit at their centre', async () => { + for (const [name, sel] of [['share', SHARE], ['remove', REMOVE]]) { + const m = await measure(dp, sel); + assert(m, name + ' action not rendered'); + assert(m.w >= MIN && m.h >= MIN, `${name} renders ${m.w.toFixed(1)}x${m.h.toFixed(1)}, below ${MIN}x${MIN}`); + assert(m.visible, name + ' is not visible'); + assert(m.clippedBy.length === 0, name + ' is clipped by ' + m.clippedBy.join(', ')); + assert(m.hit, `${name} centre hits "${m.hitOn}", not the control`); + } + }); + + await step('desktop: actions do not overlap each other or the rest of the row, and the page has no horizontal overflow', async () => { + const share = await measure(dp, SHARE), remove = await measure(dp, REMOVE); + assert(!overlaps(share, remove), 'share and remove overlap'); + const parts = await dp.evaluate((sel) => { + const row = document.querySelector(sel).closest('.ch-item'); + return [...row.querySelectorAll('.ch-badge, .ch-item-name, .ch-user-badge, .ch-unread-badge, .ch-color-dot, .ch-color-clear, .ch-item-time, .ch-item-preview')] + .map((e) => { const r = e.getBoundingClientRect(); return { name: String(e.className).split(' ')[0], left: r.left, right: r.right, top: r.top, bottom: r.bottom }; }); + }, SHARE); + assert(parts.length >= 4, 'row parts not found'); + for (const p of parts) { + assert(!overlaps(share, p), 'share overlaps ' + p.name); + assert(!overlaps(remove, p), 'remove overlaps ' + p.name); + } + const overflow = await dp.evaluate(() => document.documentElement.scrollWidth - document.documentElement.clientWidth); + assert(overflow <= 0, 'page overflows horizontally by ' + overflow + 'px'); + }); + + await step('desktop: ARIA and title contract is unchanged', async () => { + const a = await dp.evaluate(([s, r]) => [s, r].map((sel) => { + const e = document.querySelector(sel); + return { role: e.getAttribute('role'), tabindex: e.getAttribute('tabindex'), label: e.getAttribute('aria-label'), title: e.getAttribute('title'), popup: e.getAttribute('aria-haspopup') }; + }), [SHARE, REMOVE]); + assert(a[0].role === 'button' && a[0].tabindex === '0' && a[0].label === 'Share ' + CHANNEL && a[0].title === 'Share channel key (QR + URL)' && a[0].popup === 'dialog', + 'share ARIA/title changed: ' + JSON.stringify(a[0])); + assert(a[1].role === 'button' && a[1].tabindex === '0' && a[1].label === 'Remove ' + CHANNEL && a[1].title === 'Remove channel and clear saved key', + 'remove ARIA/title changed: ' + JSON.stringify(a[1])); + }); + + await step('desktop: Tab reaches share then remove with focus-visible, and Enter runs each action without side effects', async () => { + await dp.focus(`#chList .ch-item[data-hash="${HASH}"]`); + await dp.keyboard.press('Tab'); + await assertFocused(dp, SHARE, 'share (Tab from the row)'); + await dp.keyboard.press('Enter'); + await dp.waitForSelector('#chShareModal:not(.hidden)'); + await dp.keyboard.press('Escape'); + await dp.waitForFunction(() => document.getElementById('chShareModal').classList.contains('hidden')); + + await dp.focus(SHARE); // the closed modal does not have to hand focus back + await dp.keyboard.press('Tab'); + await assertFocused(dp, REMOVE, 'remove (Tab from share)'); + const dialog = new Promise((resolve) => dp.once('dialog', (d) => { const text = d.message(); d.dismiss().then(() => resolve(text)); })); + await dp.keyboard.press('Enter'); + const text = await dialog; + assert(/Remove channel/.test(text), 'remove did not ask for confirmation: ' + text); + const kept = await dp.evaluate(([sel, name]) => !!document.querySelector(sel) && + !!JSON.parse(localStorage.getItem('corescope_channel_keys') || '{}')[name], [REMOVE, CHANNEL]); + assert(kept, 'dismissing the confirmation still removed the channel'); + }); + } finally { + await desk.ctx.close(); + } + + // ── Mobile: the flat chat-app list, no inline actions ──────────────────── + const mob = await openChannels(browser, errors, { viewport: { width: 390, height: 844 }, hasTouch: true, isMobile: true }); + const mp = mob.page; + try { + // Wait for the channel's row, whatever layout rendered it; the step below + // asserts which layout it is. + await mp.waitForSelector(`#chList [data-hash="${HASH}"]`, { state: 'visible' }); + + await step('mobile 390x844: the list renders no share/remove actions, and every visible tap target is at least 44x44', async () => { + const r = await mp.evaluate((hash) => ({ + mobileRow: !!document.querySelector(`#chList .ch-row[data-hash="${hash}"]`), + actions: document.querySelectorAll('#chList .ch-icon-btn, #chList [data-share-channel], #chList [data-remove-channel]').length, + desktopRows: document.querySelectorAll('#chList .ch-item').length, + small: [...document.querySelectorAll('.ch-sidebar button, .ch-sidebar a[href], .ch-sidebar [role="button"], .ch-sidebar [tabindex="0"]')] + .filter((e) => { const b = e.getBoundingClientRect(); return b.width > 0 && b.height > 0 && getComputedStyle(e).visibility === 'visible'; }) + .map((e) => { const b = e.getBoundingClientRect(); return { name: e.id || String(e.className).split(' ')[0] || e.tagName, w: +b.width.toFixed(1), h: +b.height.toFixed(1) }; }) + .filter((t) => t.w < 44 || t.h < 44), + overflow: document.documentElement.scrollWidth - document.documentElement.clientWidth, + }), HASH); + assert(r.mobileRow, 'the channel is not rendered as a mobile .ch-row'); + assert(r.actions === 0 && r.desktopRows === 0, `mobile list renders desktop actions (actions=${r.actions}, .ch-item rows=${r.desktopRows})`); + assert(r.small.length === 0, 'mobile tap targets below 44x44: ' + JSON.stringify(r.small)); + assert(r.overflow <= 0, 'mobile page overflows horizontally by ' + r.overflow + 'px'); + }); + + await step('mobile: tapping the channel row opens the channel', async () => { + await mp.tap(`#chList [data-hash="${HASH}"]`); + await mp.waitForFunction((h) => location.hash.includes('/channels/') && + (location.hash.includes(encodeURIComponent(h)) || location.hash.includes(h)), HASH); + }); + } finally { + await mob.ctx.close(); + } + + // ── Tablet: characterization of the known clipping (not gating) ────────── + const tab = await openChannels(browser, errors, { viewport: { width: 768, height: 1024 }, hasTouch: true }); + try { + await tab.page.waitForSelector(REMOVE, { state: 'attached' }); + await step('tablet 768x1024: characterization only (known, separate layout issue)', async () => { + const share = await measure(tab.page, SHARE), remove = await measure(tab.page, REMOVE); + assert(share && remove, 'tablet actions not rendered'); + const fmt = (n, m) => `${n} ${m.w.toFixed(0)}x${m.h.toFixed(0)} hit=${m.hit ? 'yes' : 'no (' + m.hitOn + ')'} clipped=${m.clippedBy.join('; ') || 'no'}`; + console.log(' KNOWN LIMITATION (tracked separately, not fixed here): ' + fmt('share', share) + ' | ' + fmt('remove', remove)); + }); + } finally { + await tab.ctx.close(); + } + + await step('no page errors, console errors or unhandled rejections', async () => { + assert(errors.length === 0, errors.length + ' error(s):\n ' + errors.join('\n ')); + }); + + await browser.close(); + console.log(`\ntest-issue-2052-touch-target-e2e.js: ${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); +} + +main().catch((e) => { console.error(e); process.exit(1); }); From 69161e2153a2fd0ca030b8fc72a08795701ffeec Mon Sep 17 00:00:00 2001 From: Openclaw Date: Fri, 25 Sep 2026 07:07:38 +0200 Subject: [PATCH 3/3] test(e2e): fail instead of hang when remove shows no confirmation The remove step waited for the confirm dialog with a bare page.once promise, so a missing dialog stalled the Playwright step until the job timeout. Use page.waitForEvent('dialog') with a timeout so it fails the step. Also state precisely what the focus check covers: the :focus-visible state and the existing opacity cue, not a focus ring (.ch-icon-btn:focus keeps outline: none, unchanged, as on master). Co-Authored-By: Claude Opus 5.5 --- test-issue-2052-touch-target-e2e.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test-issue-2052-touch-target-e2e.js b/test-issue-2052-touch-target-e2e.js index 874715e40..15799e51f 100644 --- a/test-issue-2052-touch-target-e2e.js +++ b/test-issue-2052-touch-target-e2e.js @@ -68,6 +68,9 @@ async function openChannels(browser, errors, options) { // Keyboard focus lands on the control, matches :focus-visible, and the // existing focus rule (.ch-icon-btn:focus { opacity: 1 }) takes effect. The // opacity has a 0.15s transition, so wait for its end state, not a time. +// This checks the focus state and the existing opacity cue only; the same +// rule also sets outline: none (unchanged here, as on master), so no focus +// ring is asserted. async function assertFocused(page, selector, name) { const st = await page.evaluate((sel) => { const e = document.querySelector(sel); @@ -188,7 +191,10 @@ async function main() { await dp.focus(SHARE); // the closed modal does not have to hand focus back await dp.keyboard.press('Tab'); await assertFocused(dp, REMOVE, 'remove (Tab from share)'); - const dialog = new Promise((resolve) => dp.once('dialog', (d) => { const text = d.message(); d.dismiss().then(() => resolve(text)); })); + // waitForEvent has a timeout, so a missing confirmation fails the step + // instead of hanging the run. + const dialog = dp.waitForEvent('dialog', { timeout: 10000 }) + .then(async (d) => { const text = d.message(); await d.dismiss(); return text; }); await dp.keyboard.press('Enter'); const text = await dialog; assert(/Remove channel/.test(text), 'remove did not ask for confirmation: ' + text);