Skip to content

fix(mobile): keep tab screen content reachable above the tab bar - #8079

Merged
iscekic merged 9 commits into
mainfrom
drain/tab-screen-tab-bar-clearance
Oct 21, 2026
Merged

iscekic merged 9 commits into
mainfrom
drain/tab-screen-tab-bar-clearance

Conversation

@iscekic

@iscekic iscekic commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • The profile Environment variables screen (and seven sibling profile tab screens) scrolled in a plain ScrollView, so their content could sit behind the absolute bottom tab bar; the inline add/edit form's Cancel/Save buttons ended up under the bar with only a sliver above it.
  • The Environment variables, Commands, Agents, MCP servers, Skills, Slash commands, Repo bindings, and New profile screens now scroll in TabScreenScrollView, the shared tab-screen scroller whose viewport ends at the tab bar's top edge, matching the Profile overview and the documented rule in apps/mobile/AGENTS.md ("Use TabScreenScrollView for scrolling screens under the absolute tab bar").
  • The keyboard handling (automaticallyAdjustKeyboardInsets, keyboardShouldPersistTaps) is preserved on every migrated screen; TabScreenScrollView supports both.
  • The three profile form sheets (agent, MCP, slash command) are full-window native-sheet surfaces, not tab screens; they keep their own ScrollView inside the sheet.
  • The Code Reviewer Review memory proposal list now ends with the shared tab bar clearance (useTabBarBottomPadding), the same way the sibling review list does, so the last proposal scrolls clear of the bar.
  • The Passkeys screen keeps its Add control in a footer that reserves the tab bar and scrolls the body above it. In a phone held sideways that body band is shorter than the empty state's icon bubble plus its copy, so only the key icon rendered and the title and hint started below the fold. The screen now renders the compact empty state there, and a compact state pinned to the top of its caller's layout no longer spends a fixed 64pt of air on a band that has none to spare.

Investigation notes

  • Static review of the other three reported surfaces on main:
    • Home renders inside TabScreenScrollView, whose frame margin already ends the viewport at the tab bar's top edge (components/tab-screen.tsx).
    • The Agents empty state renders through EmptyState/CenteredState with the screen's StateSurfaceInsets band and a compact presentation decision for short bands.
  • Android E2E on the emulator (signed in, portrait + landscape, default density and wm density 560):
    • Home in landscape scrolls its full content; at scroll end the last row's bottom sits well above the tab bar's top edge. The task buttons and the EXPLORE section are reachable by scrolling in both densities. The reported "content does not scroll" behavior did not reproduce.
    • The Agents empty state in landscape at default density renders both actions fully above the tab bar (New session button and Session history link bottom y≈792 px vs tab bar top y≈961 px). At wm density 560 the state's content is taller than the space above the bar: its centered form overlaps the bar at rest and the state scrolls, so both actions become fully visible and tappable after one swipe. The reported "content does not scroll" behavior did not reproduce at either size.
  • Three of the four reported findings did not reproduce on main — the Home tab-bar clipping, the Agents empty-state clipping (both actions render above the bar at default density and the state scrolls at large display size), and the Home no-scroll finding; the confirmed variables-screen defect is fixed here.
  • Per the approved plan, the confirmed defect is fixed here; the three non-reproducing findings are documented rather than changed.

