Skip to content

fix(mobile): enlarge organization rename control tap target - #6370

Closed
iscekic wants to merge 1 commit into
mainfrom
kwf/explorer-a-control-is-too-small-to-tap-reliably-78c35a25-89e7
Closed

iscekic wants to merge 1 commit into
mainfrom
kwf/explorer-a-control-is-too-small-to-tap-reliably-78c35a25-89e7

Conversation

@iscekic

@iscekic iscekic commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • The organization screen's rename pencil button is now a 44×44dp target, clearing the 28dp minimum.
  • The button keeps its size beside a long organization name; the name truncates to one line instead.

Changelog for maintainers

  • The rename control in the organization eyebrow row gains h-[44px] w-[44px] shrink-0 items-center justify-center, and the hitSlop={12} workaround is removed.
  • The row sets min-h-[44px] and gives up its own py-3; that padding moves to the truncated name Text, so the target grows without changing the row height.
  • Rename visibility is unchanged: it still follows the money-role gate, and members or roles without billing access render no control.
  • New mounted test renders the screen with mocked dependencies, covers owner/admin/billing_manager and member/boundary states, and compiles the rendered classes through the Tailwind/NativeWind pipeline to prove h-[44px]/w-[44px] resolve to 44 native units.
  • Review hint: start at the eyebrow row of the organization card in apps/mobile/src/components/organization/hub-screen.tsx; the balance and seats rows sit in the same card, so confirm their spacing is unchanged.
  • Review hint: the supplied device capture reports an unrecorded platform while the request asked for Android, so Android target-size coverage is not proven by it.

E2E proof

[e1] controls below 28dp on a side: manage-org rename control — android emulator-5554, manage-org screen reached by signing in as the pack's org-owner account (owner/admin/billing_manager gate the rename control); appium hierarchy shows content-desc="Rename organization" bounds="[882,344][997,460]" = 115x116px at "displayDensity":420 (2.625) = 43.8x44.2dp >= 28dp, min over all clickable controls 36.19dp; tapping it opened the rename dialog (e1-tap-target.log); no UX-DEFECT observed.

[e1] controls below 28dp on a side: manage-org rename control — e2e-mobile-app/e1-hierarchy.png

E2E proof — log excerpts

[e1] controls below 28dp on a side: manage-org rename control -> pass :: android emulator-5554, manage-org screen reached by signing in as the pack's org-owner account (owner/admin/billing_manager gate the rename control); appium hierarchy shows content-desc="Rename organization" bounds="[882,344][997,460]" = 115x116px at "displayDensity":420 (2.625) = 43.8x44.2dp >= 28dp, min over all clickable controls 36.19dp; tapping it opened the rename dialog (e1-tap-target.log); no UX-DEFECT observed.
/home/igor_kilocode_ai/.local/share/kwf/sections/explorer-a-control-is-too-small-to-tap-reliably-78c35a25-89e7/e2e-mobile-app/e1-tap-target.log
# e1 android emulator-5554 — manage-org rename control tap target
# source: e1-hierarchy-manage-org.xml (appium hierarchy of the manage-org screen)
<redacted>
# source: e1-logcat.txt (device session info from the appium session)
"displayDensity":420
# computed: width 997-882=115px, height 460-344=116px; density 420dpi/160 = 2.625; 115/2.625=43.81dp, 116/2.625=44.19dp
# every clickable control on the manage-org screen, min side in dp (density 2.625):
   38.48dp  101x102px  Go back
   43.81dp  115x116px  Rename organization
   36.19dp  204x95px  Add credits
   51.43dp  914x135px  Members
   51.43dp  914x135px  Credit activity
   51.43dp  914x135px  Invoices
   54.86dp  914x144px  Kilo Pass. Not subscribed
   54.10dp  914x142px  Low balance alert, Off
   54.10dp  360x142px  Home, tab, 1 of 3
   54.10dp  360x142px  Agents, tab, 2 of 3
   54.10dp  360x142px  Profile, tab, 3 of 3
# min over controls: 36.19dp  (threshold 28dp)
Owner request

Explorer finding: a control is too small to tap reliably

The user-agent explorer found this while using the app like a user.
One finding per item; the explorer never edits product code.

Flow: accessibility
Found on revision: 59ef790

Repro:

  1. set this state first: account e2e-mobile-cloud-android@example.com signed in, $10 credits, 6 Code Reviewer pull requests, a second organization to switch to, day mode, display density 320, battery 90% charging, portrait, app cold-started via the Expo dev launcher.; the device in light mode
  2. open the app on emulator-5554
  3. reach the screen the capture names (the screen the capture names)
  4. the capture shows the defect named below

