fix(mobile): let a second tap on inert transcript close the keyboard - #280
Merged
Merged
Conversation
Every terminal tap re-focuses the hidden textarea, so once the on-screen keyboard
is open the only way to close it is the accessory bar's dismiss chevron. Tapping
the transcript to get the screen back is the obvious gesture and it did nothing.
A tap on INERT content with the keyboard already up now dismisses it. Nothing
else claims that gesture: an inert row has no action to trigger, so by that point
the tap has already done its only other job (the mouse report).
Scoped to 'content' ON PURPOSE. The prompt row ('input') keeps
focus-then-position, so a second tap there still places the caret — that is real
capability and trading it away would be a worse deal than the bug. A separate
test pins it rather than leaving it to the reader.
Actionable rows are unchanged: readbacks, "esc to interrupt" status rows and menu
selections still blur via _isActionableMobileTerminalTap, which runs first.
`keeps the hidden keyboard input focused after an inert Claude transcript tap`
asserted the OLD behaviour and is renamed and inverted, since revising that
behaviour is the point of this change. Its setup already focused the terminal
before tapping, so it was always exercising the second-tap case.
test/terminal-touch-tap.test.ts: 28 tests. The two new ones fail on master —
`closes the keyboard on a second tap of INERT transcript content` behaviourally,
by asserting blur where master re-focuses.
test/mobile/keyboard.test.ts: 51 tests, 5 failed | 46 passed — the same five
pre-existing failures as master, untouched here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Uh oh!
There was an error while loading. Please reload this page.
Ark0N pushed a commit
that referenced
this pull request
Aug 11, 2026
#279 and #280 auto-merge cleanly, but the merged result was red: neither branch could see the other, and CI cannot see either, because the only test covering #279 lives in test/mobile/** which test:ci excludes. Two problems, both in #279's test: 1. The in-terminal case tapped the terminal's top-left corner, i.e. an inert transcript row, and asserted focus was retained. That is precisely the gesture #280 redefines, so #280 turned it red. Aim it at the PROMPT row instead: the one in-terminal tap whose outcome neither PR claims, so it still proves the #terminalContainer exemption without asserting the toggle's behaviour. 2. The "a real control is exempt" case was VACUOUS. It picked the first button measuring >8px, which is .welcome-ralph-link inside the welcome overlay hideWelcome() had already hidden: the rect still measures, but elementFromPoint at that point returns .xterm-screen, so the case tapped the TERMINAL and passed for the wrong reason. It only surfaced because #280 changed what a terminal tap does. Require the sampled point to actually resolve to the button, and fail loudly when no control is usable rather than silently asserting nothing. Mutation-checked: removing the install, the #terminalContainer exemption, the control exemption or the `if (moved) return` scroll guard each turns the test red on its own. The control exemption had no coverage before. Also fold the duplicated tap slop into one constant: initTerminal's TAP_THRESHOLD now reads MOBILE_KEYBOARD_DISMISS_TAP_SLOP instead of re-declaring 8, since a drift between them is exactly the bug the second #279 commit fixed. And restore the comment the slop constant was inserted into the middle of, which left "Regions where a tap must NOT dismiss" sitting above the slop rather than the selector it documents. test/mobile/keyboard.test.ts: 5 failed | 47 passed (52). Master is 5 failed | 46 passed (51) — the same five pre-existing failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
prmiguel pushed a commit
to incursion-space/codeman
that referenced
this pull request
Aug 21, 2026
… a runner `npm test` ran config/vitest.config.ts, which includes the browser, visual and perf suites. On any machine without chromium, a free port and per-machine PNG baselines that fails ~87 tests on a clean master, so the repo's most obvious command could not be used as a pass/fail signal. The workaround had spread into four docs as "never run bare `npm test`" warnings. `npm test` now runs config/vitest.ci.config.ts — byte-for-byte what CI runs — so local green means CI green. Verified: 264 files, 5248 tests, exit 0. The suites it leaves out are not abandoned; each has a command: test:browser 5 Playwright files (chromium + a live server; codex-predictive-echo also needs a real codex binary) test:mobile unchanged — the above plus per-machine PNG baselines test:perf 2 wall-clock benchmarks; need an otherwise idle machine test:all the old everything-behaviour, kept reachable test:ci is untouched (CI still calls it). test:watch and test:coverage follow test onto the gate's config. The more important half is the hole this closes. The exclusion list lived as literals in one config and pointed one way only: a file excluded from CI and added to no runner would be tested by NOTHING, silently, with every command still green — vitest counts "no files matched a filter" as success. That is the same shape as the Ark0N#279/Ark0N#280 blind spot already documented in CLAUDE.md. So the globs moved to config/test-suites.ts, one array per REASON a suite cannot run in CI, and all three configs derive from it. test/test-suite-partition.test.ts then checks the arithmetic against the files on disk: it fails if any test file is reachable by no runner, or by two. Confirmed it fires by orphaning a file and watching it name it. The partition is exact today: gate 264 + browser 5 + perf 2 + mobile 9 = 280 = every *.test.ts in the repo⚠️ One sharp edge, deliberate and documented: a file filter must match its runner. `npm test -- test/mobile/keyboard.test.ts` now matches nothing and exits GREEN having run zero tests, because the gate's config excludes that path. CLAUDE.md recommended exactly that command in the on-screen-keyboard note; that line now says `npm run test:mobile -- <file>`, and the Testing section calls out the trap, since a green run of zero tests is worse than a red one. Docs synced: CLAUDE.md, AGENTS.md, .github/CONTRIBUTING.md, README.md, README.zh-CN.md, and two ci.yml comments that claimed only test/mobile/** was excluded — it is three suites, and 5 Playwright files rather than 3. Co-Authored-By: Claude Opus 5 (1M context) <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 freeto 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.
Once the on-screen keyboard is open, tapping the terminal cannot close it.
Every terminal tap re-focuses the hidden textarea, so the only way to dismiss the keyboard is the accessory bar's chevron. Tapping the transcript to get the screen back is the obvious gesture and it does nothing — you tap, and the keyboard just stays.
The change
A tap on inert content with the keyboard already up now dismisses it. Nothing else claims that gesture: an inert row has no action to trigger, so by that point the tap has already done its only other job (the mouse report immediately above).
Scoped to
contenton purpose. The prompt row (input) keeps focus-then-position, so a second tap there still places the caret. That is real capability and trading it away would be a worse deal than the bug — there is a separate test pinning it rather than leaving it to the reader.Actionable rows are untouched: readbacks,
esc to interruptstatus rows and menu selections still blur through_isActionableMobileTerminalTap, which runs first.One existing test is inverted
keeps the hidden keyboard input focused after an inert Claude transcript tapasserted the old behaviour, so it is renamed totoggles the keyboard shut on a second inert Claude transcript tapand its final assertion flipped.I want to be explicit about that rather than bury it: revising that behaviour is this PR. Its setup already focused the terminal before tapping, so it was always exercising the second-tap case — the assertion simply encoded the outcome I am changing. No other test was touched, and nothing was weakened to make something pass.
Tests
test/terminal-touch-tap.test.ts— 28 tests. Two are new:closes the keyboard on a second tap of INERT transcript content— fails on master behaviourally, asserting blur where master re-focuses.keeps the prompt row focusing rather than toggling, so the caret can still be placed— passes on master and here. It is the guarantee that makes this change safe, so it is pinned separately rather than assumed.Verified the first is not vacuous: removing the new branch fails it, and only it.
Verification
Against master
26416f98:Those five are the same pre-existing failures master has — stale layout and accessory-bar expectations plus a CJK timeout — untouched here.
Measured on a Pixel-class viewport (412×915) against a running server:
Relationship to #279
Independent. #279 dismisses on a tap outside the terminal; this dismisses on a second tap inside it on inert content. They touch the same function but different branches, and either can land without the other. Happy to rebase this one if #279 goes first.
🤖 Generated with Claude Code