From 8ab854fb2f86ae2a5d8e54b8212da8f3e4d87df4 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 5 Oct 2026 06:58:21 +0000 Subject: [PATCH 1/5] test(touch-targets): measure the remaining 44 px controls at 48 px (#235) test-touch-targets.js now loads every local stylesheet in index.html order and has a viewport meta, so the iPhone harness really is 390 px wide and the max-width: 640px rules apply. It lists the controls from #235 and the other 44 px controls found on master (the coarse-pointer #630 block, Leaflet zoom and live toggles, the channel-proposals buttons, the live node-filter hit area and region tap pad). The channel header's back button is .ch-back (40 px); .ch-back-btn has no markup. test-issue-2052-touch-target-e2e.js raises the mobile sidebar check to 48 and measures, at 390x844, the open channel's back button, every sender avatar and the add-channel dialog's close button, with no horizontal overflow. Both fail on master: 24 harness assertions, 4 E2E steps. Co-Authored-By: Claude Opus 5.5 --- test-issue-2052-touch-target-e2e.js | 88 +++++++++++++++++------ test-touch-targets.js | 106 ++++++++++++++++++++++------ 2 files changed, 153 insertions(+), 41 deletions(-) diff --git a/test-issue-2052-touch-target-e2e.js b/test-issue-2052-touch-target-e2e.js index 3f22c2d2d..face2554c 100644 --- a/test-issue-2052-touch-target-e2e.js +++ b/test-issue-2052-touch-target-e2e.js @@ -12,9 +12,11 @@ * (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 (WCAG_MIN: other sidebar controls, e.g. the - * coarse-pointer .region-pill, are outside #2052 and still 44), and - * tapping a row opens the channel. + * sidebar is at least 48x48 (the coarse-pointer .region-pill included, + * #235), and tapping a row opens the channel. In the open channel the + * header's back button and every sender avatar, and in the add-channel + * dialog the close button, are at least 48x48 with no horizontal + * overflow (#235). * - 768-1330 px (the narrow sectioned sidebar; 768 with touch): share and * remove are at least 48x48, visible, unclipped, hit at their centre and * do not overlap. Before upstream PR 2078 the remove action was clipped @@ -40,7 +42,8 @@ const HASH = 'user:' + CHANNEL; const SHARE = `#chList [data-share-channel="${HASH}"]`; const REMOVE = `#chList [data-remove-channel="${HASH}"]`; const MIN = 48; -const WCAG_MIN = 44; +// A server channel the E2E fixture has messages in, so sender avatars render. +const MSG_CHANNEL = '#test'; let passed = 0, failed = 0; async function step(name, fn) { @@ -121,20 +124,26 @@ function overlaps(a, b) { return !(a.right <= b.left || b.right <= a.left || a.bottom <= b.top || b.bottom <= a.top); } -// Share and remove: at least MIN square, visible, unclipped, hit at their -// centre, and not on top of each other. +// One control: at least MIN square, visible, unclipped and hit at its centre. +async function assertTarget(page, selector, name) { + const m = await measure(page, selector); + assert(m, name + ' 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`); + return m; +} + +// Share and remove: each a full target, and not on top of each other. async function assertActionTargets(page) { - const found = {}; - for (const [name, sel] of [['share', SHARE], ['remove', REMOVE]]) { - const m = await measure(page, 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`); - found[name] = m; - } - assert(!overlaps(found.share, found.remove), 'share and remove overlap'); + const share = await assertTarget(page, SHARE, 'share'); + const remove = await assertTarget(page, REMOVE, 'remove'); + assert(!overlaps(share, remove), 'share and remove overlap'); +} + +function horizontalOverflow(page) { + return page.evaluate(() => document.documentElement.scrollWidth - document.documentElement.clientWidth); } async function main() { @@ -229,7 +238,7 @@ async function main() { // 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 ${WCAG_MIN}x${WCAG_MIN}`, async () => { + await step(`mobile 390x844: the list renders no share/remove actions, and every visible tap target is at least ${MIN}x${MIN}`, async () => { const r = await mp.evaluate(([hash, min]) => ({ 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, @@ -239,10 +248,10 @@ async function main() { .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 < min || t.h < min), overflow: document.documentElement.scrollWidth - document.documentElement.clientWidth, - }), [HASH, WCAG_MIN]); + }), [HASH, MIN]); 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 ${WCAG_MIN}x${WCAG_MIN}: ` + JSON.stringify(r.small)); + assert(r.small.length === 0, `mobile tap targets below ${MIN}x${MIN}: ` + JSON.stringify(r.small)); assert(r.overflow <= 0, 'mobile page overflows horizontally by ' + r.overflow + 'px'); }); @@ -251,6 +260,45 @@ async function main() { await mp.waitForFunction((h) => location.hash.includes('/channels/') && (location.hash.includes(encodeURIComponent(h)) || location.hash.includes(h)), HASH); }); + + await step(`mobile: the open channel's back button is at least ${MIN}x${MIN}, unclipped and hit at its centre (#235)`, async () => { + await mp.waitForSelector('#chHeader .ch-back', { state: 'visible' }); + await assertTarget(mp, '#chHeader .ch-back', 'back button'); + const overflow = await horizontalOverflow(mp); + assert(overflow <= 0, 'channel view overflows horizontally by ' + overflow + 'px'); + }); + + await step(`mobile: in a channel with messages every sender avatar is at least ${MIN}x${MIN}, and nothing overflows horizontally (#235)`, async () => { + await mp.tap('#chHeader .ch-back'); + await mp.tap(`#chList [data-hash="${MSG_CHANNEL}"]`); + await mp.waitForSelector('#chMessages .ch-avatar.ch-tappable', { state: 'visible' }); + const r = await mp.evaluate((min) => { + const msgs = document.getElementById('chMessages'); + const avatars = [...msgs.querySelectorAll('.ch-avatar.ch-tappable')].map((e) => e.getBoundingClientRect()); + return { + count: avatars.length, + small: avatars.filter((b) => b.width < min || b.height < min).map((b) => b.width.toFixed(1) + 'x' + b.height.toFixed(1)), + msgsOverflow: msgs.scrollWidth - msgs.clientWidth, + }; + }, MIN); + assert(r.count > 0, 'no sender avatars rendered in ' + MSG_CHANNEL); + assert(r.small.length === 0, `${r.small.length} of ${r.count} avatars below ${MIN}x${MIN}: ` + r.small.slice(0, 5).join(', ')); + assert(r.msgsOverflow <= 0, 'message list overflows horizontally by ' + r.msgsOverflow + 'px'); + await assertTarget(mp, '#chHeader .ch-back', 'back button'); + const overflow = await horizontalOverflow(mp); + assert(overflow <= 0, 'channel view overflows horizontally by ' + overflow + 'px'); + }); + + await step(`mobile: the add-channel dialog's close button is at least ${MIN}x${MIN} and hit at its centre, and the page does not overflow (#235)`, async () => { + await mp.tap('#chHeader .ch-back'); + await mp.tap('#chAddChannelBtn'); + await mp.waitForSelector('#chModalClose', { state: 'visible' }); + await assertTarget(mp, '#chModalClose', 'dialog close'); + const overflow = await horizontalOverflow(mp); + assert(overflow <= 0, 'page with the dialog open overflows horizontally by ' + overflow + 'px'); + await mp.tap('#chModalClose'); + await mp.waitForFunction(() => document.getElementById('chAddChannelModal').classList.contains('hidden')); + }); } finally { await mob.ctx.close(); } diff --git a/test-touch-targets.js b/test-touch-targets.js index da84a0121..48294e298 100644 --- a/test-touch-targets.js +++ b/test-touch-targets.js @@ -24,25 +24,31 @@ const assert = require('assert'); const { chromium, devices } = require('playwright'); const REPO = __dirname; -const CSS = fs.readFileSync(path.join(REPO, 'public/style.css'), 'utf8'); +// Every local stylesheet, in the order index.html links them, so the +// cascade matches the app (home.css restyles .suggest-claim, live.css and +// channel-proposals.css carry their own controls). +const SHEETS = [...fs.readFileSync(path.join(REPO, 'public/index.html'), 'utf8') + .matchAll(//g)].map((m) => m[1]); +const CSS = SHEETS.map((f) => fs.readFileSync(path.join(REPO, 'public', f), 'utf8')).join('\n'); // All listed button surfaces use the shared 48px house minimum (#2052). const DEFAULT_MIN = 48; -// Each entry: [selector, tag, classes, optional inner-html]. +// Each entry: [selector, tag, classes, optional wrapper]. The wrapper is +// markup around the control, with {} where the control goes, for rules that +// only apply inside a parent (`.modal > .modal-close`, `.filter-bar .btn`). // Tag matters because some rules are scoped to `button.ch-item` and some only // apply to specific input[type=...]. // +// The harness is an iPhone 13 (390 px wide, coarse pointer), so the +// `@media (max-width: 640px)` and `@media (pointer: coarse)` rules apply. +// // Not listed, and why: // .compare-btn the Compare CTA was removed in #1646 (see style.css) -// .ch-back-btn display:none outside the mobile channels layout, so a -// standalone element in this harness measures 0x0 -// .filter-toggle-btn hidden on mobile since #1461 (`.filter-bar > -// .filter-toggle-btn`, display:none !important); the -// control actually shown is the navbar mirror, which -// mobile-page-actions.js builds with class "nav-btn -// filter-toggle-btn-mirror mpa-btn-pill", so the .nav-btn -// entry below already measures it +// .ch-back-btn no markup renders it since #1367; the channel header's +// back button is .ch-back, listed below +// .feed-show-btn, .legend-toggle-btn live.css hides both at <=640px +// (display:none !important), the width this harness has const BUTTON_SELECTORS = [ ['.btn', 'button', 'btn'], ['.btn-icon', 'button', 'btn-icon'], @@ -64,10 +70,30 @@ const BUTTON_SELECTORS = [ ['.copy-link-btn', 'button', 'copy-link-btn'], ['.alab-btn', 'button', 'alab-btn'], ['.fav-star', 'button', 'fav-star'], + // #235: the controls that were still 44 px (40 px for .ch-back). + ['.theme-toggle', 'label', 'theme-toggle'], + ['.modal > .modal-close', 'button', 'modal-close', '
{}
'], + ['.ch-modal-close', 'button', 'modal-close ch-modal-close', '
{}
'], + ['.ch-back', 'button', 'ch-back', '
{}
'], + ['.ch-avatar.ch-tappable', 'div', 'ch-avatar ch-tappable'], + ['.suggest-claim', 'button', 'suggest-claim'], + ['.detail-back-btn', 'button', 'detail-back-btn'], + ['.filter-toggle-btn', 'button', 'filter-toggle-btn'], + ['.filter-bar .btn', 'button', 'btn', '
{}
'], + ['.filter-group .btn', 'button', 'btn', '
{}
'], + ['.tab-btn', 'button', 'tab-btn'], + ['.region-pill', 'button', 'region-pill'], + ['.region-dropdown-trigger','button', 'region-dropdown-trigger'], + ['.multi-select-trigger', 'button', 'multi-select-trigger'], + ['.node-count-pill', 'span', 'node-count-pill'], + ['.analytics-time-range button', 'button', '', '
{}
'], + ['.leaflet-control-zoom a','a', '', '
{}
'], + ['.live-leaflet-toggle a', 'a', '', '
{}
'], ]; -// Form controls. min-WIDTH is not enforced on these (text fields legitimately -// span a wide column); we only require min-height: 48px. +// Form controls and text buttons that only set a height. min-WIDTH is not +// enforced on these (text fields legitimately span a wide column); we only +// require min-height: 48px. Entry: [selector, tag, classes, inner, attrs, wrapper]. const FIELD_SELECTORS = [ ['select', 'select', '', ''], ['input[type=text]', 'input', '', null, { type: 'text' }], @@ -79,32 +105,53 @@ const FIELD_SELECTORS = [ ['input[type=url]', 'input', '', null, { type: 'url' }], ['input[type=date]', 'input', '', null, { type: 'date' }], ['input[type=time]', 'input', '', null, { type: 'time' }], + // #235 + ['.filter-bar input', 'input', '', null, { type: 'text' }, '
{}
'], + ['.filter-bar select', 'select', '', '', null, '
{}
'], + ['.ch-proposals-toolbar button', 'button', '', 'x', null, '
{}
'], + ['.ch-proposals-actions button', 'button', '', 'x', null, '
{}
'], + ['.live-node-filter-hitarea', 'label', 'live-node-filter-hitarea', 'x'], +]; + +// Invisible ::after tap pads that give a compact control its hit area. +// Entry: [name, control markup with data-pad on the padded element]. +const PAD_SELECTORS = [ + ['live region dropdown ::after', '
' + + '
'], ]; +function wrap(wrapper, html) { + return wrapper ? wrapper.replace('{}', html) : html; +} + function buildSampleHtml() { const buttons = BUTTON_SELECTORS - .map(([_, tag, cls]) => `<${tag} class="${cls}" data-sel="${cls}">x`) + .map(([_, tag, cls, wrapper], i) => wrap(wrapper, `<${tag} class="${cls}" data-btn="${i}">x`)) .join('\n '); const fields = FIELD_SELECTORS - .map(([sel, tag, cls, inner, attrs]) => { + .map(([sel, tag, cls, inner, attrs, wrapper], i) => { const attrStr = attrs ? Object.entries(attrs).map(([k, v]) => `${k}="${v}"`).join(' ') : ''; - const open = `<${tag} ${attrStr} data-sel="${sel.replace(/[\[\]=]/g, '_')}">`; + const open = `<${tag} class="${cls}" ${attrStr} data-field="${i}">`; const close = tag === 'input' ? '' : `${inner || ''}`; - return open + close; + return wrap(wrapper, open + close); }) .join('\n '); + const pads = PAD_SELECTORS.map(([, html]) => html).join('\n '); // .sort-help sample mirrors the markup the JS produces (post-fix): // tabindex="0" so :focus-within can fire on touch tap. return ` + +
${buttons} ${fields} + ${pads} ⓘ Tip body @@ -155,8 +202,8 @@ async function run() { } // --- Buttons: rendered hit area must be at least 48x48 CSS px. - for (const [selector, , cls] of BUTTON_SELECTORS) { - const dim = await page.$eval(`[data-sel="${cls}"]`, (el) => { + for (const [i, [selector]] of BUTTON_SELECTORS.entries()) { + const dim = await page.$eval(`[data-btn="${i}"]`, (el) => { const r = el.getBoundingClientRect(); const cs = getComputedStyle(el); return { w: r.width, h: r.height, mh: cs.minHeight, mw: cs.minWidth }; @@ -170,9 +217,8 @@ async function run() { } // --- Form controls: rendered height must be at least 48 CSS px. - for (const [selector, , , , attrs] of FIELD_SELECTORS) { - const dataKey = selector.replace(/[\[\]=]/g, '_'); - const dim = await page.$eval(`[data-sel="${dataKey}"]`, (el) => { + for (const [i, [selector]] of FIELD_SELECTORS.entries()) { + const dim = await page.$eval(`[data-field="${i}"]`, (el) => { const r = el.getBoundingClientRect(); const cs = getComputedStyle(el); return { h: r.height, mh: cs.minHeight }; @@ -182,6 +228,24 @@ async function run() { `expected height >=48, got ${dim.h}`); } + // --- Tap pads: the ::after box must be at least 48x48. + for (const [name, html] of PAD_SELECTORS) { + const key = html.match(/data-pad="([^"]+)"/)[1]; + const dim = await page.$eval(`[data-pad="${key}"]`, (el) => { + const cs = getComputedStyle(el, '::after'); + return { w: parseFloat(cs.width), h: parseFloat(cs.height) }; + }); + record(`${name}: tap pad ${dim.w.toFixed(1)}x${dim.h.toFixed(1)}`, + dim.w >= DEFAULT_MIN && dim.h >= DEFAULT_MIN, + `expected >=${DEFAULT_MIN}x${DEFAULT_MIN}, got ${dim.w}x${dim.h}`); + } + + // The coarse-pointer and <=640px rules above only apply if the harness + // really is a phone. + const env = await page.evaluate(() => ({ coarse: matchMedia('(pointer: coarse)').matches, narrow: matchMedia('(max-width: 640px)').matches })); + record('iPhone context matches (pointer: coarse) and (max-width: 640px)', env.coarse && env.narrow, + `expected both true, got ${JSON.stringify(env)}`); + // --- MAJOR-1 verification: .sort-help is keyboard/tap focusable AND the // tooltip becomes visible on focus (tap-to-reveal works without hover). const tabIndex = await page.$eval('#sortHelp', (el) => el.getAttribute('tabindex')); From cd1a70e70a2425903d8e656ba1c27a234e0b8646 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 5 Oct 2026 07:10:50 +0000 Subject: [PATCH 2/5] fix(css): raise the remaining 44 px controls to the 48 px touch-target minimum (#235) Every control that was still sized at 44 px now uses the 48 px house minimum from #2052: - .theme-toggle; .modal-close / .ch-modal-close and .ch-avatar.ch-tappable (max-width: 640px); .suggest-claim; .ch-back-btn (no markup renders it). - The channel header's back button, .ch-back, which was 40 px. - The #630 coarse-pointer block: .filter-bar/.filter-group .btn, .tab-btn, .region-pill, .region-dropdown-trigger, .multi-select-trigger, .node-count-pill, .analytics-time-range button, .detail-back-btn, .filter-toggle-btn, and the .filter-bar input/select height. - Leaflet zoom buttons and the live page's map toggles. - The live region dropdown's ::after tap pad and the node-filter hit area, whose inline style in live.js moves to a live.css rule. - The channel-proposals toolbar and action buttons. At 390 px the region pills no longer fit beside the channel title and + Add. The mobile sidebar header kept max-height: 56px, which on master already let the list cover the lower half of the 44 px pills; with 48 px pills + Add wrapped under the list and could not be tapped. The cap goes, title and + Add share the first row and the pills take the second (115 px). test-issue-1224-channels-mobile-ux-e2e.js now checks that the header holds its controls and the list starts below it (<=120 px) instead of the <=60 px bound that only the cap kept true. .feed-show-btn and .legend-toggle-btn keep their 44 px rule: live.css hides both at <=640px, the only width where it applies. Co-Authored-By: Claude Opus 5.5 --- public/channel-proposals.css | 2 +- public/live.css | 12 ++++--- public/live.js | 2 +- public/style.css | 42 +++++++++++++---------- test-issue-1224-channels-mobile-ux-e2e.js | 25 ++++++++++---- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/public/channel-proposals.css b/public/channel-proposals.css index 53d44cdc3..44535f9a3 100644 --- a/public/channel-proposals.css +++ b/public/channel-proposals.css @@ -75,7 +75,7 @@ .ch-proposals-builtin { font-size: 12px; color: var(--warning); } .ch-proposals-actions { display: flex; gap: 6px; align-items: center; } .ch-proposals-actions button, -.ch-proposals-toolbar button { min-height: 44px; } +.ch-proposals-toolbar button { min-height: 48px; } .ch-proposals-state { font-size: 12px; text-transform: capitalize; diff --git a/public/live.css b/public/live.css index 818841b71..93a3175e8 100644 --- a/public/live.css +++ b/public/live.css @@ -495,8 +495,8 @@ position: absolute; top: 50%; left: 50%; - width: max(100%, 44px); - height: 44px; + width: max(100%, 48px); + height: 48px; transform: translate(-50%, -50%); } @@ -762,9 +762,9 @@ body.live-fullscreen:not(.nav-pinned) .live-header { /* No absolute positioning needed; they stack normally in leaflet-right */ } .live-leaflet-toggle a { - width: 44px !important; - height: 44px !important; - line-height: 44px !important; + width: 48px !important; + height: 48px !important; + line-height: 48px !important; font-size: 20px !important; background-color: var(--surface-1) !important; color: var(--text) !important; @@ -994,6 +994,8 @@ body.live-fullscreen #liveFullscreenToggle:hover, /* #1110 Live page node filter — match toolbar control sizing & theme */ .live-node-filter-wrap { position: relative; display: inline-flex; align-items: center; } +/* The label around the compact input is its 48px hit area (#235). */ +.live-node-filter-hitarea { display: inline-flex; align-items: center; min-height: 48px; cursor: text; } input.live-node-filter-input { box-sizing: border-box; height: 30px; diff --git a/public/live.js b/public/live.js index 382638050..2b5622b7b 100644 --- a/public/live.js +++ b/public/live.js @@ -1285,7 +1285,7 @@
-