Chat UX polish: italic, clickable links, thumbnails, streaming autoscroll - #8
Merged
Merged
Conversation
…roll - MarkdownText: italic now actually renders italic (was a no-op style); markdown links [label](url) and bare http(s)://… URLs become clickable via ClickableText + ACTION_VIEW intent. - ChatScreen: sent image attachments render a 120dp thumbnail above the text in the user bubble; autoscroll now also keys on the tail message text so the view stays pinned to the bottom during streaming. - Attachments are copied into <filesDir>/attachments and their paths are stored on GeminiMessage.attachmentPaths (and persisted via ChatStore) so reloads still show thumbnails.
Reviewer's GuidePolishes the chat UX by fixing italic markdown rendering, adding clickable links in model output, showing persistent image thumbnails for user messages, and improving streaming autoscroll while threading attachment file paths through view model, domain, and storage layers. Sequence diagram for image attachment lifecycle and thumbnailssequenceDiagram
actor User
participant ChatScreen
participant ChatViewModel
participant RestGeminiCore
participant ChatStore
participant FileSystem
User->>ChatScreen: Selects image attachment
ChatScreen->>ChatViewModel: onAttachmentPicked(uri)
ChatViewModel->>ChatViewModel: loadPendingAttachment(uri)
ChatViewModel->>ChatViewModel: persistAttachment(context, id, bytes, mime)
ChatViewModel->>FileSystem: Write bytes to filesDir/attachments/id.ext
FileSystem-->>ChatViewModel: absolutePath
ChatViewModel-->>ChatScreen: PendingAttachment(localPath)
User->>ChatScreen: Presses send
ChatScreen->>ChatViewModel: sendMessage(text, pendingAttachments)
ChatViewModel->>ChatViewModel: Map PendingAttachment to Attachment(bytes, mimeType, localPath)
ChatViewModel->>RestGeminiCore: startChatTurn(text, attachments)
RestGeminiCore->>RestGeminiCore: Build GeminiMessage
RestGeminiCore->>RestGeminiCore: attachmentPaths = attachments.mapNotNull(localPath)
RestGeminiCore->>ChatStore: saveMessage(GeminiMessage(attachmentPaths))
ChatStore->>FileSystem: Write JSON with attachmentPaths
ChatStore-->>RestGeminiCore: Persisted
RestGeminiCore-->>ChatViewModel: Streamed response
ChatViewModel-->>ChatScreen: Updated message list
ChatStore->>ChatStore: loadMessage(json)
ChatStore->>FileSystem: Read JSON with attachmentPaths
ChatStore-->>RestGeminiCore: GeminiMessage(attachmentPaths)
RestGeminiCore-->>ChatViewModel: Restored conversation
ChatViewModel-->>ChatScreen: Messages with attachmentPaths
ChatScreen->>ChatScreen: MessageBubble(message)
ChatScreen->>ChatScreen: AttachmentThumbnails(message.attachmentPaths)
ChatScreen->>FileSystem: decodeThumbnail(path) via BitmapFactory
FileSystem-->>ChatScreen: ImageBitmap
ChatScreen-->>User: Bubble shows 120dp thumbnails above text
Sequence diagram for clickable links in model outputsequenceDiagram
actor User
participant ChatScreen
participant MarkdownText
participant ProseText
participant AndroidContext
participant SystemBrowser
ChatScreen->>MarkdownText: Render model message text
MarkdownText->>MarkdownText: annotateInline(line, baseColor)
MarkdownText->>MarkdownText: Detect [label](url) and bare http(s) URLs
MarkdownText->>MarkdownText: pushStringAnnotation(URL_TAG, url)
MarkdownText->>ProseText: ProseText(AnnotatedString)
ProseText->>ProseText: hasLinks = getStringAnnotations(URL_TAG).isNotEmpty()
ProseText->>User: Display ClickableText with underlined links
User->>ProseText: Taps on link
ProseText->>ProseText: getStringAnnotations(URL_TAG, offset)
ProseText->>AndroidContext: openUrl(context, url)
AndroidContext->>AndroidContext: Normalise URL and build ACTION_VIEW Intent
AndroidContext->>SystemBrowser: startActivity(intent)
SystemBrowser-->>User: Open link in system browser
Updated class diagram for GeminiMessage attachments and thumbnailsclassDiagram
class GeminiMessage {
String id
String text
Boolean isUser
Long timestamp
MessageRole role
ToolCall toolCall
ToolCallResult toolResult
List~String~ attachmentPaths
}
class Attachment {
ByteArray bytes
String mimeType
String localPath
equals(other)
hashCode()
}
class PendingAttachment {
String id
ByteArray bytes
String mimeType
String displayName
Int sizeBytes
String localPath
equals(other)
hashCode()
}
class ChatViewModel {
RestGeminiCore core
sendMessage(text, attachments)
loadPendingAttachment(context, uri)
persistAttachment(context, id, bytes, mime)
}
class RestGeminiCore {
startChatTurn(text, attachments)
buildUserGeminiMessage(text, attachments)
}
class ChatStore {
saveMessage(GeminiMessage)
loadMessage(jsonObject)
}
ChatViewModel --> PendingAttachment : creates
ChatViewModel --> Attachment : maps_to_payload
Attachment --> GeminiMessage : localPath_copied_to_attachmentPaths
RestGeminiCore --> GeminiMessage : constructs
ChatStore --> GeminiMessage : persists_and_restores
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe pull request adds image attachment thumbnail rendering in the chat UI with local file persistence, and introduces interactive URL link support in markdown messages. Changes span the UI layer (ChatScreen), view model (ChatViewModel), message serialization (ChatStore, RestGeminiCore), domain model (GeminiMessage), and markdown rendering (MarkdownText). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatScreen
participant ChatViewModel
participant StorageAPI as App Storage
participant RestGeminiCore
participant ChatStore as Message Store
User->>ChatScreen: Select image attachment
ChatScreen->>ChatViewModel: readAttachment(uri, type)
ChatViewModel->>ChatViewModel: Generate attachment ID
ChatViewModel->>StorageAPI: persistAttachment(bytes, id, mimeType)
StorageAPI-->>ChatViewModel: localPath (absolute file path)
ChatViewModel->>ChatViewModel: Create PendingAttachment(id, localPath)
User->>ChatViewModel: sendMessage(text, attachments)
ChatViewModel->>ChatViewModel: Create Attachment(bytes, mimeType, localPath)
ChatViewModel->>RestGeminiCore: sendMessage(message, attachments)
RestGeminiCore->>RestGeminiCore: Map localPaths to attachmentPaths
RestGeminiCore->>RestGeminiCore: Create GeminiMessage(attachmentPaths)
RestGeminiCore->>ChatStore: Store message
ChatStore->>ChatStore: Serialize attachmentPaths to JSON
RestGeminiCore-->>ChatScreen: GeminiMessage with attachmentPaths
ChatScreen->>ChatScreen: decodeThumbnail(path) for each path
ChatScreen->>ChatScreen: Render AttachmentThumbnails
ChatScreen-->>User: Display message with thumbnails
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- Decoding thumbnails synchronously in
decodeThumbnail(called fromAttachmentThumbnails) does file IO and bitmap work on the main thread; consider moving this to a background/produceState-style loader or using an image-loading library to avoid jank when many attachments are present. - The hardcoded
MaterialThemeLinkcolor inannotateInlinebypasses the current theme’s color scheme; you may want to derive the link color fromMaterialTheme.colorScheme(e.g.,primary) and thread it through instead of keeping a fixed constant to improve theming and contrast behavior. - The comment on
persistAttachmentsays the image is copied into cache, but it currently usescontext.filesDir/attachments; either switch tocacheDiror adjust the comment to reflect that these thumbnails are persisted in app files, not cache.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Decoding thumbnails synchronously in `decodeThumbnail` (called from `AttachmentThumbnails`) does file IO and bitmap work on the main thread; consider moving this to a background/`produceState`-style loader or using an image-loading library to avoid jank when many attachments are present.
- The hardcoded `MaterialThemeLink` color in `annotateInline` bypasses the current theme’s color scheme; you may want to derive the link color from `MaterialTheme.colorScheme` (e.g., `primary`) and thread it through instead of keeping a fixed constant to improve theming and contrast behavior.
- The comment on `persistAttachment` says the image is copied into cache, but it currently uses `context.filesDir/attachments`; either switch to `cacheDir` or adjust the comment to reflect that these thumbnails are persisted in app files, not cache.
## Individual Comments
### Comment 1
<location path="app/src/main/kotlin/com/gemini/app/ui/chat/ChatScreen.kt" line_range="1090-1099" />
<code_context>
+ Row(horizontalArrangement = Arrangement.spacedBy(6.dp)) {
</code_context>
<issue_to_address>
**issue (performance):** Bitmap decoding is happening synchronously on the main thread, which can cause jank for large or multiple images.
`decodeThumbnail(path)` is run inside `remember { ... }`, so both the bounds check and `BitmapFactory.decodeFile` execute on the main thread during composition. Decoding several large (20MB+) images this way can stall the UI. Move decoding to a background dispatcher (e.g. `produceState` / `LaunchedEffect` with `withContext(Dispatchers.IO)`) or use an image-loading library that manages threading and caching for you.
</issue_to_address>
### Comment 2
<location path="app/src/main/kotlin/com/gemini/app/ui/chat/ChatViewModel.kt" line_range="269-278" />
<code_context>
+ // Copy the picked image into app-owned cache so the chat bubble can show a
+ // thumbnail without holding an Android content:// permission that may be
+ // revoked, and so reloads from ChatStore can still find the file.
+ private fun persistAttachment(
+ context: Context,
+ id: String,
+ bytes: ByteArray,
+ mime: String
+ ): String? = runCatching {
+ val ext = when {
+ mime.contains("png") -> "png"
+ mime.contains("webp") -> "webp"
+ mime.contains("gif") -> "gif"
+ mime.contains("heic") -> "heic"
+ mime.contains("heif") -> "heif"
+ else -> "jpg"
+ }
+ val dir = java.io.File(context.filesDir, "attachments").also { it.mkdirs() }
+ val file = java.io.File(dir, "$id.$ext")
+ file.writeBytes(bytes)
+ file.absolutePath
+ }.getOrNull()
</code_context>
<issue_to_address>
**issue (bug_risk):** Persisted attachment files under `filesDir/attachments` are never cleaned up and may accumulate indefinitely.
Because these files are never deleted, internal storage can grow without bound, particularly for users who send many large images. Consider using `cacheDir` so the system can reclaim space, or implement an explicit cleanup strategy (e.g., tie deletion to message removal, cap the directory size, or periodically delete old attachments).
</issue_to_address>
### Comment 3
<location path="app/src/main/kotlin/com/gemini/app/ui/chat/MarkdownText.kt" line_range="328-330" />
<code_context>
+ )
+}
+
+private fun openUrl(context: Context, url: String) {
+ runCatching {
+ val normalised = if (url.startsWith("http://") || url.startsWith("https://")) url
+ else "https://$url"
+ val intent = Intent(Intent.ACTION_VIEW, Uri.parse(normalised))
</code_context>
<issue_to_address>
**issue (bug_risk):** For non-http(s) links (mailto:, tel:, custom app schemes), the normalization prepends `https://`, which breaks those URIs.
The normalization currently assumes anything not starting with `http://` or `https://` is a bare host and prepends `https://`, so `mailto:foo@bar.com` becomes `https://mailto:foo@bar.com`. Instead, distinguish between URLs with and without a scheme (e.g., `Uri.parse(url).scheme == null` or a host-style regex) and only prepend `https://` when there is no scheme, leaving `mailto:`, `tel:`, and custom schemes unchanged.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1090
to
+1099
| Row(horizontalArrangement = Arrangement.spacedBy(6.dp)) { | ||
| paths.forEach { path -> | ||
| val bitmap = remember(path) { decodeThumbnail(path) } | ||
| if (bitmap != null) { | ||
| Image( | ||
| bitmap = bitmap, | ||
| contentDescription = null, | ||
| contentScale = ContentScale.Crop, | ||
| modifier = Modifier | ||
| .size(120.dp) |
There was a problem hiding this comment.
issue (performance): Bitmap decoding is happening synchronously on the main thread, which can cause jank for large or multiple images.
decodeThumbnail(path) is run inside remember { ... }, so both the bounds check and BitmapFactory.decodeFile execute on the main thread during composition. Decoding several large (20MB+) images this way can stall the UI. Move decoding to a background dispatcher (e.g. produceState / LaunchedEffect with withContext(Dispatchers.IO)) or use an image-loading library that manages threading and caching for you.
Comment on lines
+269
to
+278
| private fun persistAttachment( | ||
| context: Context, | ||
| id: String, | ||
| bytes: ByteArray, | ||
| mime: String | ||
| ): String? = runCatching { | ||
| val ext = when { | ||
| mime.contains("png") -> "png" | ||
| mime.contains("webp") -> "webp" | ||
| mime.contains("gif") -> "gif" |
There was a problem hiding this comment.
issue (bug_risk): Persisted attachment files under filesDir/attachments are never cleaned up and may accumulate indefinitely.
Because these files are never deleted, internal storage can grow without bound, particularly for users who send many large images. Consider using cacheDir so the system can reclaim space, or implement an explicit cleanup strategy (e.g., tie deletion to message removal, cap the directory size, or periodically delete old attachments).
Comment on lines
+328
to
+330
| private fun openUrl(context: Context, url: String) { | ||
| runCatching { | ||
| val normalised = if (url.startsWith("http://") || url.startsWith("https://")) url |
There was a problem hiding this comment.
issue (bug_risk): For non-http(s) links (mailto:, tel:, custom app schemes), the normalization prepends https://, which breaks those URIs.
The normalization currently assumes anything not starting with http:// or https:// is a bare host and prepends https://, so mailto:foo@bar.com becomes https://mailto:foo@bar.com. Instead, distinguish between URLs with and without a scheme (e.g., Uri.parse(url).scheme == null or a host-style regex) and only prepend https:// when there is no scheme, leaving mailto:, tel:, and custom schemes unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Small UX corrections in the conversation view:
_foo_/*foo*now actually render italic (the priorSpanStylewas a no-op that only reset weight/decoration).[label](https://…)and barehttp(s)://…URLs in model output are now tappable and open in the system browser viaACTION_VIEW. Implemented withClickableText+pushStringAnnotation(URL_TAG, …); falls back to plainTextwhen a paragraph has no URLs so selection/long-press still works.<filesDir>/attachments/att-<nano>.<ext>on pick so (a) the thumbnail survivesChatStorereload, and (b) we don't depend on the originalcontent://URI permission.GeminiMessagegainedattachmentPaths: List<String>, persisted byChatStore. The📎 image (png, 128KB)\nprefix RestGeminiCore adds to the bubble is hidden when a thumbnail is present (the thumbnail already conveys it).LaunchedEffectnow also keys on the tail message's text, not onlymessages.size, so the view stays pinned to the bottom while a model response streams in. The "only scroll if the user is near the bottom" rule is preserved.Files touched
app/.../ui/chat/MarkdownText.kt— italic fix, URL annotations (markdown + bare),ProseTextwrapper,openUrlintent helper.app/.../ui/chat/ChatScreen.kt—tailTextkey on autoscroll effect,AttachmentThumbnailscomposable +decodeThumbnail(sampledBitmapFactory, ~320 px target).app/.../ui/chat/ChatViewModel.kt—persistAttachmentcopies picked bytes into cache;PendingAttachment/AttachmentcarrylocalPath.core-bridge/.../RestGeminiCore.kt— propagatelocalPathonto the user'sGeminiMessage.attachmentPaths.core-bridge/.../storage/ChatStore.kt— persist & restoreattachmentPaths.domain/.../GeminiCore.kt— newattachmentPathsfield onGeminiMessage.Test plan
_italique_,*italique*,[Google](https://google.com), and a barehttps://example.com/foo— verify italic renders, both links open the browser.📎 image …prefix line.https://claude.ai/code/session_015dVKN2jG34HKP9SeBpSvS5
Summary by Sourcery
Polish the chat conversation UI with richer text rendering, clickable links, image thumbnails for user messages, and smoother autoscroll during streaming responses.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Improvements