Repository navigation
fix(css): unify compact touch target sizes - #85
Merged
Merged
Conversation
…argets Review found the Kpa-clawbot#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 (Kpa-clawbot#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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.nav-btnand.ch-icon-btn. Keep the shipped 44×44 minimum andtouch-action: manipulation. Clarify the WCAG/Apple versus Material guidance in the stylesheet comment.#chList .ch-icon-btn { min-width: 32px; min-height: 32px }override. Replace the falsely green static test with a browser test of the rendered touch targets, plus a corrected static policy test.Upstream context: issue 2052 in
Kpa-clawbot/CoreScope.Review finding and what it turned out to be
The first version's policy test matched only the exact selector text
.ch-icon-btn. It missed a more specific rule inside@media (max-width: 640px)that set the channel share/remove actions to 32×32.Measuring the real app showed that rule never applies. Below 768 px,
channels.jsrenders the flat.ch-rowchannel list (#1367), which has no inline share/remove actions. It also re-renders when the viewport crosses 767 px, so desktop rows never survive a resize to a narrow width.So the rule contradicted the 44×44 policy without ever sizing a control a user could touch. It is removed, and the comment now states the responsive contract.
Responsive contract (as tested)
.ch-itemrows.ch-rowlist, rows 80 px highOn mobile, the visible tap targets in the channel sidebar are all at least 44×44: region pills 44×44, Add 60×48, and the rows.
Known limitation, not addressed here
Between 768 px and about 1330 px (≈1336 px counting the resize handle over remove's right edge), share/remove in the desktop rows are clipped by the channel sidebar. The sidebar is
width: clamp(220px, 22vw, 320px); overflow: hidden, and a row with both actions needs about 293 px.This behaves identically on master and is independent of this PR. It is kept out of scope and documented as a separate follow-up. There is no layout change here. The new E2E test logs the 768×1024 measurement as a non-gating characterization step, so the limitation stays visible in CI output.
Tests
test-issue-2052-touch-target-e2e.js(new, Playwright step). It uses a user-added channel from a saved key inlocalStorage; no DOM is injected.elementFromPointat the centre hits the control. They don't overlap each other or the row's parts, and the page has no horizontal overflow.:focus-visibleand the existing focused opacity (waited for, because of its 0.15s transition). No focus ring is asserted:.ch-icon-btn:focuskeepsoutline: none, unchanged and as on master. Enter on share opens the share modal; Escape closes it. Enter on remove shows the confirmation (waited for with a timeout, so a missing dialog fails instead of hanging), which is dismissed, and the channel and key stay..ch-rowwith no share/remove and no.ch-item. Every visible sidebar tap target is ≥44×44, there is no overflow, and tapping the row opens the channel.test-issue-2052-touch-target-css.js(unit step, now a supplement). Every selector anywhere in the stylesheet, including@mediaand more specific selectors, whose last compound targets.nav-btn,.ch-icon-btn,.ch-share-btnor.ch-remove-btnmust declare min-width/min-height ≥44px (10 declarations checked)..nav-btnand.ch-icon-btnmust keeptouch-action: manipulation.Mutation evidence (scratch copies, never committed)
#chList .ch-icon-btn { min-width: 32px }waitForEvent('dialog')timeout (before the follow-up commit this hung)The first four mutants were run twice each, with the same result.
Fresh verification
All runs were local only, with no staging or production contact.
4c5efa7bin an isolated export is conflict-free. The diff against master is exactly this PR's five files, and the workflow change is two registration lines.git diff --checkis clean.node --checkpasses, and the YAML parses.Independent review
A separate reviewer re-measured this and found no blockers. They confirmed:
Their one test weakness was fixed in a follow-up commit: the remove confirmation could hang instead of fail. It is now red within the timeout. The other findings are out of scope, and all behave the same on master:
outline: none);.ch-remove-btn, .ch-share-btnrule;:activescale shrinking the hitbox;test-touch-targets.js, which is unregistered and stale, still expecting 48px.Scope
public/style.css: one dead rule removed, and comments updated.test-issue-2052-*files, plus registration intest-all.shand one line each in the unit and Playwright steps.Known unrelated baseline
A direct run of
test-frontend-helpers.json the base still has its two existing stalefavStarexpectation failures. This PR does not touch that code or relax those tests.🤖 Generated with Claude Code