feat(mobile): add native Android UI and shared chat controls (CODE-588) - #521
feat(mobile): add native Android UI and shared chat controls (CODE-588)#521Zerlight wants to merge 37 commits into
Conversation
@expo/ui 57.0.7+ Kotlin imports expo-modules-core's ColorCompat, which expo 57.0.4 does not ship - every Compose surface crashed at mount.
A bare Host and argless useMaterialColors follow the system scheme, so Compose text went dark-on-dark when the in-app appearance preference diverged from the system.
RN-drawn Android chrome (headers, backgrounds, header controls, chips, status dots, prompt cards) read app brand tokens; they now follow the same dynamic Material palette as the Compose surfaces via a platform-split useChromeColors. iOS keeps its tokens.
The tab overflow jumped straight to settings and the session menu degraded to an RN alert dialog; both now open an MD3 DropdownMenu anchored to the bar button, like the iOS UIMenu items.
…st sheet One useNativePalette contract resolves to UIKit PlatformColors on iOS and Material You roles on Android; chrome, the session surfaces, and the first-run screens (splash, sign-in, terminal) consume it instead of brand tokens. Android search hides behind a header icon and expands as a full-width view replacing the top app bar; the manual add-host flow joins the switchers' ModalBottomSheet family.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Recommended changes — one brand-glyph bug, one self-contradicting convention doc, plus parity nits and two dead entries.
Read the full diff (203 files, 12,091 lines) end-to-end. The platform seam is executed cleanly and consistently: the .ios.tsx / bare-.tsx split, the *.types.ts / *.shared.ts extraction, and the ~11 shared runtime/ hooks mean the two platform views genuinely share data and actions rather than drifting. ThemedHost/useAppMaterialColors are used without a single bypass, and the Compose invariants this PR documents (Switch onCheckedChange, selectable/selectableGroup, RowActions child click handlers, the matchContents collapse trap) hold everywhere I checked. The mock:-in-release-schema reasoning in host-store.ts and the __DEV__-gated require are both correctly argued in their comments.
Six anchored comments below. A number of things that looked like problems turned out to be fine and I want to record them so they don't get re-litigated: the pre-PR timeline-item.tsx approval branch styled every resolved approval with border-warning/40 bg-warning/10 and printed no outcome, so the new TranscriptRecord (which surfaces the option name vs dismissed) is strictly more informative, not a regression; allow_always landing on TextButton matches this PR's own AGENTS.md:66 ("one-time approval emphasized"); agent-opencode.xml's suspiciously short path is the genuine upstream mark; the dropped Math.min clamp in use-question-response.ts is dead defence because QuestionPrompt is keyed by question.requestId; .reverse() is safe on a fresh .filter() array with React Compiler off for this app; and removing mobile.connection.connecting is safe because the Android host-connection-state.tsx returns <LoadingView /> for that status. i18n is fully clean in both locales.
Test coverage (no line to anchor to). This adds roughly 40 Compose components and exactly one new test (use-question-response.test.ts). Given the repo's own constraints — Compose components can't be exercised in the mobile vitest project, and maestro e2e is Darwin/iOS-gated — the entire Android UI ships with no automated coverage, and none of the six findings below would have been caught by check:ci or pnpm test. That's a reasonable place to land for a first Android pass, but per the repo's Verification rule ("a change isn't done until you've observed it running") this needs a deliberate emulator walkthrough of every new screen before merge, not just a green CI.
Two smaller notes. (1) This is stacked on the sibling branch ruocheng/code-587, so the diff is against #520, not master — merge order across #519 → #520 → #521 matters and the reviewable content changes if anything lands out of order. (2) icon-button.tsx:32 and primary-action-fab.tsx:17 render <Icon> without a tint while the XMLs bake #FF000000 / #FFFFFFFF fills. Material3's Icon defaults tint to LocalContentColor and icon-button.tsx does set colors={{ contentColor }}, so inheritance most likely covers it — I'm flagging it only because the failure mode is an invisible glyph in one theme and it costs one glance on the emulator to confirm.
Claude Opus ｜ 𝕏
| <path | ||
| android:fillColor="#FFFFFFFF" | ||
| android:fillType="evenOdd" | ||
| android:pathData="M20.998,10.949H24v3.102h-3v3.028h-1.487V20H18v-2.921h-1.487V20H15v-2.921H9V20H7.488v-2.921H6V20H4.487v-2.921H3V14.05H0V10.95h3V5h17.998v5.949zM6,10.949h1.488V8.102H6v2.847zm10.51,0H18V8.102h-1.49v2.847z" /> |
There was a problem hiding this comment.
This path is a pixel-art robot face, but the Claude Code mark everywhere else in the repo is the Anthropic "A" — packages/presentation/ui/src/native/agent-icon.tsx:11-12 starts M4.709 15.955l4.72-2.647…. Since thread-row.ios.tsx:53 renders that same AgentIcon, an Android user and an iOS user looking at the same claude-code thread see two different brand marks.
What makes this look like a substitution slip rather than a deliberate choice: agent-codex.xml and agent-opencode.xml both match agent-icon.tsx's GLYPH_PATHS character-for-character, so the other two were clearly derived from it and only this one wasn't. Relatedly, thread-row.tsx:13 says these are "vendored as vector drawables from @proj-airi/lobe-icons (the web glyph set)" — that's accurate for codex and opencode but not for claude-code, which has no entry in lobe-icons' icons.json at all. Worth converting the agent-icon.tsx path and fixing the comment to say where the claude-code mark actually comes from.
| Platform views share `runtime/` hooks for data and actions. Keep `.ios.tsx` responsible for | ||
| SwiftUI and the Android implementation responsible for Compose; shared routes and runtime must | ||
| not import either platform's UI package. Android hosts use `ThemedHost` so Material colors follow | ||
| the app's appearance preference. Each visible list row must be a direct `LazyColumn` child. |
There was a problem hiding this comment.
This sentence contradicts form/list.android.tsx:7, added in the same PR: "LazyColumn maps each direct child to one lazy item, so keep children at section granularity."
The implementation follows the docstring, not this line — FormSection (form/section.android.tsx:23) wraps its children in a Column, so on every FormList-backed screen (settings, terminal appearance, threads, connect sections, account sections, terminals) the rows are grandchildren of LazyColumn, never direct children. The code is internally consistent and, I think, correct; it's this rule that's misstated. As written, the next contributor who reads AGENTS.md alone will conclude the whole PR is already in violation and try to "fix" it by hoisting rows out of their sections, which would cost the section chrome and turn every row into its own lazy item.
| the app's appearance preference. Each visible list row must be a direct `LazyColumn` child. | |
| the app's appearance preference. `LazyColumn` children stay at section granularity — one `FormSection` | |
| per lazy item, with rows nested inside it. |
| color={colors.onSurfaceVariant} | ||
| modifiers={[weight(1)]} | ||
| > | ||
| {t('questionProgress', { current, total })} |
There was a problem hiding this comment.
This renders unconditionally, so a single-question prompt shows "1 of 1" — and it's holding a weight(1) slot, so it also eats row space that the label could use. The iOS twin guards it at question-page.ios.tsx:90 with {total > 1 ? … : null}. Worth matching that here; note prompt-dock.tsx:89 already gets this right for the approvals pager (approvals.length > 1), so this is the odd one out rather than a deliberate Android convention.
| <ListItem | ||
| modifiers={[ | ||
| clickable(() => { | ||
| if (!busy) confirmDelete(); |
There was a problem hiding this comment.
busy gates the handler but nothing visual, so during an in-flight deletion the row stays fully tap-tinted and repeated taps land silently with no feedback. The iOS twin uses disabled(busy) (delete-account-section.ios.tsx:21), which both grays the label and refuses input.
Compose's ListItem has no disabled, which is presumably why it ended up as a handler guard — but the label color is already computed here, so dimming it when busy (e.g. colors.onSurfaceVariant instead of colors.error) would restore the parity cheaply. Worth doing on the highest-consequence, least-reversible action in the app.
| @@ -0,0 +1,9 @@ | |||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | |||
| android:width="24dp" | |||
There was a problem hiding this comment.
This asset has no importers anywhere under apps/mobile — the Android search affordance in use-trailing-actions.tsx uses a Lucide SearchIcon in an RN Pressable, and use-search-header-options.tsx uses Lucide's ArrowLeftIcon. The other 14 XMLs in this directory are all wired up; this one looks like it was added in anticipation and then not used. Either drop it or switch the search button over to it for consistency with the other Compose icon call sites.
| "expo-sharing": "~57.0.13", | ||
| "expo-sqlite": "~57.0.1", | ||
| "expo-status-bar": "~57.0.1", | ||
| "expo-symbols": "~57.0.2", |
There was a problem hiding this comment.
Nothing under apps/mobile/src imports from expo-symbols — the dependency is declared but unused, which widens the native build fingerprint for no benefit.
Also worth noting it cuts against the repo's own guidance: .agents/skills/expo-native-ui/SKILL.md:72 says to use "expo-image with source="sf:name" for SF Symbols, not expo-symbols or @expo/vector-icons". If this is intentional groundwork for a later change, it likely also needs an app.json plugin entry the way expo-font has one (app.json:90) — without it the package would silently not work on first use. Otherwise it should come back out.
1 participant
Summary
Provide native Android screens with Jetpack Compose and Material 3 Expressive while retaining SwiftUI views on iOS. Split presentation at platform boundaries and share runtime state, selection, connection, and response logic.
Part 3 of the mobile stack:
master→ #519 → #520 → #521. Depends on #520; base:ruocheng/code-587. The stack includes latest master (da9c0673). Review and merge after #519 and #520.Refs CODE-588.
Verification
devenv shell -- pnpm check:cidevenv shell -- pnpm test33bd138a.Runtime verification used the in-process mock host. Real harness/network integration and a comprehensive screen-by-screen accessibility audit remain outside this smoke check. Further visual design refinement remains open.
Checklist
pnpm check:ciandpnpm testpass