Verification

  • Android emulator on this VM against the local dev stack (pnpm dev:start mobile, Metro, Postgres), owned fake account via the app's email-code sign-in, optional analytics off.
  • Profile → Profiles → new profile → Environment variables: added a variable, opened the inline edit form. Portrait: buttons fully visible above the bar. Landscape after the fix, measured from the accessibility tree: buttons bottom ≈ y865 vs tab bar top y961, fully above the bar and tappable; variable persisted in Postgres.
  • Home exercised in landscape at default density and at wm density 560 (reset afterwards): full content reachable by scroll, last row clear of the bar at scroll end. With a seeded to-do item the Up Next section heading and its row render and scroll fully above the tab bar (heading bottom y≈875, row y≈708–764, bar top y≈961). At wm density 560 the New coding task action sits in the scroll content rather than pinned: when scrolled into view its bounds are y653–807 against a bar top of y807 and it moves up with further scrolling, so it is never held under the bar; the EXPLORE cards below it end at y751. Agents exercised in landscape at default density: empty-state actions fully above the bar.
  • Code Reviewer → Review memory with 20 seeded proposals, same account and stack. Landscape before this commit: at maximum scroll the last row's repo line reached y907 with the tab bar top at y875, so the final row sat 32 px under the bar. After this commit (fresh Metro bundle): the last row ends at y804, 71 px above the bar. Portrait after the fix: the last row ends at y2124 with the bar top at y2195. The list's later-page footer carries the same clearance.
  • Profile agents screen with six seeded agents: landscape at default density, the Add agent action's bounds were y617–732 against a bar top of y875, and tapping it opened the Add agent sheet in place; at wm density 560 (reset afterwards) the action was y463–617 against a bar top of y807 and the tap opened the same sheet.
  • Agents empty state at wm density 560 in landscape (reset afterwards): at rest the centered form spans y596–996 against a bar top of y807, and the state scrolls; after one swipe New session sits at y540–663 and Session history at y700–752, both above the bar, and tapping Session history opened the history screen.
  • New profile form in landscape: the Create profile action rendered at y608–723 against a bar top of y875. A tap with an empty name produced the form's name validation instead of reaching the tab bar; with a name the action created the profile and the row persisted in Postgres.
  • pnpm typecheck, pnpm lint, pnpm check:unused, pnpm format:check pass in apps/mobile; @kilocode/harness-sdk and @kilocode/trpc built first.
  • The affected mounted suites (variables, commands — including the save-on-exit suite — mcp, skills, repo-bindings, new-profile, review-memory, passkeys) pass with the scroller and clearance hooks mocked at the module boundary; pnpm --filter kilo-app run test is 1207 files, 18063 tests, all passing. The new passkeys landscape suite covers the portrait, phone-landscape and tablet-landscape presentations.
  • main is merged in. The single conflicting file (the commands screen) keeps the newer save-on-exit header and back handler from main together with the shared scroller, and its new save-on-exit suite mocks the scroller the same way its sibling suites do.
  • Passkeys empty state, same account and stack, Android landscape at wm density 560 (reset afterwards), rotated with the screen open: before the change, with no scroll applied, the title was clipped at the body's bottom edge (its accessibility bounds ended at y=560, the body viewport's edge) and the hint started at y=598, below it; only the key icon was painted, and the copy appeared only after a scroll. After the change the title occupies y≈347–433 and the hint y≈445–507 inside the body, above the footer Add control at y≈597 and the tab bar at y≈807, with no scroll. Portrait keeps the full stack (icon bubble at y≈625–709, title y≈802–888, hint y≈900–1023), and landscape at default density keeps the copy visible at y≈261–381.
  • No new dependencies; no i18n copy changes; mobile changelog is release-generated and intentionally unchanged.
  • Android verified on the emulator; no native changes.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Scope: The profile-variables screen provably lacks tab-bar clearance on main (plain ScrollView, violates the documented TabScreenScrollView rule). The Home and Agents findings contradict the existing TabScreenScrollView/CenteredState contracts in code, so they need live reproduction first. Scope for this PR?
    Answer: Fix all four in one PR (Recommended)
    Confidence: 0.80 · Precedent: Fix the root cause and the class of the issue on every affected surface: session 7f3470e0ba6f, "fix class of issue preventing progress"
  2. Question: Density: The findings say 'large text size'/'large display size', but apps/mobile/AGENTS.md forbids changing simulator font scale/Dynamic Type for testing. Verify the large-size cases via adb wm density (display size) instead?
    Answer: wm density via adb (Recommended)
    Confidence: 0.90 · Precedent: Never change simulator font scaling: session ad6a3f589364, "never mess with simulator font scaling, always use the default"
  3. Question: Non-repro: If a Home/Agents finding does not reproduce on main after real reproduction attempts (landscape, large density, both themes), what should the PR do?
    Answer: Ship confirmed fixes, document non-repro (Recommended)
    Confidence: 0.80 · Precedent: If a defect does not reproduce, ship the reasonable fix and close the task: session 9dc66123e7f3, owner chose "Merge fix(mobile): polish quick chat composer edge and session list loading rows #7613 and close this"
  4. Question: Fix shape: If reproduction confirms a Home or Agents defect, where should the fix land when the shared TabScreenScrollView/CenteredState layer already ends its viewport at the tab bar?
    Answer: Call-site fix over shared surgery (Recommended)
    Confidence: 0.60 · Precedent: Fix the root cause and the class of the issue: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class". The root cause is the caller that breaks the contract. Also move any other tab screens that use a plain ScrollView onto TabScreenScrollView.
  5. Question: Passkeys scope: The refreshed evidence now attaches a Passkeys empty-state finding (title and hint invisible at large display density in landscape) as a duplicate of this item. The passkeys screen is a tab screen that keeps its Add control in a footer above the tab bar and scrolls its body in a plain ScrollView, so the reported defect is that the body's top-placed empty state does not fit or scroll in a very short band — not a plain scroller sitting under the bar. Include it in this PR?
    Answer: Reproduce, fix only if real (Recommended)
    Confidence: 0.80 · Precedent: Fix the root cause and the class on every affected surface; also, if you saw a gap, fix it.