Observed: controls below 28dp on a side: manage-org: 1 small control(s): إعادة تسمية المؤسسة (32x32px)
Expected: every control is at least 28dp on a side

Evidence (from the device run):

Follow-ups (not changed here)

  • not proved live: Observed: controls below 28dp on a side: manage-org: 1 small control(s): إعادة تسمية المؤسسة (32x32px) Expected: every control is at least 28dp on a side (no capture cited it)

Open findings (not fixed here)

  • not proved live: account-arabic.png is no longer on the host that took it, so no publish can carry it
  • not proved live: account-switcher.png is no longer on the host that took it, so no publish can carry it
  • not proved live: agents-tab.png is no longer on the host that took it, so no publish can carry it
  • not proved live: code-reviewer.png is no longer on the host that took it, so no publish can carry it

@iscekic
iscekic marked this pull request as draft September 19, 2026 14:11
@kilo-code-bot

kilo-code-bot Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The PR diff is limited to the organization hub rename tap target, a new mounted test, and an inert case-guard allowlist description; the PR files are unchanged since the prior review (the delta was a rebase onto newer main), and the changed lines remain correct.

Files Reviewed (3 files)
  • apps/mobile/src/components/organization/hub-screen.tsx
  • apps/mobile/src/components/organization/hub-screen.mounted.test.tsx
  • apps/mobile/src/lib/case-guard.test.ts
Previous Review Summaries (3 snapshots, latest commit be3a378)

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

Previous review (commit be3a378)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change since the prior review only removed the unrelated passkey classification tests from the PR; the organization rename control's unconditional 44x44 native-unit target, the py-3 shift from the row to the truncating name, and the case-guard allowlist entry from the earlier review are unchanged and remain correct. No new issues found in the changed lines.

Files Reviewed (3 files)
  • apps/mobile/src/components/organization/hub-screen.tsx
  • apps/mobile/src/components/organization/hub-screen.mounted.test.tsx
  • apps/mobile/src/lib/case-guard.test.ts

Previous review (commit 22653f3)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (4 files)
  • apps/mobile/src/components/organization/hub-screen.tsx
  • apps/mobile/src/components/organization/hub-screen.mounted.test.tsx
  • apps/mobile/src/lib/auth/passkey-client.test.ts
  • apps/mobile/src/lib/case-guard.test.ts

Previous review (commit 8a20cb0)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • apps/mobile/src/components/organization/hub-screen.tsx
  • apps/mobile/src/components/organization/hub-screen.mounted.test.tsx

Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the kwf/explorer-a-control-is-too-small-to-tap-reliably-78c35a25-89e7 branch from 8a20cb0 to 22653f3 Compare September 19, 2026 14:41
@iscekic

iscekic commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Rejected, no code change (kwf kwf-fix-platform-bd20).

