Repository navigation
fix(mobile): keep the outgoing tab out of the tap that switches tabs - #8267
Conversation
A bottom-tab press navigates, but the tab being left stays what a touch is hit-tested against until the destination screen lays out: a tap that starts in that window runs whatever the outgoing tab draws at that point, so the Profile Code Reviewer row's tap reached the Home new-task button and opened the new-session composer. The window measured up to about 0.8 s on the emulator, and the route state changes before it closes, so neither one frame nor the route commit is a safe boundary. The tab surface now claims touches that start while a pressed tab is settling, reading the pending window when the touch arrives. The tab entry arms it on press-in, and each tab screen closes it from its own first layout. Adds the gate's unit and mounted tests, and pins the three Profile agent rows' destinations for an organization and the personal scope.
iscekic
added
drain
|
bot: Android emulator evidence for the reported sequence, exercised on the German UI against a local dev backend with a test account. Sequence: on a settled Home, press the Profile tab and then tap the Profile row's point (the same point is Home's new-task button) about 50 ms later, repeated at 0 / 200 / 400 / 800 ms after the tab press.
A settled tap on the same point opens the code reviewer platform list (second image). Both captures are the personal scope. With an organization selected the same sequence produced the same two results: the stray tap left the app on the Profile tab, and a settled tap opened the platform list for that organization.
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only post-review commit ( Assumptions: The branch merged Files Reviewed (1 file, incremental 7105fbc..eb82ec3)
Notes: All six prior inline threads have author replies describing fixes in Previous Review Summaries (4 snapshots, latest commit 7105fbc)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 7105fbc)Status: No Issues Found | Recommendation: Merge Executive SummaryThe one incremental commit ( Overview
Files Reviewed (5 files, incremental 283d600..7105fbc)
Notes: Previous review (commit 283d600)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe lint-cleanup commit only reworks three test files and introduces no new defect; the outstanding 250 ms same-tab settle, which can end the gate before a pop-to-root transition finishes, is unchanged in Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files, incremental c3c6d1f..283d600)
Notes: no memory leak was introduced. The test changes add fake timers only inside two tests and restore real timers; the production store still holds one timestamp and one timer handle that every arm, settle, and reset clears, and registers no listeners. No markdown documentation files were changed in this PR. Fix these issues in Kilo Cloud Previous review (commit c3c6d1f)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe follow-up correctly closes the window for cancelled presses and presses of the already-selected tab, but the new 250 ms same-tab settle can end the gate before a pop-to-root transition finishes. Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files, incremental c3c6d1f)
Notes: no memory leak was introduced. The module-level store holds one timestamp and one Fix these issues in Kilo Cloud Previous review (commit ebb39bb)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe tab-switch gate fixes the reported stray-tap defect, but arming it on every touch down leaves tab content unresponsive for the 1200 ms fallback whenever a press never navigates, and the excluded KiloClaw tab always takes that fallback. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (13 files)
Notes: the module-level store holds a single timestamp, registers no listeners or timers, and the mounted tests reset it, so this PR introduces no memory leak. The shared profile test helper no longer mocks the path builders, and no helper consumer relied on the old literal paths. No markdown documentation files were changed in this PR. Reviewed by deepseek-flash-latest · Input: 62.9K · Output: 3.1K · Cached: 399.4K Review guidance: REVIEW.md from base branch |
The tab being left can lay out again during a switch; closing the window on that layout reopened it while the outgoing views were still the touch targets. Each tab screen now reports its layout only while it holds focus. A press that opens no screen closes its window on a timer: pressing the tab that is already focused settles after the pop back to that tab's root, and a gesture that ends without a press settles on the next tick. The press listeners move to a module of their own, where the profile entry's behaviour (prevent the default, return to the root) is asserted directly instead of by reading the layout source.
Use the timer API instead of a promise callback for the press-out case, type the navigation mocks, and name the focus-effect stand-in parameter so it is not read as a promise callback.
A same-tab press pops that tab's stack, and the screen being left stays the touch target until the pop ends. Each tab's stack now closes the input window on `transitionEnd`, and the same-tab timer only bounds a press that opens no transition at all.
iscekic
added
merge:human
…t-gate # Conflicts: # apps/mobile/src/app/(app)/(tabs)/_layout.tsx # apps/mobile/src/lib/profile-tab-anchor.test.ts
The profile tab press now goes through `planProfileTabPress` inside `profileTabPressListeners`, so the listener suite pins the cross-tab, root re-press and stack-pop outcomes directly instead of reading the layout source.
iscekic
removed
the
merge:bot
|
bot: Fresh verification on the merged tree (Android emulator, German UI, test organization selected). Each run starts from a settled tab, presses the target tab on the bar, then taps the overlapping point where the Profile Code Reviewer row and the Start tab's new-task button coincide. Observed: the quick second tap is consumed — the app stays on the pressed tab and no composer appears in four runs; the reverse direction stays on the Start tab and opens no reviewer; a settled tap on the Profile row opens the code-reviewer platform list for the selected organization; a settled tap on the new-task button opens the composer; a tap two seconds after a switch still lands.
|
iscekic
added
the
merge:bot
What changed
Tapping the Profile Code Reviewer row right after pressing the Profile tab opened
the New session composer instead of the code reviewer. The row itself is correct:
with a settled screen it pushes
getCodeReviewerProfilePath(scope). The tap wasdelivered to the tab being left.
A bottom-tab press navigates, but the tab being left stays what a touch is hit-tested
against until the destination screen lays out. A tap that starts inside that window runs
whatever the outgoing tab draws at that point. On the emulator the Profile row
(
[37,908][1043,1052]) and Home's "New coding task" button ([37,938][1043,1053]) sharea tap point, so the second tap opened the composer.
The window is not a frame and not the route commit:
So the boundary has to be the destination tab's first layout.
Change
apps/mobile/src/lib/tab-switch-input-gate.ts(new): the pending window as atimestamp, read when a touch arrives (React state could only apply with the commit
whose absence is the problem).
TAB_SWITCH_INPUT_FALLBACK_MS(1200 ms) caps a switchthat never lays out.
apps/mobile/src/components/tab-screen-input-gate.tsx(new):TabSwitchInputGateclaims touches on the tab surface while the window is open;
TabSwitchInputSettlecloses it from the tab screen's own first layout.
apps/mobile/src/app/(app)/(tabs)/_layout.tsx: the surface gate wraps the tabnavigator, so every tab is covered, including any tab whose own layout file this
change does not touch.
apps/mobile/src/app/(app)/(tabs)/{ (0_home), (2_agents), (3_profile), (4_chat) }/_layout.tsx:their
Stackis wrapped inTabSwitchInputSettle.apps/mobile/src/components/tab-bar-button.tsx: the entry arms the window onpress-in, before the navigation starts.
apps/mobile/src/components/profile-screen.agent-rows.mounted.test.tsx(new): pins thethree Profile agent rows — Code Reviewer, Security Agent, Profiles — for an
organization and for the personal scope. Nothing covered them; only the Home copies
were pinned. The shared helper now mocks just the scope, so the assertions observe the
real route builders.
One tab's own layout belongs to an excluded feature and is intentionally untouched;
the surface gate covers that tab, and its window closes through the fallback.
End-to-end verification (Android emulator, German UI, a test organization selected)
Sequence on a settled Home:
input tap 945 2260(Profile tab) theninput tap 540 980(the Profile row's rect, which is Home's new-task button rect).
Before — the same sequence with the gate's flag forced closed (no gate in effect), and
with the pre-fix build earlier in the session:
After — with the gate in place, six runs at 0 / 200 / 400 / 800 ms:
PROFILEis the app staying on the Profile tab: the stray tap is consumed, and thecomposer never appears.
Still working after the change, same device and locale, organization selected:
CODE-PRÜFERfor that organization;Tradeoff of gating the whole tab surface: a tab press inside the settling window (for
example, correcting a mis-tap immediately) is consumed; the next press works. This
follows the approved scope for the gate.
Screenshots and the UI dumps they were classified from are posted as a comment on this
PR.
Tests
apps/mobile:pnpm test— 1197 files, 17910 tests passed, 2 skipped.pnpm typecheck,oxlint,oxfmton every changed file: clean.Proxy answers
Answer: Do not end no_change. Your own evidence shows a real defect: during a tab switch, a tap meant for the Profile row reaches Home's 'Neue Programmieraufgabe' button. Fix that class of issue: make sure the outgoing tab's screen cannot receive touches during a transition, or that the incoming screen owns input as soon as it is visible, on every tab. Also add the mounted regression test that pins the Code Reviewer, Security Agent and Profiles row destinations. Verify on Android, in German, with an org selected. For proof, reproduce the quick tap during a tab switch before and after the fix, and post logs or screenshots on the PR.
Confidence: 0.55 · Precedent: Fix the root cause and the class of the issue; end without a PR only with proof that the finding is wrong (an earlier session).
Answer: Whole tab area, including the tab bar
Confidence: 0.65 · Precedent: Do the least amount of work with no drop in quality; one gate leaves no hole and fixes the whole class (earlier sessions).
Answer: Settle on route commit + 700 ms fallback
Confidence: 0.75 · Precedent: Design and UX must be best-in-class; a fixed window swallows real taps and can still leak on a slow switch (an earlier session).
Answer: Tab press only
Confidence: 0.70 · Precedent: Programmatic changes have no stray tap, and arming on them would swallow real input after redirects (an earlier session).
Two of these were adjusted after measurement, with the reasons above:
touch targets), so each tab screen settles the window from its own first layout, with
a 1200 ms cap instead of 700 ms — a tap at 777 ms still reached the outgoing screen.
noted above.
Merge from main
mainmoved after this branch's last push and the PR conflicted in two files. Resolved bymerging
maininto the branch (no rebase):apps/mobile/src/lib/tab-press-listeners.ts: the profile entry now returns the press planthat
mainintroduced (planProfileTabPress). A cross-tab press and a form-sheet re-pressreturn to the navigator, so the tab keeps its stack and sheet; a re-press that moves the
tab to its root dispatches the targeted stack pop (
POP_TO_TOP) so the unsaved-changesguard can intercept it. The input window still settles on its own timer for a press that
opens no screen.
apps/mobile/src/app/(app)/(tabs)/_layout.tsx: the tab screens keep the listener helpers,and the tab navigator stays wrapped in the surface gate. The
max-linesexemptionmainadded is no longer needed now that the profile entry lives in
lib/tab-press-listeners.ts.apps/mobile/src/lib/profile-tab-anchor.test.ts: replaces the source-reading pinsmainadded with assertions on the listener itself — cross-tab press, root re-press, and the
pop from a screen above the root.
Checks on the merged tree:
pnpm --filter kilo-app run typecheckclean,pnpm --filter kilo-app run lintclean, andpnpm --filter kilo-app test— 1256 files, 18610 tests passed, 2 skipped.Re-verified on the Android emulator on the merged tree, German UI, test organization selected
(the surface gate consumes the stray tap, the same sequences that used to open the composer):
button), 4 runs: stayed on the Profile tab every time, no composer;
the Start tab, no code reviewer opened;
CODE-PRÜFERopens the platform list for theselected organization;
Screenshots of the Profile rows, the opened platform list, and the state after a quick switch
tap are posted as a comment on this PR.
Review follow-ups
The first review round tightened three things, all in the same window:
left can lay out again mid-switch;
next tick, and a press on the already-focused tab when its stack transition ends
(
transitionEnd), with a 500 ms cap when there is no transition at all;lib/tab-press-listeners.ts, where the profileentry's behaviour is asserted directly instead of by reading the layout source.
Re-verified on the emulator after these changes: quick taps at 0 / 200 / 400 / 800 ms
stayed on the Profile tab (7 of 7 runs, no composer); a settled tap opened the platform
list; the new-task button opened the composer; and a pop back to the tab root followed
by a stray tap at the row point stayed on the tab root (2 of 2 runs).