@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Oct 19, 2026
@kilo-code-bot

kilo-code-bot Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 409484af makes the Passkeys empty state compact in a short landscape window and removes the top pin's fixed air only for compact top-placed states; both changes are correctly scoped and the new suite covers portrait, phone-landscape and tablet-landscape presentations.

Assumptions: Verification is static; I did not run the mobile mounted suites. isShortViewport is documented for centered scrollers, so calling it from the plain footer-bounded body is a deliberate extension of the helper rather than a measured-band read — the resulting compact presentation is safe because it only drops decoration and its top air.

Files Reviewed (4 files)
  • apps/mobile/src/components/empty-state.tsx
  • apps/mobile/src/components/passkeys-screen.tsx
  • apps/mobile/src/components/passkeys-screen.test-helpers.ts
  • apps/mobile/src/components/passkeys-screen.landscape.mounted.test.tsx

Targeted current-code checks: empty-state.tsx:94 only drops pt-16 when compact is set, and the Passkeys screen is the only top-placed caller that passes compact (the Agents and session-list compact states use the default centered placement), so no existing caller changes. passkeys-screen.tsx:362 reads width/height from useWindowDimensions, which updates on rotation, and isShortViewport treats non-finite dimensions as never-short, so a partial platform mock cannot compact the state unexpectedly. The earlier CRITICAL on profile-variables-screen.tsx:211 remains resolved by the @/components/tab-screen mock at profile-variables-screen.mounted.test.tsx:61.

Previous Review Summaries (7 snapshots, latest commit 92879f5)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 92879f5)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 92879f574 adds the same @/components/tab-screen module mock used by the sibling profile suites to the command save suite, completing the fix for the previously reported test-rendering failure; no new issues found.

Assumptions: none. Verification is static; I did not run the mobile mounted suites. profile-agents-screen.tsx and profile-kilo-commands-screen.tsx have no mounted suites in the repo, so no additional mocks are required for them.

Files Reviewed (1 file)
  • apps/mobile/src/components/profiles/profile-commands-screen.save.mounted.test.tsx

Targeted current-code checks: profile-variables-screen.mounted.test.tsx:61 now mocks @/components/tab-screen, resolving the earlier CRITICAL inline comment on profile-variables-screen.tsx:211; profile-commands-screen.tsx imports only TabScreenScrollView from that module, so the new mock is complete.

Previous review (commit 7656f56)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 7656f56 adds tab-bar clearance to the Review memory proposal list by rendering the shared useTabBarBottomPadding() spacer inside ListFooterComponent, mirroring the sibling review list exactly, and mocks the hook at the module boundary in the mounted suite; no new issues found.

Files Reviewed (2 files)
  • apps/mobile/src/components/code-reviewer/review-memory-screen.tsx
  • apps/mobile/src/components/code-reviewer/review-memory-screen.mounted.test.tsx