Why: (already implemented, verified live: no change needed: The reported platform fork is absent: the cited compiler import is test-only, and the rename control already shares unconditional 44×44 native-unit sizing across iOS and Android. Existing implementation: apps/mobile/src/components/organization/hub-screen.tsx:109-126 provides one sh

@iscekic
iscekic marked this pull request as ready for review September 19, 2026 15:33
@iscekic

iscekic commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

kilo-review — independent audit of the published diff.

Status: 1 Issues

File Line Issue
apps/mobile/src/lib/auth/passkey-client.test.ts 273 This organization tap-target patch adds unrelated passkey classification tests and a matching case-guard exception.

@iscekic
iscekic marked this pull request as draft September 19, 2026 16:20
@iscekic

iscekic commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Accepted. Pushed 2588c34 (kwf kwf-fix-review-791b).

What changed:

  • apps/mobile/src/lib/auth/passkey-client.test.ts:273 — accepted: the unrelated passkey casing-test matrix is removed because it does not belong in this tap-target patch. The case-guard allowlist entry stays; the production classifier still lowercases the credential-error haystack to match it (lib/auth/passkey-client.ts:133).
  • The rename control in the organization eyebrow row gains h-[44px] w-[44px] shrink-0 items-center justify-center, and the hitSlop={12} workaround is removed.
  • The row sets min-h-[44px] and gives up its own py-3; that padding moves to the truncated name Text, so the target grows without changing the row height.
  • Rename visibility is unchanged: it still follows the money-role gate, and members or roles without billing access render no control.
  • New mounted test renders the screen with mocked dependencies, covers owner/admin/billing_manager and member/boundary states, and compiles the rendered classes through the Tailwind/NativeWind pipeline to prove h-[44px]/w-[44px] resolve to 44 native units.
  • Review hint: start at the eyebrow row of the organization card in apps/mobile/src/components/organization/hub-screen.tsx; the balance and seats rows sit in the same card, so confirm their spacing is unchanged.
  • Review hint: the supplied device capture reports an unrecorded platform while the request asked for Android, so Android target-size coverage is not proven by it.

@iscekic
iscekic force-pushed the kwf/explorer-a-control-is-too-small-to-tap-reliably-78c35a25-89e7 branch from 2588c34 to be3a378 Compare September 19, 2026 17:00
@iscekic
iscekic marked this pull request as ready for review September 19, 2026 17:13
@iscekic

iscekic commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

This description names a scenario the proof did not capture:

  • not proved live: Observed: controls below 28dp on a side: manage-org: 1 small control(s): إعادة تسمية المؤسسة (32x32px) Expected: every control is at least 28dp on a side (no capture cited it)

A repeated proof run rebuilds the same evidence, so no proof run is dispatched for a named gap. Merging with this gap open is your decision.

@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 19, 2026
@iscekic iscekic self-assigned this Sep 19, 2026
@iscekic iscekic removed the human-ready The PR is ready for human review. label Sep 19, 2026
@iscekic
iscekic marked this pull request as draft September 19, 2026 22:55
@iscekic
iscekic force-pushed the kwf/explorer-a-control-is-too-small-to-tap-reliably-78c35a25-89e7 branch from bcda937 to 90b6d06 Compare September 19, 2026 23:41
@iscekic
iscekic marked this pull request as ready for review September 19, 2026 23:50
@iscekic

iscekic commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

This description names a scenario the proof did not capture:

  • not proved live: Observed: controls below 28dp on a side: manage-org: 1 small control(s): إعادة تسمية المؤسسة (32x32px) Expected: every control is at least 28dp on a side (no capture cited it)

A repeated proof run rebuilds the same evidence, so no proof run is dispatched for a named gap. Merging with this gap open is your decision.

@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 20, 2026
@iscekic

iscekic commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as a duplicate of #6395.

This PR and ten others came from the same explorer finding, a-control-is-too-small-to-tap-reliably. Nine of the eleven edit apps/mobile/src/components/agents/session-filter-button.tsx, so they could never all merge — the first to land would conflict the rest.

#6395 is kept because it is the broadest of the group (14 files of the 34 the group touches).

Any screen still below the tap-target bar after #6395 merges will be found again by the same scan, and will now arrive as one finding: the intake refuses a finding whose slug already has an open PR or a live section (kwf 07c842fd).

@iscekic iscekic closed this Sep 20, 2026
@iscekic iscekic reopened this Sep 20, 2026
@iscekic

iscekic commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

Reopened — I closed this in error.

I closed it as a duplicate of #6395 on the grounds that eleven PRs shared one explorer finding. That was right about the finding and wrong about this PR: it is not a subset of #6395. It changes files #6395 does not touch, so closing it discarded real work rather than removing a copy.

Checked properly: of the ten I closed, only #6365 and #6363 are true subsets of #6395. The other eight, including this one, each fix a screen or a helper the keeper leaves alone.

What remains true: these PRs overlap on apps/mobile/src/components/agents/session-filter-button.tsx, so they will conflict with each other and cannot all merge as they stand. That is a sequencing problem to resolve, not a reason to throw the work away.

Apologies for the churn.

@iscekic

iscekic commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

Closing so this defect has one PR instead of nine.

All nine came from the same explorer finding, a-control-is-too-small-to-tap-reliably, on different screens. Nine of them edit apps/mobile/src/components/agents/session-filter-button.tsx, so the first to merge conflicts the rest — they were never all mergeable.

#6395 is kept as the broadest (14 files).

I reopened this PR earlier today after closing it, because it is not a subset of #6395 and I did not want to pretend otherwise. The work it carries is not being dropped: the six source files across this group that #6395 does not touch are now one queued item, tap targets on the screens the first pass did not cover, which names each component and the PR it came from —

  • components/agents/session-list-header-actions.tsx
  • components/login/idle-auth.tsx
  • components/organization/hub-screen.tsx
  • components/organization/members-screen.tsx
  • components/ui/icon-button.tsx
  • lib/a11y/tap-target.ts (the shared helper)

One change through the shared helper is a better answer than nine screen-by-screen edits that conflict.

Cause, now fixed: each finding was dispatched the moment it was found. The explorer pools findings for an hour and dispatches one item per defect (kwf d25ae7de), and the intake refuses a slug that already has open work (kwf 07c842fd).

@iscekic iscekic closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant