[codex] Add Android mobile support#3579
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const sfSymbol = typeof props.name === "string" ? props.name : props.name.ios; | ||
| const AndroidIcon = sfSymbol ? ANDROID_ICON_BY_SF_SYMBOL[sfSymbol] : undefined; | ||
|
|
||
| if (!AndroidIcon) { |
There was a problem hiding this comment.
🟡 Medium components/AppSymbol.tsx:128
When props.name resolves to an SF Symbol not present in ANDROID_ICON_BY_SF_SYMBOL (e.g. "sidebar.left", "arrow.up.left.and.arrow.down.right"), this branch returns props.fallback ?? null, silently dropping the icon on Android. Several screens already use unmapped symbol names, so those controls render with no icon at all on Android. The missing symbols need corresponding entries in ANDROID_ICON_BY_SF_SYMBOL, or a generic placeholder icon should be rendered here instead of null when no fallback is provided.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/AppSymbol.tsx around line 128:
When `props.name` resolves to an SF Symbol not present in `ANDROID_ICON_BY_SF_SYMBOL` (e.g. `"sidebar.left"`, `"arrow.up.left.and.arrow.down.right"`), this branch returns `props.fallback ?? null`, silently dropping the icon on Android. Several screens already use unmapped symbol names, so those controls render with no icon at all on Android. The missing symbols need corresponding entries in `ANDROID_ICON_BY_SF_SYMBOL`, or a generic placeholder icon should be rendered here instead of `null` when no fallback is provided.
| if (Platform.OS !== "android") return; | ||
| onExpandedChange?.(isExpanded); | ||
| }, [isExpanded, onExpandedChange]); |
There was a problem hiding this comment.
🟡 Medium threads/ThreadComposer.tsx:221
On Android, isExpanded stays true when the editor has content but is blurred, so the useEffect calls onExpandedChange(true). The parent (ThreadDetailScreen) then sets composerBottomInset to 0, removing the bottom safe-area padding even though the keyboard is dismissed — the blurred composer can overlap the system gesture/navigation area. Consider reporting isFocused to onExpandedChange instead of isExpanded, since the parent uses this signal to decide whether safe-area padding is needed.
if (Platform.OS !== "android") return;
- onExpandedChange?.(isExpanded);
- }, [isExpanded, onExpandedChange]);
+ onExpandedChange?.(isFocused);
+ }, [isFocused, onExpandedChange]);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadComposer.tsx around lines 221-223:
On Android, `isExpanded` stays `true` when the editor has content but is blurred, so the `useEffect` calls `onExpandedChange(true)`. The parent (`ThreadDetailScreen`) then sets `composerBottomInset` to `0`, removing the bottom safe-area padding even though the keyboard is dismissed — the blurred composer can overlap the system gesture/navigation area. Consider reporting `isFocused` to `onExpandedChange` instead of `isExpanded`, since the parent uses this signal to decide whether safe-area padding is needed.
| public func scrollViewWillBeginDragging(_ scrollView: UIScrollView) { | ||
| // A direct gesture takes ownership from any interrupted programmatic jump. | ||
| // Resume visible-file events immediately so the inspector follows the finger. | ||
| isProgrammaticScrollActive = false | ||
| contentView.isVerticalScrollActive = true | ||
| } |
There was a problem hiding this comment.
🟡 Medium ios/T3ReviewDiffView.swift:394
scrollViewWillBeginDragging clears isProgrammaticScrollActive but leaves pendingScrollFileId and pendingScrollAnimated set. If scrollToFile was queued before the target header existed (e.g. rows still decoding), a later setRowsJson calls applyPendingScrollIfNeeded() and jumps to the old target, overriding the user's manual scroll. Clear the pending scroll request when the user takes control.
| public func scrollViewWillBeginDragging(_ scrollView: UIScrollView) { | |
| // A direct gesture takes ownership from any interrupted programmatic jump. | |
| // Resume visible-file events immediately so the inspector follows the finger. | |
| isProgrammaticScrollActive = false | |
| contentView.isVerticalScrollActive = true | |
| } | |
| public func scrollViewWillBeginDragging(_ scrollView: UIScrollView) { | |
| // A direct gesture takes ownership from any interrupted programmatic jump. | |
| // Resume visible-file events immediately so the inspector follows the finger. | |
| isProgrammaticScrollActive = false | |
| pendingScrollFileId = nil | |
| pendingScrollAnimated = false | |
| contentView.isVerticalScrollActive = true | |
| } |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift around lines 394-399:
`scrollViewWillBeginDragging` clears `isProgrammaticScrollActive` but leaves `pendingScrollFileId` and `pendingScrollAnimated` set. If `scrollToFile` was queued before the target header existed (e.g. rows still decoding), a later `setRowsJson` calls `applyPendingScrollIfNeeded()` and jumps to the old target, overriding the user's manual scroll. Clear the pending scroll request when the user takes control.
| paddingTop: 8, | ||
| paddingBottom: target ? 0 : Math.max(insets.bottom, 18), | ||
| paddingTop: isAndroid ? insets.top + 8 : 8, | ||
| paddingBottom: target ? (isAndroid ? 72 : 0) : Math.max(insets.bottom, 18), |
There was a problem hiding this comment.
🟡 Medium review/ReviewCommentComposerSheet.tsx:174
The hard-coded 72 px paddingBottom on Android assumes the KeyboardStickyView footer is always 72 px tall, but the footer height is actually 44 (button) + 8 (pt-2) + Math.max(insets.bottom, 10). On devices where insets.bottom exceeds ~20 (e.g. iPhones with a home indicator, where it can be ~34), the footer is taller than 72 px and overlaps the bottom of the comment input and attachment strip, hiding them from view and making them untappable. The padding should be computed from the actual footer height instead of hard-coded.
- paddingBottom: target ? (isAndroid ? 72 : 0) : Math.max(insets.bottom, 18),
+ paddingBottom: target ? (isAndroid ? Math.max(insets.bottom, 10) + 52 : 0) : Math.max(insets.bottom, 18),🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/review/ReviewCommentComposerSheet.tsx around line 174:
The hard-coded `72` px `paddingBottom` on Android assumes the `KeyboardStickyView` footer is always 72 px tall, but the footer height is actually `44` (button) + `8` (`pt-2`) + `Math.max(insets.bottom, 10)`. On devices where `insets.bottom` exceeds ~20 (e.g. iPhones with a home indicator, where it can be ~34), the footer is taller than 72 px and overlaps the bottom of the comment input and attachment strip, hiding them from view and making them untappable. The padding should be computed from the actual footer height instead of hard-coded.
juliusmarminge
force-pushed
the
android-dev-pr-3514
branch
from
927d43b to
212ac28
Compare
juliusmarminge
changed the base branch from
main
to
t3code/ipad-responsive-mobile-layout
| backgroundColor: drawerBg, | ||
| paddingTop: insets.top + 10, | ||
| paddingBottom: Math.max(insets.bottom, 18), | ||
| paddingTop: insets.top, |
There was a problem hiding this comment.
🟡 Medium threads/ThreadNavigationDrawer.tsx:134
The PR removed paddingBottom: Math.max(insets.bottom, 18) from the drawer container, leaving the ScrollView's contentInset as the only source of bottom spacing. contentInset is iOS-only on React Native's ScrollView, so on Android the last thread rows render with no bottom padding and extend into the bottom safe area, making them partially obscured and harder to tap. Consider restoring paddingBottom on the container so the inset applies cross-platform.
| paddingTop: insets.top, | |
| paddingTop: insets.top, | |
| paddingBottom: Math.max(insets.bottom, 18), |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadNavigationDrawer.tsx around line 134:
The PR removed `paddingBottom: Math.max(insets.bottom, 18)` from the drawer container, leaving the `ScrollView`'s `contentInset` as the only source of bottom spacing. `contentInset` is iOS-only on React Native's `ScrollView`, so on Android the last thread rows render with no bottom padding and extend into the bottom safe area, making them partially obscured and harder to tap. Consider restoring `paddingBottom` on the container so the inset applies cross-platform.
| accessibilityLabel: `Archive ${props.thread.title}`, | ||
| icon: "archivebox", | ||
| label: "Archive", | ||
| onPress: props.onArchive, |
There was a problem hiding this comment.
🟡 Medium home/HomeScreen.tsx:299
The Archive swipe action passes props.onArchive directly as onPress, so tapping it does not close the swipeable row first. The Delete action calls swipeableMethods.close() before deleting, but Archive skips that step — the row stays open with action buttons visible, and openSwipeableRef in HomeScreen is never cleared via onSwipeableClose. Wrap the archive handler to close the swipeable first, matching the Delete behavior.
| onPress: props.onArchive, | |
| onPress: () => { | |
| swipeableRef.current?.close(); | |
| props.onArchive(); | |
| }, |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/home/HomeScreen.tsx around line 299:
The Archive swipe action passes `props.onArchive` directly as `onPress`, so tapping it does not close the swipeable row first. The Delete action calls `swipeableMethods.close()` before deleting, but Archive skips that step — the row stays open with action buttons visible, and `openSwipeableRef` in `HomeScreen` is never cleared via `onSwipeableClose`. Wrap the archive handler to close the swipeable first, matching the Delete behavior.
| view.setSpellCheck(spellCheck) | ||
| } | ||
|
|
||
| Events( |
There was a problem hiding this comment.
🟡 Medium t3composereditor/T3ComposerEditorModule.kt:48
The Events(...) declaration omits onComposerSubmit, and T3ComposerEditorView defines no corresponding EventDispatcher or hardware-keyboard handler. As a result, the onSubmit prop exposed by ComposerEditorProps is never invoked on Android — hardware-keyboard submit that works on iOS silently does nothing here. Consider adding "onComposerSubmit" to the Events list, wiring up an onComposerSubmit dispatcher in the view, and detecting the submit key combination (e.g., Enter without Shift) to fire it.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorModule.kt around line 48:
The `Events(...)` declaration omits `onComposerSubmit`, and `T3ComposerEditorView` defines no corresponding `EventDispatcher` or hardware-keyboard handler. As a result, the `onSubmit` prop exposed by `ComposerEditorProps` is never invoked on Android — hardware-keyboard submit that works on iOS silently does nothing here. Consider adding `"onComposerSubmit"` to the `Events` list, wiring up an `onComposerSubmit` dispatcher in the view, and detecting the submit key combination (e.g., Enter without Shift) to fire it.
| backgroundColorValue, | ||
| cursorColorValue, | ||
| paletteColors, | ||
| ) |
There was a problem hiding this comment.
🟡 Medium t3terminal/T3TerminalView.kt:252
createTerminal() assigns the result of GhosttyBridge.nativeCreate() directly to terminalHandle without checking for a 0 return value, which indicates native session creation failed. When this happens, terminalHandle stays 0L and emitResize() proceeds to fire onResize and call feedPendingBuffer()/renderSnapshot() as if the terminal were live, so the JS layer never learns creation failed and the view stays blank. Consider checking the return value and emitting an error event when it is 0.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt around line 252:
`createTerminal()` assigns the result of `GhosttyBridge.nativeCreate()` directly to `terminalHandle` without checking for a `0` return value, which indicates native session creation failed. When this happens, `terminalHandle` stays `0L` and `emitResize()` proceeds to fire `onResize` and call `feedPendingBuffer()`/`renderSnapshot()` as if the terminal were live, so the JS layer never learns creation failed and the view stays blank. Consider checking the return value and emitting an error event when it is `0`.
| ``` | ||
|
|
||
| The script downloads Zig 0.15.2 when needed, checks out the pinned upstream Ghostty revision, and | ||
| rebuilds all four Android ABIs with 16 KB page-size support. |
There was a problem hiding this comment.
🟡 Medium t3-terminal/README.md:50
The new README section states the script rebuilds "all four Android ABIs with 16 KB page-size support," but build-libghostty-android.sh never passes -Wl,-z,max-page-size=16384 and -Wl,-z,common-page-size=16384 to the linker. A developer following these instructions will rebuild and vendor .so files that are not 16 KB-page compatible while the docs claim otherwise. Add the required linker flags to the script so the rebuilt libraries match the documented behavior.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/README.md around line 50:
The new README section states the script rebuilds "all four Android ABIs with 16 KB page-size support," but `build-libghostty-android.sh` never passes `-Wl,-z,max-page-size=16384` and `-Wl,-z,common-page-size=16384` to the linker. A developer following these instructions will rebuild and vendor `.so` files that are not 16 KB-page compatible while the docs claim otherwise. Add the required linker flags to the script so the rebuilt libraries match the documented behavior.
| Prop("editable") { view: T3ComposerEditorView, editable: Boolean -> | ||
| view.setEditable(editable) | ||
| } | ||
| Prop("scrollEnabled") { view: T3ComposerEditorView, scrollEnabled: Boolean -> |
There was a problem hiding this comment.
🟡 Medium t3composereditor/T3ComposerEditorModule.kt:35
The scrollEnabled prop does not actually disable scrolling. setScrollEnabled only toggles editor.isVerticalScrollBarEnabled, which controls whether the scrollbar is drawn — not whether the view scrolls. When scrollEnabled={false} is passed from JS, the editor remains scrollable and only the scrollbar disappears, so the prop silently does not work. Consider disabling touch interception or overriding onTouchEvent to actually prevent scrolling when scrollEnabled is false.
Also found in 1 other location(s)
apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt:199
setScrollEnabled()at line199only assignseditor.isVerticalScrollBarEnabled. Android'ssetVerticalScrollBarEnabled()controls whether the scrollbar is drawn, not whether anEditTextcan actually scroll. When callers passscrollEnabled=false, long composer contents can still be vertically scrolled; only the scrollbar disappears, so the exposed prop does not work on Android.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorModule.kt around line 35:
The `scrollEnabled` prop does not actually disable scrolling. `setScrollEnabled` only toggles `editor.isVerticalScrollBarEnabled`, which controls whether the scrollbar is drawn — not whether the view scrolls. When `scrollEnabled={false}` is passed from JS, the editor remains scrollable and only the scrollbar disappears, so the prop silently does not work. Consider disabling touch interception or overriding `onTouchEvent` to actually prevent scrolling when `scrollEnabled` is false.
Also found in 1 other location(s):
- apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt:199 -- `setScrollEnabled()` at line `199` only assigns `editor.isVerticalScrollBarEnabled`. Android's `setVerticalScrollBarEnabled()` controls whether the scrollbar is drawn, not whether an `EditText` can actually scroll. When callers pass `scrollEnabled=false`, long composer contents can still be vertically scrolled; only the scrollbar disappears, so the exposed prop does not work on Android.
| </> | ||
| )} | ||
|
|
||
| <GitActionProgressOverlay progress={gitActionProgress} onDismiss={dismissGitActionResult} /> |
There was a problem hiding this comment.
🟡 Medium threads/ThreadRouteScreen.tsx:725
GitActionProgressOverlay positions itself at insets.top + 48, which was calibrated for the native iOS header. On Android, the new AndroidScreenHeader is Math.max(insets.top, 12) + 58 tall, so the overlay renders ~10px inside the header and overlaps the title and action buttons while a git action is running or after it completes. Consider passing a platform-aware top offset to GitActionProgressOverlay so it clears the Android header.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadRouteScreen.tsx around line 725:
`GitActionProgressOverlay` positions itself at `insets.top + 48`, which was calibrated for the native iOS header. On Android, the new `AndroidScreenHeader` is `Math.max(insets.top, 12) + 58` tall, so the overlay renders ~10px inside the header and overlaps the title and action buttons while a git action is running or after it completes. Consider passing a platform-aware top offset to `GitActionProgressOverlay` so it clears the Android header.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 6 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 6 issues found in the latest run.
- ✅ Fixed: iOS associated domains removed
- Restored the associatedDomains array with applinks and webcredentials entries for clerk.t3.codes, which was inadvertently dropped when relyingParty was removed from the variant config.
- ✅ Fixed: Composer scroll flag ignored
- Added a scrollEnabled property to SelectionAwareEditText with a scrollTo override that blocks scrolling when disabled, so setScrollEnabled now controls actual scroll behavior, not just the scroll bar.
- ✅ Fixed: Collapsed files skew highlight indices
- Added a visibleToOriginalIndex mapping array built during rebuildVisibleRows, and the onVisibleRowsChanged callback now translates filtered indices back to original row indices before emitting the visible-range event.
- ✅ Fixed: Stale rows after content reset
- setContentResetKey now increments rowsDecodeGeneration and clears rows, visibleRows, visibleToOriginalIndex, and canvasView.rows so in-flight decodes from the previous section are rejected and stale content is immediately cleared.
- ✅ Fixed: Top scroll never clears file
- emitVisibleFile now checks if verticalOffset is at zero and emits onVisibleFileChange with a null fileId in that case, matching the iOS behavior that selects the "All files" navigator destination.
- ✅ Fixed: Collapsed comments stay tall
- Added collapsedCommentIds to DiffCanvasView and updated rowHeight to return 44dp for collapsed comments (matching iOS's 44pt) instead of the full expanded height.
Or push these changes by commenting:
@cursor push a991bbde89
Preview (a991bbde89)
diff --git a/apps/mobile/app.config.ts b/apps/mobile/app.config.ts
--- a/apps/mobile/app.config.ts
+++ b/apps/mobile/app.config.ts
@@ -107,6 +107,7 @@
icon: variant.iosIcon,
supportsTablet: true,
bundleIdentifier: iosBundleIdentifier,
+ associatedDomains: ["applinks:clerk.t3.codes", "webcredentials:clerk.t3.codes"],
infoPlist: {
NSAppTransportSecurity: {
NSAllowsArbitraryLoads: true,
diff --git a/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt b/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
--- a/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
+++ b/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
@@ -197,6 +197,7 @@
fun setScrollEnabled(scrollEnabled: Boolean) {
editor.isVerticalScrollBarEnabled = scrollEnabled
+ editor.scrollEnabled = scrollEnabled
}
fun setAutoFocus(autoFocus: Boolean) {
@@ -424,7 +425,12 @@
private class SelectionAwareEditText(context: Context) : EditText(context) {
var selectionListener: ((Int, Int) -> Unit)? = null
var pasteImagesListener: ((List<String>) -> Unit)? = null
+ var scrollEnabled = true
+ override fun scrollTo(x: Int, y: Int) {
+ if (scrollEnabled) super.scrollTo(x, y)
+ }
+
override fun onSelectionChanged(selStart: Int, selEnd: Int) {
super.onSelectionChanged(selStart, selEnd)
selectionListener?.invoke(selStart, selEnd)
diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
--- a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
+++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
@@ -32,6 +32,7 @@
private val onToggleComment by EventDispatcher()
private var rows: List<DiffRow> = emptyList()
private var visibleRows: List<DiffRow> = emptyList()
+ private var visibleToOriginalIndex: IntArray = intArrayOf()
private var collapsedFileIds: Set<String> = emptySet()
private var viewedFileIds: Set<String> = emptySet()
private var selectedRowIds: Set<String> = emptySet()
@@ -56,11 +57,13 @@
init {
canvasView.onRowTap = { row, gesture -> handleRowTap(row, gesture) }
canvasView.onVisibleRowsChanged = { first, last ->
+ val originalFirst = if (first in visibleToOriginalIndex.indices) visibleToOriginalIndex[first] else first
+ val originalLast = if (last in visibleToOriginalIndex.indices) visibleToOriginalIndex[last] else last
onDebug(
mapOf(
"message" to "visible-range",
- "firstRowIndex" to first,
- "lastRowIndex" to last,
+ "firstRowIndex" to originalFirst,
+ "lastRowIndex" to originalLast,
),
)
emitVisibleFile(first)
@@ -81,7 +84,12 @@
fun setContentResetKey(value: String) {
if (contentResetKey == value) return
contentResetKey = value
+ rowsDecodeGeneration += 1
tokensDecodeGeneration += 1
+ rows = emptyList()
+ visibleRows = emptyList()
+ visibleToOriginalIndex = intArrayOf()
+ canvasView.rows = emptyList()
canvasView.tokensByRowId = emptyMap()
lastVisibleFileId = null
pendingInitialScroll = true
@@ -314,21 +322,27 @@
private fun rebuildVisibleRows() {
val filtered = ArrayList<DiffRow>(rows.size)
+ val indexMapping = ArrayList<Int>(rows.size)
var currentFileCollapsed = false
- rows.forEach { row ->
+ rows.forEachIndexed { originalIndex, row ->
if (row.kind == "file") {
currentFileCollapsed = collapsedFileIds.contains(row.resolvedFileId)
filtered.add(row)
+ indexMapping.add(originalIndex)
} else if (!currentFileCollapsed) {
if (row.kind != "comment" || !collapsedCommentIds.contains(row.id)) {
filtered.add(row)
+ indexMapping.add(originalIndex)
} else {
filtered.add(row.copy(commentText = "Comment collapsed"))
+ indexMapping.add(originalIndex)
}
}
}
visibleRows = filtered
+ visibleToOriginalIndex = indexMapping.toIntArray()
canvasView.rows = filtered
+ canvasView.collapsedCommentIds = collapsedCommentIds
canvasView.viewedFileIds = viewedFileIds
canvasView.selectedRowIds = selectedRowIds
applyPendingInitialScroll()
@@ -360,6 +374,13 @@
private fun emitVisibleFile(firstVisibleIndex: Int) {
if (visibleRows.isEmpty()) return
+ if (canvasView.verticalOffset() <= 0) {
+ if (lastVisibleFileId != null) {
+ lastVisibleFileId = null
+ onVisibleFileChange(mapOf("fileId" to null))
+ }
+ return
+ }
val start = firstVisibleIndex.coerceIn(0, visibleRows.lastIndex)
val fileId = (start downTo 0)
.asSequence()
@@ -590,6 +611,11 @@
field = value
invalidate()
}
+ var collapsedCommentIds: Set<String> = emptySet()
+ set(value) {
+ field = value
+ rebuildOffsets()
+ }
var theme: DiffTheme = DiffTheme.fallback("light")
set(value) {
field = value
@@ -691,7 +717,11 @@
private fun rowHeight(row: DiffRow): Int = when (row.kind) {
"file" -> style.fileHeaderHeightPx.toInt()
- "comment" -> max((style.rowHeightPx * 3.2f).toInt(), (56 * density).toInt())
+ "comment" -> if (collapsedCommentIds.contains(row.id)) {
+ (44 * density).toInt()
+ } else {
+ max((style.rowHeightPx * 3.2f).toInt(), (56 * density).toInt())
+ }
else -> style.rowHeightPx.toInt()
}.coerceAtLeast(1)You can send follow-ups to the cloud agent here.
|
|
||
| fun setScrollEnabled(scrollEnabled: Boolean) { | ||
| editor.isVerticalScrollBarEnabled = scrollEnabled | ||
| } |
There was a problem hiding this comment.
Composer scroll flag ignored
Medium Severity
On Android, setScrollEnabled only toggles the vertical scroll bar visibility, not the actual scrolling behavior of the EditText. This allows the editor to scroll even when scrollEnabled={false} is set, unlike iOS.
Reviewed by Cursor Bugbot for commit 276b453. Configure here.
| canvasView.setVerticalOffset(0) | ||
| canvasView.setHorizontalOffset(0) | ||
| applyPendingInitialScroll() | ||
| } |
There was a problem hiding this comment.
Stale rows after content reset
Medium Severity
When contentResetKey changes, the view clears tokens and scroll but does not bump rowsDecodeGeneration or clear rows. An in-flight setRowsJson decode from the previous review section can still post after the reset and repopulate the canvas with the old diff.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 276b453. Configure here.
| if (fileId == lastVisibleFileId) return | ||
| lastVisibleFileId = fileId | ||
| onVisibleFileChange(mapOf("fileId" to fileId)) | ||
| } |
There was a problem hiding this comment.
Top scroll never clears file
Medium Severity
Android never emits onVisibleFileChange with a null fileId when the diff is scrolled to the top. The navigator keeps highlighting the first file instead of the shared “all files” destination that iOS reports at offset zero.
Reviewed by Cursor Bugbot for commit 276b453. Configure here.
| "file" -> style.fileHeaderHeightPx.toInt() | ||
| "comment" -> max((style.rowHeightPx * 3.2f).toInt(), (56 * density).toInt()) | ||
| else -> style.rowHeightPx.toInt() | ||
| }.coerceAtLeast(1) |
There was a problem hiding this comment.
Collapsed comments stay tall
Medium Severity
On Android, collapsing a review comment only swaps the label to “Comment collapsed” while rowHeight still allocates the full multi-line comment height, unlike iOS where collapsed comments use a short row. Scroll offsets, visible-row indexing, and tap targets diverge from iOS and from the collapsed state users expect.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 276b453. Configure here.
ApprovabilityVerdict: Needs human review 21 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
juliusmarminge
force-pushed
the
t3code/ipad-responsive-mobile-layout
branch
from
770c781 to
9f54b06
Compare
juliusmarminge
force-pushed
the
android-dev-pr-3514
branch
from
d08c09c to
1053338
Compare
juliusmarminge
force-pushed
the
android-dev-pr-3514
branch
from
1053338 to
2ae9365
Compare
| } | ||
| } catch (_: Exception) { | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale token patches after reset
High Severity
The setTokensPatchJson function asynchronously applies token patches. It only checks tokensResetKey for relevance, missing a crucial check against contentResetKey. This can cause token patches from a previous diff to be applied to the current view, resulting in incorrect highlighting.
Reviewed by Cursor Bugbot for commit 2ae9365. Configure here.
| token.fontStyle and 1 != 0 -> Typeface.create(Typeface.MONOSPACE, Typeface.ITALIC) | ||
| token.fontStyle and 2 != 0 -> Typeface.create(Typeface.MONOSPACE, Typeface.BOLD) | ||
| else -> Typeface.MONOSPACE | ||
| } |
There was a problem hiding this comment.
Bold italic tokens render wrong
Low Severity
The when statement for textPaint.typeface in drawLineRow applies fontStyle bits sequentially, causing tokens with both italic and bold flags to render only as italic. This results in bold styling being lost and diverges from the intended combined styling.
Reviewed by Cursor Bugbot for commit 2ae9365. Configure here.
| if (imageUris.isNotEmpty()) { | ||
| pasteImagesListener?.invoke(imageUris) | ||
| return true | ||
| } |
There was a problem hiding this comment.
Paste skips text when images present
Medium Severity
On paste, if any clipboard item exposes an image URI, Android invokes onComposerPasteImages and returns true without calling super.onTextContextMenuItem. Plain-text paste from the same or another clip item never runs, so mixed or text-first paste silently fails.
Reviewed by Cursor Bugbot for commit 2ae9365. Configure here.
| "onToggleViewedFile", | ||
| "onPressLine", | ||
| "onToggleComment", | ||
| ) |
There was a problem hiding this comment.
Review diff pull refresh missing
Medium Severity
The Android T3ReviewDiffSurface module does not define the refreshing prop or onPullToRefresh event that exist on iOS. SourceFileSurface passes both when refresh is enabled, so pull-to-refresh on the native source file view does nothing on Android.
Reviewed by Cursor Bugbot for commit 2ae9365. Configure here.
The react-native-screens patch adds iOS-only glass chrome props (subtitle, largeSubtitle, navigationItemStyle, headerCenterBarButtonItems, headerToolbarItems) to the native spec. Android codegen generates abstract setters for them, so ScreenStackHeaderConfigViewManager stopped compiling. No-op overrides keep the Android build working. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| private fun recreateTerminal() { | ||
| if (terminalHandle == 0L) return | ||
| destroyTerminal() | ||
| createTerminal() | ||
| feedPendingBuffer() | ||
| renderSnapshot() | ||
| } |
There was a problem hiding this comment.
🟡 Medium t3terminal/T3TerminalView.kt:274
recreateTerminal() calls feedPendingBuffer() immediately after recreating the native terminal, so when terminalKey changes it replays whatever initialBuffer currently holds. Since terminalKey is applied before initialBuffer in the module, the native terminal for the new session is created and fed the previous session's buffer, causing the old transcript to be rendered and its PTY responses emitted via onInput before the new initialBuffer arrives. Avoid feeding the pending buffer here so the new session starts empty until the correct initialBuffer is set.
private fun recreateTerminal() {
if (terminalHandle == 0L) return
destroyTerminal()
createTerminal()
- feedPendingBuffer()
renderSnapshot()
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt around lines 274-280:
`recreateTerminal()` calls `feedPendingBuffer()` immediately after recreating the native terminal, so when `terminalKey` changes it replays whatever `initialBuffer` currently holds. Since `terminalKey` is applied before `initialBuffer` in the module, the native terminal for the new session is created and fed the *previous* session's buffer, causing the old transcript to be rendered and its PTY responses emitted via `onInput` before the new `initialBuffer` arrives. Avoid feeding the pending buffer here so the new session starts empty until the correct `initialBuffer` is set.
| } | ||
| } | ||
| } | ||
| if (imageUris.isNotEmpty()) { |
There was a problem hiding this comment.
🟡 Medium t3composereditor/T3ComposerEditorView.kt:447
When the clipboard contains mixed content (an item with both a URI and text), onTextContextMenuItem fires pasteImagesListener and return true, so super.onTextContextMenuItem(id) never runs and the text portion of the paste is silently dropped. Consider delegating to super for the text portion so mixed clipboard contents are not partially lost, or document why only image URIs should be handled here.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt around line 447:
When the clipboard contains mixed content (an item with both a URI and text), `onTextContextMenuItem` fires `pasteImagesListener` and `return true`, so `super.onTextContextMenuItem(id)` never runs and the text portion of the paste is silently dropped. Consider delegating to `super` for the text portion so mixed clipboard contents are not partially lost, or document why only image URIs should be handled here.
| const { layout, panes, togglePrimarySidebar } = useAdaptiveWorkspaceLayout(); | ||
|
|
||
| if (!layout.usesSplitView) { | ||
| if (Platform.OS === "android" || !layout.usesSplitView) { |
There was a problem hiding this comment.
🟡 Medium layout/workspace-sidebar-toolbar.tsx:15
WorkspaceSidebarToolbar returns null on Android regardless of layout.usesSplitView, so split-view Android routes lose the sidebar toggle and New task/Return to chat buttons. The Android exclusion should be removed unless Android genuinely lacks split-view support.
| if (Platform.OS === "android" || !layout.usesSplitView) { | |
| if (!layout.usesSplitView) { |
Also found in 1 other location(s)
apps/mobile/src/features/review/ReviewSheet.tsx:663
On Android the new
!isAndroidguard atReviewSheetline 663 removes the entire right-side toolbar, which is the only placeThreadGitMenuis rendered. The replacementAndroidScreenHeaderonly shows the section selector, so opening review on Android no longer exposes the commit/push/"More" git actions that this screen previously provided. Users must leave review to perform those actions, which is a regression in core review functionality.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/layout/workspace-sidebar-toolbar.tsx around line 15:
`WorkspaceSidebarToolbar` returns `null` on Android regardless of `layout.usesSplitView`, so split-view Android routes lose the sidebar toggle and `New task`/`Return to chat` buttons. The Android exclusion should be removed unless Android genuinely lacks split-view support.
Also found in 1 other location(s):
- apps/mobile/src/features/review/ReviewSheet.tsx:663 -- On Android the new `!isAndroid` guard at `ReviewSheet` line 663 removes the entire right-side toolbar, which is the only place `ThreadGitMenu` is rendered. The replacement `AndroidScreenHeader` only shows the section selector, so opening review on Android no longer exposes the commit/push/"More" git actions that this screen previously provided. Users must leave review to perform those actions, which is a regression in core review functionality.
| ghostty_render_state_row_cells_get(session->row_cells, | ||
| GHOSTTY_RENDER_STATE_ROW_CELLS_DATA_BG_COLOR, | ||
| &background); | ||
| if (style.inverse) std::swap(foreground, background); |
There was a problem hiding this comment.
🟡 Medium cpp/t3_terminal_jni.cpp:369
nativeSnapshot serializes GHOSTTY_RENDER_STATE_ROW_CELLS_DATA_FG_COLOR directly into the snapshot without applying bold-color handling. The libghostty-vt header documents that this value does not include bold-color mapping and that the caller must handle bold styling separately. As a result, ANSI bright/bold text is drawn with the non-bright palette entry instead of the intended bold color, because the Android renderer only uses the kBold flag to switch typefaces. Consider applying the bold/bright color mapping in the snapshot path when style.bold is set, or document why bold-color handling is intentionally deferred to the renderer.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/cpp/t3_terminal_jni.cpp around line 369:
`nativeSnapshot` serializes `GHOSTTY_RENDER_STATE_ROW_CELLS_DATA_FG_COLOR` directly into the snapshot without applying bold-color handling. The `libghostty-vt` header documents that this value does not include bold-color mapping and that the caller must handle bold styling separately. As a result, ANSI bright/bold text is drawn with the non-bright palette entry instead of the intended bold color, because the Android renderer only uses the `kBold` flag to switch typefaces. Consider applying the bold/bright color mapping in the snapshot path when `style.bold` is set, or document why bold-color handling is intentionally deferred to the renderer.
| override fun onDown(event: MotionEvent): Boolean { | ||
| onRequestKeyboard?.invoke() | ||
| return true | ||
| } |
There was a problem hiding this comment.
🟡 Medium t3terminal/TerminalCanvasView.kt:221
onDown invokes onRequestKeyboard on every touch sequence, so starting a scroll drag opens the soft keyboard even though the user never tapped to type. Consider requesting the keyboard from onSingleTapUp instead of onDown, so only taps trigger it.
override fun onDown(event: MotionEvent): Boolean {
- onRequestKeyboard?.invoke()
return true
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/TerminalCanvasView.kt around lines 221-224:
`onDown` invokes `onRequestKeyboard` on every touch sequence, so starting a scroll drag opens the soft keyboard even though the user never tapped to type. Consider requesting the keyboard from `onSingleTapUp` instead of `onDown`, so only taps trigger it.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 11 total unresolved issues (including 8 from previous reviews).
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Shared horizontal scroll across files
- Replaced the single horizontalOffset field with a per-file HashMap<String, Int> so each file maintains its own horizontal scroll position, matching iOS's horizontalOffsetsByFileId behavior.
- ✅ Fixed: Paste sends unreadable content URIs
- Added copyToLocalFile() that copies content:// clipboard images into app cache (t3-composer-paste/*.png) and emits file:// URIs that convertPastedImagesToAttachments can read, matching the iOS writeTemporaryImage pattern.
- ✅ Fixed: Word diff highlights missing Android
- Added wordDiffRanges field to DiffRow, parsing from row JSON, and implemented drawWordDiffRanges() to render intra-line add/delete highlights matching iOS's behavior.
Or push these changes by commenting:
@cursor push 5664a9a0e6
Preview (5664a9a0e6)
diff --git a/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt b/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
--- a/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
+++ b/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
@@ -7,6 +7,7 @@
import android.graphics.Paint
import android.graphics.RectF
import android.graphics.Typeface
+import android.net.Uri
import android.text.Editable
import android.text.InputType
import android.text.Spanned
@@ -20,6 +21,8 @@
import expo.modules.kotlin.viewevent.EventDispatcher
import expo.modules.kotlin.views.ExpoView
import org.json.JSONObject
+import java.io.File
+import java.util.UUID
import kotlin.math.max
class T3ComposerEditorView(context: Context, appContext: AppContext) : ExpoView(context, appContext) {
@@ -434,21 +437,39 @@
if (id == android.R.id.paste || id == android.R.id.pasteAsPlainText) {
val clipboard = context.getSystemService(Context.CLIPBOARD_SERVICE) as? ClipboardManager
val clip = clipboard?.primaryClip
- val imageUris = buildList {
+ val fileUris = buildList {
if (clip != null) {
for (index in 0 until clip.itemCount) {
clip.getItemAt(index).uri?.let { uri ->
val mimeType = context.contentResolver.getType(uri)
- if (mimeType?.startsWith("image/") == true) add(uri.toString())
+ if (mimeType?.startsWith("image/") == true) {
+ copyToLocalFile(uri)?.let { add(it) }
+ }
}
}
}
}
- if (imageUris.isNotEmpty()) {
- pasteImagesListener?.invoke(imageUris)
+ if (fileUris.isNotEmpty()) {
+ pasteImagesListener?.invoke(fileUris)
return true
}
}
return super.onTextContextMenuItem(id)
}
+
+ private fun copyToLocalFile(contentUri: Uri): String? {
+ return try {
+ val pasteDir = File(context.cacheDir, "t3-composer-paste")
+ pasteDir.mkdirs()
+ val destFile = File(pasteDir, "${UUID.randomUUID()}.png")
+ context.contentResolver.openInputStream(contentUri)?.use { input ->
+ destFile.outputStream().use { output ->
+ input.copyTo(output)
+ }
+ }
+ Uri.fromFile(destFile).toString()
+ } catch (_: Exception) {
+ null
+ }
+ }
}
diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
--- a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
+++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
@@ -86,7 +86,7 @@
lastVisibleFileId = null
pendingInitialScroll = true
canvasView.setVerticalOffset(0)
- canvasView.setHorizontalOffset(0)
+ canvasView.resetHorizontalOffsets()
applyPendingInitialScroll()
}
@@ -416,6 +416,7 @@
val change: String,
val oldLineNumber: Int?,
val newLineNumber: Int?,
+ val wordDiffRanges: List<DiffWordDiffRange>?,
val commentText: String,
val commentRangeLabel: String,
val commentSectionTitle: String,
@@ -423,6 +424,11 @@
val resolvedFileId: String get() = fileId.ifEmpty { id }
}
+private data class DiffWordDiffRange(
+ val start: Int,
+ val end: Int,
+)
+
private data class DiffToken(
val content: String,
val color: Int?,
@@ -567,12 +573,13 @@
)
private var rowOffsets = intArrayOf(0)
private var verticalOffset = 0
- private var horizontalOffset = 0
+ private var horizontalOffsetsByFileId = HashMap<String, Int>()
private var lastVisibleRange: Pair<Int, Int>? = null
var rows: List<DiffRow> = emptyList()
set(value) {
field = value
+ horizontalOffsetsByFileId.clear()
rebuildOffsets()
}
var tokensByRowId: Map<String, List<DiffToken>> = emptyMap()
@@ -603,7 +610,7 @@
var contentWidthPx: Int = (1200 * density).toInt()
set(value) {
field = max(value, suggestedMinimumWidth)
- setHorizontalOffset(horizontalOffset)
+ clampHorizontalOffsets()
invalidate()
}
var onRowTap: ((DiffRow, String) -> Unit)? = null
@@ -619,7 +626,7 @@
override fun onSizeChanged(width: Int, height: Int, oldWidth: Int, oldHeight: Int) {
super.onSizeChanged(width, height, oldWidth, oldHeight)
setVerticalOffset(verticalOffset)
- setHorizontalOffset(horizontalOffset)
+ clampHorizontalOffsets()
}
override fun onDraw(canvas: Canvas) {
@@ -665,20 +672,58 @@
fun maxVerticalOffset(): Int = max(0, (rowOffsets.lastOrNull() ?: 0) - height)
fun setHorizontalOffset(value: Int) {
+ val fileId = fileIdAtVerticalCenter() ?: return
val nextOffset = value.coerceIn(0, maxHorizontalOffset())
- if (horizontalOffset == nextOffset) return
- horizontalOffset = nextOffset
+ val current = horizontalOffsetsByFileId[fileId] ?: 0
+ if (current == nextOffset) return
+ horizontalOffsetsByFileId[fileId] = nextOffset
invalidate()
}
fun scrollByHorizontal(delta: Int) {
- setHorizontalOffset(horizontalOffset + delta)
+ val fileId = fileIdAtVerticalCenter() ?: return
+ val current = horizontalOffsetsByFileId[fileId] ?: 0
+ setHorizontalOffsetForFile(fileId, current + delta)
}
- fun horizontalOffset(): Int = horizontalOffset
+ fun horizontalOffset(): Int {
+ val fileId = fileIdAtVerticalCenter() ?: return 0
+ return horizontalOffsetsByFileId[fileId] ?: 0
+ }
fun maxHorizontalOffset(): Int = max(0, contentWidthPx - width)
+ fun resetHorizontalOffsets() {
+ horizontalOffsetsByFileId.clear()
+ }
+
+ private fun setHorizontalOffsetForFile(fileId: String, value: Int) {
+ val nextOffset = value.coerceIn(0, maxHorizontalOffset())
+ val current = horizontalOffsetsByFileId[fileId] ?: 0
+ if (current == nextOffset) return
+ horizontalOffsetsByFileId[fileId] = nextOffset
+ invalidate()
+ }
+
+ private fun clampHorizontalOffsets() {
+ val maxOffset = maxHorizontalOffset()
+ val iterator = horizontalOffsetsByFileId.entries.iterator()
+ while (iterator.hasNext()) {
+ val entry = iterator.next()
+ entry.setValue(entry.value.coerceIn(0, maxOffset))
+ }
+ }
+
+ private fun fileIdAtVerticalCenter(): String? {
+ if (rows.isEmpty()) return null
+ val centerY = verticalOffset + height / 2
+ val index = rowIndexAt(centerY).coerceIn(0, rows.lastIndex)
+ return (index downTo 0)
+ .asSequence()
+ .map { rows[it].resolvedFileId }
+ .firstOrNull { it.isNotEmpty() }
+ }
+
private fun rebuildOffsets() {
rowOffsets = IntArray(rows.size + 1)
rows.forEachIndexed { index, row ->
@@ -760,7 +805,7 @@
fill(canvas, theme.hunkBackground, 0f, top.toFloat(), width.toFloat(), bottom.toFloat())
textPaint.color = theme.hunkText
textPaint.textSize = style.codeFontSizePx
- drawScrollableCode(canvas, top, bottom) { codeX ->
+ drawScrollableCode(canvas, top, bottom, row.resolvedFileId) { codeX ->
canvas.drawText(
row.text.ifEmpty { row.content },
codeX,
@@ -773,7 +818,7 @@
private fun drawNoticeRow(canvas: Canvas, row: DiffRow, top: Int, bottom: Int) {
textPaint.color = theme.mutedText
textPaint.textSize = style.codeFontSizePx
- drawScrollableCode(canvas, top, bottom) { codeX ->
+ drawScrollableCode(canvas, top, bottom, row.resolvedFileId) { codeX ->
canvas.drawText(row.text, codeX, centeredBaseline(top, bottom, textPaint), textPaint)
}
}
@@ -782,7 +827,7 @@
fill(canvas, theme.headerBackground, style.gutterWidthPx, top.toFloat(), width.toFloat(), bottom.toFloat())
boldTextPaint.color = theme.text
boldTextPaint.textSize = 12f * density
- drawScrollableCode(canvas, top, bottom) { codeX ->
+ drawScrollableCode(canvas, top, bottom, row.resolvedFileId) { codeX ->
canvas.drawText(
row.commentSectionTitle.ifEmpty { row.commentRangeLabel.ifEmpty { "Comment" } },
codeX,
@@ -824,7 +869,8 @@
}
val tokens = tokensByRowId[row.id]
- drawScrollableCode(canvas, top, bottom) { codeX ->
+ drawScrollableCode(canvas, top, bottom, row.resolvedFileId) { codeX ->
+ drawWordDiffRanges(canvas, row, codeX, top, bottom)
if (tokens.isNullOrEmpty()) {
textPaint.textSize = style.codeFontSizePx
textPaint.color = when (row.change) {
@@ -859,16 +905,40 @@
canvas.drawText(newNumber, style.changeBarWidthPx + style.gutterWidthPx / 2f, baseline, textPaint)
}
+ private fun drawWordDiffRanges(canvas: Canvas, row: DiffRow, codeX: Float, top: Int, bottom: Int) {
+ val ranges = row.wordDiffRanges
+ if (ranges.isNullOrEmpty()) return
+ val change = row.change
+ if (change != "add" && change != "delete") return
+
+ val fillColor = if (change == "add") withAlpha(theme.addBar, 71) else withAlpha(theme.deleteBar, 71)
+ textPaint.textSize = style.codeFontSizePx
+ val charWidth = textPaint.measureText("m")
+ val highlightHeight = max(4f * density, min((bottom - top).toFloat() - 4f * density, textPaint.fontMetrics.let { -it.ascent + it.descent }))
+ val highlightY = (top + bottom) / 2f - highlightHeight / 2f
+ val cornerRadius = 3f * density
+
+ for (range in ranges) {
+ if (range.end <= range.start) continue
+ val startX = codeX + range.start * charWidth
+ val rangeWidth = max(2f * density, (range.end - range.start) * charWidth)
+ backgroundPaint.color = fillColor
+ canvas.drawRoundRect(startX, highlightY, startX + rangeWidth, highlightY + highlightHeight, cornerRadius, cornerRadius, backgroundPaint)
+ }
+ }
+
private fun drawScrollableCode(
canvas: Canvas,
top: Int,
bottom: Int,
+ fileId: String,
draw: (Float) -> Unit,
) {
val gutterEnd = style.changeBarWidthPx + style.gutterWidthPx
+ val offset = horizontalOffsetsByFileId[fileId] ?: 0
canvas.save()
canvas.clipRect(gutterEnd, top.toFloat(), width.toFloat(), bottom.toFloat())
- draw(gutterEnd + style.codePaddingPx - horizontalOffset)
+ draw(gutterEnd + style.codePaddingPx - offset)
canvas.restore()
}
@@ -895,10 +965,12 @@
private fun drawHorizontalScrollIndicator(canvas: Canvas) {
val maxOffset = maxHorizontalOffset()
if (maxOffset <= 0 || width <= 0) return
+ val currentOffset = horizontalOffset()
+ if (currentOffset <= 0) return
val trackWidth = width.toFloat()
val thumbWidth = max(24f * density, trackWidth * trackWidth / contentWidthPx)
val thumbTravel = trackWidth - thumbWidth
- val left = thumbTravel * horizontalOffset / maxOffset
+ val left = thumbTravel * currentOffset / maxOffset
fill(
canvas,
withAlpha(theme.mutedText, 110),
@@ -962,6 +1034,15 @@
change = row.optString("change", "context"),
oldLineNumber = row.optNullableInt("oldLineNumber"),
newLineNumber = row.optNullableInt("newLineNumber"),
+ wordDiffRanges = row.optJSONArray("wordDiffRanges")?.let { rangesArray ->
+ List(rangesArray.length()) { rangeIndex ->
+ val rangeObj = rangesArray.getJSONObject(rangeIndex)
+ DiffWordDiffRange(
+ start = rangeObj.optInt("start"),
+ end = rangeObj.optInt("end"),
+ )
+ }
+ },
commentText = row.optString("commentText"),
commentRangeLabel = row.optString("commentRangeLabel"),
commentSectionTitle = row.optString("commentSectionTitle"),You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 2b34778. Configure here.
|
|
||
| fun horizontalOffset(): Int = horizontalOffset | ||
|
|
||
| fun maxHorizontalOffset(): Int = max(0, contentWidthPx - width) |
There was a problem hiding this comment.
Shared horizontal scroll across files
Medium Severity
The T3ReviewDiffView on Android uses a single horizontal scroll offset for the entire diff. This differs from iOS's per-file tracking and can cause files to display with incorrect horizontal alignment if another file was previously scrolled.
Reviewed by Cursor Bugbot for commit 2b34778. Configure here.
| } | ||
| } catch (_: Exception) { | ||
| emptyList() | ||
| } |
There was a problem hiding this comment.
Word diff highlights missing Android
Medium Severity
The Android diff parser and renderer never read or draw wordDiffRanges from row JSON, though the shared adapter still sends them. Intra-line add/delete highlights that iOS paints via drawWordDiffRanges are absent on Android whenever those ranges are used instead of syntax tokens alone.
Reviewed by Cursor Bugbot for commit 2b34778. Configure here.
| } | ||
| if (imageUris.isNotEmpty()) { | ||
| pasteImagesListener?.invoke(imageUris) | ||
| return true |
There was a problem hiding this comment.
Paste sends unreadable content URIs
Medium Severity
The Android composer emits clipboard content:// image URIs from paste, while convertPastedImagesToAttachments reads paths with expo-file-system File(uri) after iOS copies images into owned file:// t3-composer-paste paths. Pasted images in the Android composer can fail to attach even when the native paste event fires.
Reviewed by Cursor Bugbot for commit 2b34778. Configure here.
|
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
Or push these changes by commenting: Preview (969f425c0b)diff --git a/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt b/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
--- a/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
+++ b/apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt
@@ -437,7 +437,9 @@
val imageUris = buildList {
if (clip != null) {
for (index in 0 until clip.itemCount) {
- clip.getItemAt(index).uri?.let { uri ->
+ val item = clip.getItemAt(index)
+ if (!item.text.isNullOrEmpty()) continue
+ item.uri?.let { uri ->
val mimeType = context.contentResolver.getType(uri)
if (mimeType?.startsWith("image/") == true) add(uri.toString())
}
diff --git a/apps/mobile/modules/t3-review-diff/android/build.gradle b/apps/mobile/modules/t3-review-diff/android/build.gradle
--- a/apps/mobile/modules/t3-review-diff/android/build.gradle
+++ b/apps/mobile/modules/t3-review-diff/android/build.gradle
@@ -16,4 +16,5 @@
dependencies {
implementation project(':expo-modules-core')
+ implementation 'androidx.swiperefreshlayout:swiperefreshlayout:1.1.0'
}
diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffModule.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffModule.kt
--- a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffModule.kt
+++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffModule.kt
@@ -44,6 +44,9 @@
Prop("initialRowIndex") { view: T3ReviewDiffView, initialRowIndex: Double ->
view.setInitialRowIndex(initialRowIndex)
}
+ Prop("refreshing") { view: T3ReviewDiffView, refreshing: Boolean ->
+ view.setRefreshing(refreshing)
+ }
Events(
"onDebug",
@@ -52,6 +55,7 @@
"onToggleViewedFile",
"onPressLine",
"onToggleComment",
+ "onPullToRefresh",
)
AsyncFunction("scrollToFile") { view: T3ReviewDiffView, fileId: String, animated: Boolean ->
diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
--- a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
+++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt
@@ -12,6 +12,7 @@
import android.view.ViewGroup
import android.view.ViewConfiguration
import android.widget.OverScroller
+import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
import expo.modules.kotlin.AppContext
import expo.modules.kotlin.viewevent.EventDispatcher
import expo.modules.kotlin.views.ExpoView
@@ -24,12 +25,16 @@
class T3ReviewDiffView(context: Context, appContext: AppContext) : ExpoView(context, appContext) {
private val canvasView = DiffCanvasView(context)
+ private val swipeRefreshLayout = object : SwipeRefreshLayout(context) {
+ override fun canChildScrollUp(): Boolean = canvasView.verticalOffset() > 0
+ }
private val onDebug by EventDispatcher()
private val onVisibleFileChange by EventDispatcher()
private val onToggleFile by EventDispatcher()
private val onToggleViewedFile by EventDispatcher()
private val onPressLine by EventDispatcher()
private val onToggleComment by EventDispatcher()
+ private val onPullToRefresh by EventDispatcher()
private var rows: List<DiffRow> = emptyList()
private var visibleRows: List<DiffRow> = emptyList()
private var collapsedFileIds: Set<String> = emptySet()
@@ -66,12 +71,21 @@
emitVisibleFile(first)
}
- addView(
+ swipeRefreshLayout.addView(
canvasView,
LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT),
)
+ swipeRefreshLayout.setOnRefreshListener { onPullToRefresh(emptyMap()) }
+ addView(
+ swipeRefreshLayout,
+ LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT),
+ )
}
+ fun setRefreshing(value: Boolean) {
+ swipeRefreshLayout.isRefreshing = value
+ }
+
fun setTokensResetKey(value: String) {
if (tokensResetKey == value) return
tokensResetKey = value
@@ -163,6 +177,7 @@
}
fun setTokensPatchJson(value: String) {
+ val capturedContentResetKey = contentResetKey
payloadDecodeExecutor.execute {
try {
val payload = JSONObject(value)
@@ -171,6 +186,7 @@
payload.optJSONObject("tokensByRowId") ?: JSONObject(),
)
post {
+ if (capturedContentResetKey != contentResetKey) return@post
if (resetKey.isNotEmpty() && resetKey != tokensResetKey) return@post
if (decodedTokens.isNotEmpty()) {
canvasView.tokensByRowId = canvasView.tokensByRowId + decodedTokens
@@ -211,7 +227,14 @@
val deltaX = event.x - lastTouchX
val deltaY = event.y - lastTouchY
if (max(abs(deltaX), abs(deltaY)) > touchSlop) {
- dragAxis = if (abs(deltaY) >= abs(deltaX)) DragAxis.VERTICAL else DragAxis.HORIZONTAL
+ if (abs(deltaY) >= abs(deltaX)) {
+ if (deltaY > 0 && canvasView.verticalOffset() == 0) {
+ return false
+ }
+ dragAxis = DragAxis.VERTICAL
+ } else {
+ dragAxis = DragAxis.HORIZONTAL
+ }
}
}
return dragAxis != null
@@ -839,6 +862,7 @@
textPaint.textSize = style.codeFontSizePx
textPaint.color = token.color ?: theme.text
textPaint.typeface = when {
+ token.fontStyle and 3 == 3 -> Typeface.create(Typeface.MONOSPACE, Typeface.BOLD_ITALIC)
token.fontStyle and 1 != 0 -> Typeface.create(Typeface.MONOSPACE, Typeface.ITALIC)
token.fontStyle and 2 != 0 -> Typeface.create(Typeface.MONOSPACE, Typeface.BOLD)
else -> Typeface.MONOSPACEYou can send follow-ups to the cloud agent here. |
There was a problem hiding this comment.
🟡 Medium archive/ArchivedThreadsScreen.tsx:307
ArchivedThreadRow wraps rows in ThreadSwipeable but never passes resetKey, so when LegendList recycles a cell that was left open or mid-drag onto a different thread, the swipe state leaks and the new thread appears already swiped with the wrong actions exposed. Pass a unique key like `${thread.environmentId}:${thread.id}` to resetKey so ThreadSwipeable snaps back to closed on reuse.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/archive/ArchivedThreadsScreen.tsx around line 307:
`ArchivedThreadRow` wraps rows in `ThreadSwipeable` but never passes `resetKey`, so when `LegendList` recycles a cell that was left open or mid-drag onto a different thread, the swipe state leaks and the new thread appears already swiped with the wrong actions exposed. Pass a unique key like `` `${thread.environmentId}:${thread.id}` `` to `resetKey` so `ThreadSwipeable` snaps back to closed on reuse.
There was a problem hiding this comment.
🟡 Medium files/ThreadFilesRouteScreen.tsx:524
handleSelectFile in ThreadFileScreen always calls navigation.navigate("ThreadFile", ...), pushing a new route onto the stack for every file selected from the inspector. ThreadFilesTreeScreen uses resolveFileSelectionNavigationAction() to decide between replace and navigate based on fileInspector.supported, but ThreadFileScreen omits this check. On wide layouts with a persistent file inspector, each selection grows the back stack, so Back walks through every previously opened file instead of returning to chat in one step. Consider applying the same resolveFileSelectionNavigationAction logic here.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx around line 524:
`handleSelectFile` in `ThreadFileScreen` always calls `navigation.navigate("ThreadFile", ...)`, pushing a new route onto the stack for every file selected from the inspector. `ThreadFilesTreeScreen` uses `resolveFileSelectionNavigationAction()` to decide between `replace` and `navigate` based on `fileInspector.supported`, but `ThreadFileScreen` omits this check. On wide layouts with a persistent file inspector, each selection grows the back stack, so Back walks through every previously opened file instead of returning to chat in one step. Consider applying the same `resolveFileSelectionNavigationAction` logic here.
There was a problem hiding this comment.
🟡 Medium home/HomeHeader.tsx:317
IosHomeHeader calls useHardwareKeyboardCommand("focusSearch", focusSearch), but on the iOS path headerSearchBarOptions is explicitly undefined, so no component is ever attached to searchBarRef. As a result, searchBarRef.current stays null, focusSearch() is a no-op, and pressing the hardware focusSearch shortcut does nothing on iOS — the Home search field never receives focus. Consider wiring the search field ref to the native iOS toolbar item (or only registering the command on the platform where the ref is actually connected).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/home/HomeHeader.tsx around line 317:
`IosHomeHeader` calls `useHardwareKeyboardCommand("focusSearch", focusSearch)`, but on the iOS path `headerSearchBarOptions` is explicitly `undefined`, so no component is ever attached to `searchBarRef`. As a result, `searchBarRef.current` stays `null`, `focusSearch()` is a no-op, and pressing the hardware `focusSearch` shortcut does nothing on iOS — the Home search field never receives focus. Consider wiring the search field ref to the native iOS toolbar item (or only registering the command on the platform where the ref is actually connected).
There was a problem hiding this comment.
🟠 High home/HomeScreen.tsx:347
On iOS, when !hasAnyThreads is true, HomeScreen returns the empty-state branch (lines 325–350) without rendering any WorkspaceConnectionStatus. The connectionStatus variable is set only for Platform.OS !== "ios", and the iOS status row is only included in listHeader, which is never used in the early-return path. As a result, iOS users in the offline/connecting/error empty state lose the status indicator and its onOpenEnvironments action. Consider rendering an inline WorkspaceConnectionStatus (the sidebar variant) in the empty-state branch when shouldShowConnectionStatus is true on iOS, mirroring the threaded-list path.
{connectionStatus}
+ {shouldShowConnectionStatus && Platform.OS === "ios" ? (
+ <View style={{ paddingBottom: 16 }}>
+ <WorkspaceConnectionStatus
+ state={props.catalogState}
+ onPress={props.onOpenEnvironments}
+ variant="sidebar"
+ />
+ </View>
+ ) : null}
</View>🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/home/HomeScreen.tsx around lines 347-349:
On iOS, when `!hasAnyThreads` is true, `HomeScreen` returns the empty-state branch (lines 325–350) without rendering any `WorkspaceConnectionStatus`. The `connectionStatus` variable is set only for `Platform.OS !== "ios"`, and the iOS status row is only included in `listHeader`, which is never used in the early-return path. As a result, iOS users in the offline/connecting/error empty state lose the status indicator and its `onOpenEnvironments` action. Consider rendering an inline `WorkspaceConnectionStatus` (the `sidebar` variant) in the empty-state branch when `shouldShowConnectionStatus` is true on iOS, mirroring the threaded-list path.
There was a problem hiding this comment.
🟡 Medium threads/ThreadComposer.tsx:208
ComposerConnectionStatusPill renders as a pressable that calls props.onPress for every status kind, including the new "syncing" state. In ThreadComposer, onPress is wired to onReconnectEnvironment, so tapping the pill during normal message loading/syncing triggers retryEnvironment and interrupts the in-progress sync instead of acting as a passive progress indicator. Consider rendering the pill as a non-interactive View (or disabling the Pressable) when status.kind === "syncing".
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadComposer.tsx around line 208:
`ComposerConnectionStatusPill` renders as a pressable that calls `props.onPress` for every status kind, including the new `"syncing"` state. In `ThreadComposer`, `onPress` is wired to `onReconnectEnvironment`, so tapping the pill during normal message loading/syncing triggers `retryEnvironment` and interrupts the in-progress sync instead of acting as a passive progress indicator. Consider rendering the pill as a non-interactive `View` (or disabling the `Pressable`) when `status.kind === "syncing"`.
There was a problem hiding this comment.
🟠 High threads/ThreadRouteScreen.tsx:165
ThreadRouteScreen gates ThreadUnavailableScreen only on connection hydration (workspaceState.isLoadingConnections or connecting/reconnecting) but not on the thread-detail query status. When the route's thread hasn't yet appeared in the shell snapshot, useThreadSelection() returns selectedThread === null during its initial load, so the guard selectedThread !== null && selectedThreadKey === routeThreadKey fails and execution falls through to ThreadUnavailableScreen even though the thread exists and is still loading. Include the thread-detail fetch status in stillHydrating (or check it before rendering the unavailable state) so the screen shows OpeningThreadLoadingScreen until the thread detail query resolves.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadRouteScreen.tsx around line 165:
`ThreadRouteScreen` gates `ThreadUnavailableScreen` only on connection hydration (`workspaceState.isLoadingConnections` or `connecting`/`reconnecting`) but not on the thread-detail query status. When the route's thread hasn't yet appeared in the shell snapshot, `useThreadSelection()` returns `selectedThread === null` during its initial load, so the guard `selectedThread !== null && selectedThreadKey === routeThreadKey` fails and execution falls through to `ThreadUnavailableScreen` even though the thread exists and is still loading. Include the thread-detail fetch status in `stillHydrating` (or check it before rendering the unavailable state) so the screen shows `OpeningThreadLoadingScreen` until the thread detail query resolves.
There was a problem hiding this comment.
🟠 High src/Stack.tsx:127
When SettingsEnvironmentNew is opened as the initial route of the nested SettingsSheetStack via navigation.navigate("SettingsSheet", { screen: "SettingsEnvironmentNew" }), ConnectionsNewRouteScreen cannot go back, so it calls navigation.dispatch(StackActions.replace("Home")) after a successful pairing. But Home is not a route in SettingsSheetStack, so React Navigation ignores the unhandled replace action and the screen stays visible — the sheet never dismisses. Consider navigating back up to the parent stack (e.g. via the parent navigator) or including a route the nested stack can actually resolve.
Also found in 1 other location(s)
apps/mobile/src/features/projects/AddProjectScreen.tsx:277
EmptyEnvironmentStatenow dispatchesStackActions.replace("ConnectionsNew")from inside the nestedNewTaskSheetstack.replacetargets the route identified by the dispatching screen'ssourcekey, and this child-stack route key doesn't exist in the parent/root stack whereConnectionsNewis registered. As a result, the action is not handled, so tappingAdd environmentfrom the add-project flow leaves the user stuck instead of opening the connections screen.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/Stack.tsx around line 127:
When `SettingsEnvironmentNew` is opened as the initial route of the nested `SettingsSheetStack` via `navigation.navigate("SettingsSheet", { screen: "SettingsEnvironmentNew" })`, `ConnectionsNewRouteScreen` cannot go back, so it calls `navigation.dispatch(StackActions.replace("Home"))` after a successful pairing. But `Home` is not a route in `SettingsSheetStack`, so React Navigation ignores the unhandled `replace` action and the screen stays visible — the sheet never dismisses. Consider navigating back up to the parent stack (e.g. via the parent navigator) or including a route the nested stack can actually resolve.
Also found in 1 other location(s):
- apps/mobile/src/features/projects/AddProjectScreen.tsx:277 -- `EmptyEnvironmentState` now dispatches `StackActions.replace("ConnectionsNew")` from inside the nested `NewTaskSheet` stack. `replace` targets the route identified by the dispatching screen's `source` key, and this child-stack route key doesn't exist in the parent/root stack where `ConnectionsNew` is registered. As a result, the action is not handled, so tapping `Add environment` from the add-project flow leaves the user stuck instead of opening the connections screen.
| } | ||
| } | ||
|
|
||
| private data class DiffRow( |
There was a problem hiding this comment.
🟡 Medium t3reviewdiff/T3ReviewDiffView.kt:405
DiffRow is missing the wordDiffRanges field that nativeReviewDiffAdapter.ts serializes onto changed line rows. Because parseRows decodes JSON into this type, every Android diff row drops those ranges, so the native renderer has no data to mark inline word-level additions and deletions — word-diff highlighting is lost on Android even though the JS bridge computes and sends it. Consider adding a wordDiffRanges property to DiffRow so the decoded values are preserved and available to the renderer.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt around line 405:
`DiffRow` is missing the `wordDiffRanges` field that `nativeReviewDiffAdapter.ts` serializes onto changed line rows. Because `parseRows` decodes JSON into this type, every Android diff row drops those ranges, so the native renderer has no data to mark inline word-level additions and deletions — word-diff highlighting is lost on Android even though the JS bridge computes and sends it. Consider adding a `wordDiffRanges` property to `DiffRow` so the decoded values are preserved and available to the renderer.
| private fun rebuildOffsets() { | ||
| rowOffsets = IntArray(rows.size + 1) | ||
| rows.forEachIndexed { index, row -> |
There was a problem hiding this comment.
🟡 Medium t3reviewdiff/T3ReviewDiffView.kt:682
rebuildOffsets() recalculates rowOffsets but leaves lastVisibleRange unchanged, so emitVisibleRange() suppresses the onVisibleRowsChanged callback when the new rows produce the same numeric (first, last) range as the old ones. After replacing rows with a different diff at the same scroll position, the parent never receives a new visible-rows notification and the active file indicator stays stale until the user scrolls. Reset lastVisibleRange to null in rebuildOffsets() so the next draw emits the range for the new content.
private fun rebuildOffsets() {
+ lastVisibleRange = null
rowOffsets = IntArray(rows.size + 1)🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt around lines 682-684:
`rebuildOffsets()` recalculates `rowOffsets` but leaves `lastVisibleRange` unchanged, so `emitVisibleRange()` suppresses the `onVisibleRowsChanged` callback when the new rows produce the same numeric `(first, last)` range as the old ones. After replacing `rows` with a different diff at the same scroll position, the parent never receives a new visible-rows notification and the active file indicator stays stale until the user scrolls. Reset `lastVisibleRange` to `null` in `rebuildOffsets()` so the next draw emits the range for the new content.
| fun setEditable(editable: Boolean) { | ||
| editor.isEnabled = editable | ||
| editor.isFocusable = editable | ||
| editor.isFocusableInTouchMode = editable | ||
| editor.isCursorVisible = editable | ||
| } |
There was a problem hiding this comment.
🟡 Medium t3composereditor/T3ComposerEditorView.kt:191
setEditable(false) calls editor.isEnabled = editable, which fully disables the EditText — it can no longer receive focus, scroll, allow text selection, or copy actions. This makes any read-only composer inert on Android, unlike the iOS implementation which only disables editing while keeping the field viewable and scrollable. Consider using editor.isFocusable = editable / editor.isFocusableInTouchMode = editable combined with editor.isCursorVisible = editable and suppressing input via setKeyListener(null) instead of isEnabled, so scrolling and selection remain functional when editing is off.
fun setEditable(editable: Boolean) {
- editor.isEnabled = editable
- editor.isFocusable = editable
- editor.isFocusableInTouchMode = editable
- editor.isCursorVisible = editable
+ editor.isFocusable = editable
+ editor.isFocusableInTouchMode = editable
+ editor.isCursorVisible = editable
+ editor.setKeyListener(if (editable) editor.keyListener else null)
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt around lines 191-196:
`setEditable(false)` calls `editor.isEnabled = editable`, which fully disables the `EditText` — it can no longer receive focus, scroll, allow text selection, or copy actions. This makes any read-only composer inert on Android, unlike the iOS implementation which only disables editing while keeping the field viewable and scrollable. Consider using `editor.isFocusable = editable` / `editor.isFocusableInTouchMode = editable` combined with `editor.isCursorVisible = editable` and suppressing input via `setKeyListener(null)` instead of `isEnabled`, so scrolling and selection remain functional when editing is off.
|
|
||
| fun horizontalOffset(): Int = horizontalOffset | ||
|
|
||
| fun maxHorizontalOffset(): Int = max(0, contentWidthPx - width) |
There was a problem hiding this comment.
🟡 Medium t3reviewdiff/T3ReviewDiffView.kt:680
maxHorizontalOffset() computes the scroll range as contentWidthPx - width, but drawScrollableCode() clips drawing to the region right of the gutter (starting at style.changeBarWidthPx + style.gutterWidthPx). This means the actual scrollable code area is only width - (style.changeBarWidthPx + style.gutterWidthPx) wide, so horizontal scrolling stops short by the gutter width — the rightmost portion of long lines can never be scrolled into view. Consider subtracting the gutter width from the available viewport width in this calculation.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt around line 680:
`maxHorizontalOffset()` computes the scroll range as `contentWidthPx - width`, but `drawScrollableCode()` clips drawing to the region right of the gutter (starting at `style.changeBarWidthPx + style.gutterWidthPx`). This means the actual scrollable code area is only `width - (style.changeBarWidthPx + style.gutterWidthPx)` wide, so horizontal scrolling stops short by the gutter width — the rightmost portion of long lines can never be scrolled into view. Consider subtracting the gutter width from the available viewport width in this calculation.
| private fun JSONObject.optNullableInt(key: String): Int? = | ||
| if (isNull(key) || !has(key)) null else optInt(key) | ||
|
|
||
| private fun JSONObject.floatDp(key: String, fallbackPx: Float, density: Float): Float = |
There was a problem hiding this comment.
🟡 Medium t3reviewdiff/T3ReviewDiffView.kt:1019
json.floatDp calls JSONObject.optDouble() for any present key, and optDouble returns 0.0 for null or non-numeric values. So { "rowHeight": null } or { "rowHeight": "bad" } yields 0px instead of fallbackPx, collapsing the diff UI instead of preserving the default style. The same bug exists in floatSp. Consider checking optDouble(key, Double.NaN) (or guarding isNull/type) and falling back when the value is NaN.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt around line 1019:
`json.floatDp` calls `JSONObject.optDouble()` for any present key, and `optDouble` returns `0.0` for `null` or non-numeric values. So `{ "rowHeight": null }` or `{ "rowHeight": "bad" }` yields `0px` instead of `fallbackPx`, collapsing the diff UI instead of preserving the default style. The same bug exists in `floatSp`. Consider checking `optDouble(key, Double.NaN)` (or guarding `isNull`/type) and falling back when the value is `NaN`.
| fun setContentResetKey(value: String) { | ||
| if (contentResetKey == value) return | ||
| contentResetKey = value | ||
| tokensDecodeGeneration += 1 | ||
| canvasView.tokensByRowId = emptyMap() |
There was a problem hiding this comment.
🟡 Medium t3reviewdiff/T3ReviewDiffView.kt:81
setContentResetKey clears tokens and scroll position but never clears rows, visibleRows, or canvasView.rows. Since new rowsJson is decoded asynchronously on a background thread, the previous file's rows remain rendered and tappable until the new rows finish decoding and are posted back. Users can see and interact with the wrong file's lines and comments after navigating to a different file. Consider clearing the rows in setContentResetKey so the view is blanked immediately while the new payload is in flight.
fun setContentResetKey(value: String) {
if (contentResetKey == value) return
contentResetKey = value
- tokensDecodeGeneration += 1
- canvasView.tokensByRowId = emptyMap()
+ rowsDecodeGeneration += 1
+ rows = emptyList()
+ visibleRows = emptyList()
+ canvasView.rows = emptyList()
+ canvasView.tokensByRowId = emptyMap()
lastVisibleFileId = null🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt around lines 81-85:
`setContentResetKey` clears tokens and scroll position but never clears `rows`, `visibleRows`, or `canvasView.rows`. Since new `rowsJson` is decoded asynchronously on a background thread, the previous file's rows remain rendered and tappable until the new rows finish decoding and are posted back. Users can see and interact with the wrong file's lines and comments after navigating to a different file. Consider clearing the rows in `setContentResetKey` so the view is blanked immediately while the new payload is in flight.
| fun setTokensPatchJson(value: String) { | ||
| payloadDecodeExecutor.execute { | ||
| try { | ||
| val payload = JSONObject(value) | ||
| val resetKey = payload.optString("resetKey") | ||
| val decodedTokens = parseTokensObject( | ||
| payload.optJSONObject("tokensByRowId") ?: JSONObject(), | ||
| ) | ||
| post { | ||
| if (resetKey.isNotEmpty() && resetKey != tokensResetKey) return@post | ||
| if (decodedTokens.isNotEmpty()) { | ||
| canvasView.tokensByRowId = canvasView.tokensByRowId + decodedTokens | ||
| } | ||
| } | ||
| } catch (_: Exception) { | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Medium t3reviewdiff/T3ReviewDiffView.kt:165
setTokensPatchJson decodes the patch on a background thread and, after posting back to the UI thread, only guards against a stale tokensResetKey. It never checks contentResetKey, so when the user switches to a different diff before the decode finishes, a patch from the old review is still merged into canvasView.tokensByRowId for the new content. The iOS version captures and checks contentResetKey, so Android can render stale syntax-highlighting tokens from the wrong review.
fun setTokensPatchJson(value: String) {
+ val capturedContentResetKey = contentResetKey
payloadDecodeExecutor.execute {
try {
val payload = JSONObject(value)
val resetKey = payload.optString("resetKey")
val decodedTokens = parseTokensObject(
payload.optJSONObject("tokensByRowId") ?: JSONObject(),
)
post {
- if (resetKey.isNotEmpty() && resetKey != tokensResetKey) return@post
+ if (resetKey.isNotEmpty() && resetKey != tokensResetKey) return@post
+ if (capturedContentResetKey != contentResetKey) return@post
if (decodedTokens.isNotEmpty()) {
canvasView.tokensByRowId = canvasView.tokensByRowId + decodedTokens
}
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt around lines 165-181:
`setTokensPatchJson` decodes the patch on a background thread and, after posting back to the UI thread, only guards against a stale `tokensResetKey`. It never checks `contentResetKey`, so when the user switches to a different diff before the decode finishes, a patch from the old review is still merged into `canvasView.tokensByRowId` for the new content. The iOS version captures and checks `contentResetKey`, so Android can render stale syntax-highlighting tokens from the wrong review.
| container.addView( | ||
| inputView, | ||
| LinearLayout.LayoutParams(1, 1), | ||
| inputView.addTextChangedListener( |
There was a problem hiding this comment.
🟠 High t3terminal/T3TerminalView.kt:200
TextWatcher.onTextChanged only forwards the inserted substring (s.subSequence(start, end)) and then clears the EditText in afterTextChanged. For replacement edits where before > 0 and count > 0—such as IME autocorrect replacing teh with the—Android reports only the replacement text, so this code emits the without first emitting a backspace to delete the previously sent teh. The terminal receives duplicated or mangled input instead of the final committed string.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt around line 200:
`TextWatcher.onTextChanged` only forwards the inserted substring (`s.subSequence(start, end)`) and then clears the `EditText` in `afterTextChanged`. For replacement edits where `before > 0` and `count > 0`—such as IME autocorrect replacing `teh` with `the`—Android reports only the replacement text, so this code emits `the` without first emitting a backspace to delete the previously sent `teh`. The terminal receives duplicated or mangled input instead of the final committed string.
| {Platform.OS === "android" ? ( | ||
| <AndroidScreenHeader | ||
| title={showScanner ? "Scan QR Code" : "Add Environment"} | ||
| onBack={() => navigation.goBack()} |
There was a problem hiding this comment.
🟠 High connection/ConnectionsNewRouteScreen.tsx:149
The Android back button in AndroidScreenHeader calls navigation.goBack() unconditionally, but goBack() is a no-op when this screen is the initial route (e.g. opened directly via the connections/new deep link) because there is no screen to go back to. The handleSubmit path at line 127 already guards against this with a canGoBack() fallback that dispatches StackActions.replace("Home"). The Android back button lacks that same fallback, leaving the user stranded. Consider applying the same canGoBack() / replace("Home") fallback to the onBack handler.
- onBack={() => navigation.goBack()}
+ onBack={() => {
+ if (navigation.canGoBack()) {
+ navigation.goBack();
+ } else {
+ navigation.dispatch(StackActions.replace("Home"));
+ }
+ }}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/connection/ConnectionsNewRouteScreen.tsx around line 149:
The Android back button in `AndroidScreenHeader` calls `navigation.goBack()` unconditionally, but `goBack()` is a no-op when this screen is the initial route (e.g. opened directly via the `connections/new` deep link) because there is no screen to go back to. The `handleSubmit` path at line 127 already guards against this with a `canGoBack()` fallback that dispatches `StackActions.replace("Home")`. The Android back button lacks that same fallback, leaving the user stranded. Consider applying the same `canGoBack()` / `replace("Home")` fallback to the `onBack` handler.
| private fun parseThemeConfig(config: String) { | ||
| val palette = sortedMapOf<Int, Int>() | ||
| for (line in config.lineSequence()) { |
There was a problem hiding this comment.
🟡 Medium t3terminal/T3TerminalView.kt:336
parseThemeConfig only overwrites cursorColorValue when the new config contains a cursor-color line, and only replaces paletteColors when the new config contains at least one palette entry. When themeConfig is later set to a reduced or empty string, the previously parsed cursorColorValue and paletteColors remain unchanged, so the terminal keeps stale colors that no longer match the intended theme. Consider resetting cursorColorValue and paletteColors to their defaults at the start of parseThemeConfig before processing the new config.
private fun parseThemeConfig(config: String) {
+ cursorColorValue = Color.parseColor("#009FFF")
+ paletteColors = IntArray(0)
val palette = sortedMapOf<Int, Int>()
for (line in config.lineSequence()) {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt around lines 336-338:
`parseThemeConfig` only overwrites `cursorColorValue` when the new config contains a `cursor-color` line, and only replaces `paletteColors` when the new config contains at least one `palette` entry. When `themeConfig` is later set to a reduced or empty string, the previously parsed `cursorColorValue` and `paletteColors` remain unchanged, so the terminal keeps stale colors that no longer match the intended theme. Consider resetting `cursorColorValue` and `paletteColors` to their defaults at the start of `parseThemeConfig` before processing the new config.
| val foreground = buffer.int | ||
| val background = buffer.int | ||
| val cursorColor = buffer.int | ||
| val cellCount = cols * rows |
There was a problem hiding this comment.
🟡 Medium t3terminal/TerminalFrame.kt:44
TerminalFrame.decode computes cellCount = cols * rows from two unsigned 16-bit values with no overflow or bound check. Malformed snapshot data (e.g. 65535 x 65535) makes the multiplication wrap to a negative Int, so IntArray(cellCount) throws IllegalArgumentException instead of returning null. A bad native snapshot therefore crashes the Android view path via renderSnapshot().
| val cellCount = cols * rows | |
| val cellCount = cols.toLong() * rows.toLong() | |
| if (cellCount <= 0 || cellCount > 200_000L) return null | |
| val cellCountInt = cellCount.toInt() |
Also found in 1 other location(s)
apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt:305
renderSnapshot()drops invalid/empty snapshots silently. The JNInativeSnapshot()implementation returns an emptyByteArraywhenghostty_render_state_updateorghostty_render_state_colors_getfails, andTerminalFrame.decode(...)then returnsnull. Because this method only updates the canvas inside?.let(...), the oldTerminalFramestays on screen after a failed snapshot, so scroll/resize/theme/input operations can leave stale terminal contents rendered with no indication that the snapshot refresh failed.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/TerminalFrame.kt around line 44:
`TerminalFrame.decode` computes `cellCount = cols * rows` from two unsigned 16-bit values with no overflow or bound check. Malformed snapshot data (e.g. `65535 x 65535`) makes the multiplication wrap to a negative `Int`, so `IntArray(cellCount)` throws `IllegalArgumentException` instead of returning `null`. A bad native snapshot therefore crashes the Android view path via `renderSnapshot()`.
Also found in 1 other location(s):
- apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt:305 -- `renderSnapshot()` drops invalid/empty snapshots silently. The JNI `nativeSnapshot()` implementation returns an empty `ByteArray` when `ghostty_render_state_update` or `ghostty_render_state_colors_get` fails, and `TerminalFrame.decode(...)` then returns `null`. Because this method only updates the canvas inside `?.let(...)`, the old `TerminalFrame` stays on screen after a failed snapshot, so scroll/resize/theme/input operations can leave stale terminal contents rendered with no indication that the snapshot refresh failed.
The review sheet force-unwrapped an iOS-only native view and crashed on Android whenever a parseable file diff opened. Port the Android Kotlin implementation (canvas diff surface with sticky headers, fixed gutters, JSON row/token feed) from @juliusmarminge's open upstream PR pingdotgg#3579, wire module registration for both platforms, and replace the force-unwrap with a themed unavailable-notice fallback.
Port MarkdownCodeBlock + highlight state from upstream PR pingdotgg#3579 onto our token-driven code block theme: shiki layers per-token foreground over the --color-md-* base, follows the system scheme, degrades to plain themed text while streaming or before the async highlight resolves. Assistant messages only; user bubbles stay plain.
Add the refreshing prop and onPullToRefresh event the PR pingdotgg#3579 port omitted, implemented as overscroll-at-top detection inside the view's own vertical drag axis (no competing SwipeRefreshLayout intercepts; horizontal pans unaffected), with a Material spinner badge themed from the view's theme JSON. Matches the iOS UIRefreshControl contract.
|
Hey, I've been impatient and working on my own implementation of t3 mobile for Android (fork: https://github.com/nnNyx/t3code). I know you don't take contributions, but since some of this is already drafted I'm still leaving a puny comment here in case ANY of this is useful to you. Everything below was tested through ADB on a Pixel 7 running GrapheneOS, which is my daily phone. Small heads-up first: I ported this PR's review-diff Android module into the fork (attribution kept in the file headers) and it works fine on that same hardware. Stuff I encountered along the way:
Also sitting in the fork if any of it helps this PR's remaining scope: If it's preferable, I can also file any of these as separate issues. |
Port the T3CODE_IOS_PERSONAL_TEAM flow from upstream PR pingdotgg#3579: opt-in env swaps the bundle id and drops the capabilities a free Apple ID cannot sign (team-pinned signing, associated domains, Sign in with Apple, widgets/app groups, push entitlements). Default builds are unchanged.
3 participants
Summary
Why
The mobile app inherited iOS-only native modules and navigation assumptions. Android could build only after filling those native module gaps, and several shared screens rendered with incorrect insets, missing icons, inaccessible controls, or desktop/iOS-oriented interaction patterns.
The review diff also scrolled its entire native canvas horizontally, which moved line gutters and file headers with the code. It now owns horizontal code offset internally so persistent chrome stays fixed.
Impact
Android now has a usable end-to-end thread, file, Git review, composer, and connection flow. The iOS implementation keeps using its existing native toolbar and form-sheet behavior through platform-specific branches.
Validation
vp checkvp run typecheckvp run lint:mobile./gradlew :t3tools-mobile-review-diff-native:compileDebugKotlin./gradlew :app:assembleDebugNote
Add Android support to the mobile app with native terminal, composer, and diff review
AndroidScreenHeaderandAndroidHeaderIconButtoncomponents replacing native navigation headers on Android across thread, file, settings, connection, and review screens.SymbolViewin AppSymbol.tsx that maps SF Symbols to Tabler icons on Android, updating all existing usages.T3ComposerEditorView), diff review surface (T3ReviewDiffView), and header button (T3HeaderButtonView) via Expo module wiring.MarkdownCodeBlockcomponent anduseMarkdownCodeHighlighthook.Macroscope summarized 2b34778.
Note
High Risk
Large new native surface area (especially the ~1k-line diff renderer and vendored libghostty-vt binaries/JNI) plus iOS signing/entitlement branching; regressions could affect core mobile editing, review, and terminal flows on both platforms.
Overview
Adds Android Kotlin implementations for the composer editor, native header controls, and review diff Expo modules so they mirror the existing iOS view contracts (props, events, async commands) without changing the shared React Native layer.
The review diff Android surface is a custom canvas with separate vertical/horizontal scrolling, sticky file headers, line gutters that stay fixed while code pans horizontally, JSON-driven rows/tokens, and the same interaction events as iOS.
The composer Android view adds a controlled
EditTextwith token chip spans, theme JSON, image paste handling, and selection/event-count sync with JS.Terminal work vendors libghostty-vt (pinned revision, headers, license, rebuild script/docs) for Android terminal state and rendering, replacing the prior placeholder Android terminal approach described in the module README.
Build/tooling: Metro now blocklists the repo
.t3directory from the workspace watch graph; rootapp.jsonis a minimal Expo stub; moduleexpo-module.config.jsonfiles register Android packages.iOS local dev: Optional
T3CODE_IOS_PERSONAL_TEAMflow overrides bundle ID, drops team-pinned signing/associated domains, disables Clerk Apple Sign-In and the widgets plugin, and applies a plugin to strip Personal Team–incompatible capabilities; README documentsios:releaseand Personal Team env vars.Reviewed by Cursor Bugbot for commit 2b34778. Bugbot is set up for automated code reviews on this repo. Configure here.