Previous review (commit f8bb5b1)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit f8bb5b10 moves seven remaining profile tab screens onto TabScreenScrollView and adds the matching @/components/tab-screen mock to each mounted suite that renders them; the swaps preserve every existing scroll prop, every affected suite has a mock, and the previously reported render failure remains fixed.

Files Reviewed (12 files)
  • apps/mobile/src/components/profiles/new-profile-screen.tsx
  • apps/mobile/src/components/profiles/new-profile-screen.mounted.test.tsx
  • apps/mobile/src/components/profiles/profile-agents-screen.tsx
  • apps/mobile/src/components/profiles/profile-commands-screen.tsx
  • apps/mobile/src/components/profiles/profile-commands-screen.mounted.test.tsx
  • apps/mobile/src/components/profiles/profile-kilo-commands-screen.tsx
  • apps/mobile/src/components/profiles/profile-mcp-screen.tsx
  • apps/mobile/src/components/profiles/profile-mcp-screen.mounted.test.tsx
  • apps/mobile/src/components/profiles/profile-skills-screen.tsx
  • apps/mobile/src/components/profiles/profile-skills-screen.mounted.test.tsx
  • apps/mobile/src/components/profiles/repo-bindings-screen.tsx
  • apps/mobile/src/components/profiles/repo-bindings-screen.mounted.test.tsx

Previous review (commit fe3cb72)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit fe3cb724 changes no files — its tree is identical to the previously reviewed 69ad12a — so the TabScreenScrollView swap and the accompanying test mock remain as previously verified, with no new findings.

Files Reviewed (2 files)
  • apps/mobile/src/components/profiles/profile-variables-screen.tsx
  • apps/mobile/src/components/profiles/profile-variables-screen.mounted.test.tsx

Previous review (commit 69ad12a)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit only adds a documented eslint-disable max-lines exemption at the top of profile-variables-screen.mounted.test.tsx, which follows the established pattern in other mobile test suites; the previously reported test-mock failure remains resolved.

Files Reviewed (1 file)
  • apps/mobile/src/components/profiles/profile-variables-screen.mounted.test.tsx

Previous review (commit 646d891)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit adds a @/components/tab-screen mock (TabScreenScrollView: 'ScrollView') to profile-variables-screen.mounted.test.tsx, which resolves the previously reported render failure and matches the pattern already used by sibling mobile suites.

Files Reviewed (1 file)
  • apps/mobile/src/components/profiles/profile-variables-screen.mounted.test.tsx

Previous review (commit 8dd833b)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 0
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
apps/mobile/src/components/profiles/profile-variables-screen.tsx 211 Switching to TabScreenScrollView breaks the existing mounted suite: profile-variables-screen.mounted.test.tsx mocks react-native without Platform and does not mock @/components/tab-screen or react-native-safe-area-context, so the real scroller throws at render and every test in the file fails under the required mobile-test CI check.
Files Reviewed (1 file)
  • apps/mobile/src/components/profiles/profile-variables-screen.tsx - 1 issue

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 75.8K · Output: 8.7K · Cached: 1M

Review guidance: REVIEW.md from base branch main

@iscekic

iscekic commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android verification on this VM against the local dev stack, owned fake account, analytics off.

Environment variables inline form after the fix: the Cancel/Save buttons sit fully above the tab bar in portrait and landscape. In landscape the buttons' bottom edge measures y≈865 against the tab bar's top at y≈961.

Home in landscape at large display size scrolls its full content; at scroll end the last row clears the bar. The two reported Home findings and the Agents empty-state finding did not reproduce on this build; details in the PR body.

Environment variables form in portrait with buttons above the tab bar

Environment variables form in landscape with buttons above the tab bar

Home scrolled to the end in landscape at large display size

@iscekic

iscekic commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Added the missing Up Next observation: with a seeded to-do item, Home in landscape renders the Up Next heading and its row; one scroll brings both fully above the tab bar (heading bottom y≈875, row y≈708–764, bar top y≈961). At rest the heading clears the bar; the row below it scrolls up normally.

Home Up Next section scrolled above the tab bar in landscape

@iscekic iscekic added merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:human Substantive functional risk requires human review. and removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:human Substantive functional risk requires human review. labels Oct 19, 2026
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android verification of the profile agents tab screen on this VM, against the local dev stack: owned fake account, optional analytics off, six fixture agents on a test profile.

  • Landscape, default density: after scrolling the agents list to the end, the Add agent action's accessibility bounds were y=617-732 while the tab bar's top edge was y=875, so the action sits fully above the bar. Tapping it opened the Add agent sheet in place; it did not reach another tab.
  • Landscape, large display size (wm density 560, reset afterwards): the Add agent action's bounds were y=463-617 with the tab bar top at y=807. The tap still opened the same sheet.
  • Captures below show the scroll end with the action above the bar, and the sheet that the tap opened.

This is the surface where the earlier report placed the action under the bar and had the tap land on another tab.

Profile agents list scrolled to the end in landscape, add-agent action above the tab bar

Add agent sheet opened by that action in landscape

The proposal list now ends with the shared tab bar clearance, matching the review list and the other list screens, so the last proposal scrolls clear of the bar in both orientations.
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Review memory list verification on this VM's Android emulator, against the local dev stack: owned fake account, optional analytics off, 20 seeded proposal rows.

  • Landscape, maximum scroll, before this commit's change: the last proposal's repo line ended at y907 with the tab bar's top edge at y875, so the final row sat under the bar.
  • Landscape, maximum scroll, after the change (fresh Metro bundle): the last proposal ends at y804, clear of the bar at y875.
  • Portrait, maximum scroll, after the change: the last proposal ends at y2124, clear of the bar at y2195.
  • The later-page footer (retry or loading) carries the same clearance, and the empty state keeps its centered surface, which already reserves the bar band.

The two captures show the end of the list in landscape and portrait after the change.

Review memory proposal list at maximum scroll in landscape, last proposal clear of the tab bar

Review memory proposal list at maximum scroll in portrait, last proposal clear of the tab bar

@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Two more surfaces exercised on this VM's Android emulator against the local dev stack, same owned fake account.

  • New profile form in landscape: the Create profile action rendered at y608–723 against the tab bar top at y875. Tapping it with an empty name produced the form's own name validation rather than reaching the tab bar; with a name typed, the same tap created the profile and the row persisted in Postgres.
  • Agents empty state at wm density 560 in landscape (reset afterwards): the empty state is scrollable. At rest its centered form spans y596–996 against a bar top of y807; after one swipe the New session action sits at y540–663 and Session history at y700–752, both fully above the bar, and tapping Session history opened the session history screen. The capture is the post-swipe state.

Agents empty state after one swipe at large display size in landscape, both actions above the tab bar

@iscekic iscekic added the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
…bar-clearance

# Conflicts:
#	apps/mobile/src/components/profiles/profile-commands-screen.tsx
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
The passkeys screen scrolls its body above a footer that already reserves
the tab bar, so in a phone held sideways the body band is shorter than the
empty state's icon bubble plus its copy: only the key icon rendered and the
title and hint started below the fold. The screen now renders the compact
empty state there, and a compact top-pinned state drops the fixed top air.
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator against the local dev stack, signed in with an owned test account (optional analytics off), Profile → Preferences → Account → Passkeys with no stored passkeys, then rotated to landscape at wm density 560.

  • Before the change, with no scroll applied, only the key icon was painted: the title was clipped at the body's bottom edge (accessibility bounds ended at y=560, the body viewport edge) and the hint began at y=598, so the copy needed a scroll to appear.
  • After the change, still with no scroll applied, the title reads at y≈347–433 and the hint at y≈445–507 inside the body, above the footer Add a passkey control (y≈597) and the tab bar (y≈807). Portrait keeps the full stack with the icon bubble, and landscape at the default density keeps the copy visible.

Density was reset and the device returned to portrait afterwards. The account is a locally seeded test user on this VM; no production data is shown.

Landscape passkeys screen before the fix, showing only the key icon

Landscape passkeys screen after the fix, showing the title and hint above the add control

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants