Feature/convov3 pt3#2036
Merged
Merged
Conversation
Passing down highlight key
…to properly compute the neighboring rules
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the ConversationV3 Compose implementation with richer message rendering and conversation-level behaviors (dialogs, scrolling/highlighting, expandable text), and refactors state models to use immutable collections for better Compose stability.
Changes:
- Introduces centralized scroll/highlight control (scroll-to-message, jump-to-bottom, highlight animation, scroll state reporting).
- Adds new UI components and behaviors (dialogs layer, quote-tap scrolling, link open command, expandable “read more” text).
- Migrates various Compose view-state models to
kotlinx.collections.immutableand adds stability annotations.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Adds version/catalog entry for kotlinx-collections-immutable. |
| app/build.gradle.kts | Adds kotlinx-collections-immutable dependency to the app module. |
| app/src/test/java/org/thoughtcrime/securesms/conversation/v3/ConversationUIRulesTest.kt | Adds unit tests for ConversationV3 UI rules (date breaks, clustering, author label). |
| app/src/main/java/org/thoughtcrime/securesms/util/AvatarUtils.kt | Marks avatar UI models immutable and switches to ImmutableList for Compose stability. |
| app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.kt | Updates InputbarViewModel constructor call to use context parameter. |
| app/src/main/java/org/thoughtcrime/securesms/database/model/Quote.kt | Adds quoteMessageId to support scroll-to-quoted-message behavior. |
| app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.kt | Populates quoteMessageId when building Quote from DB data. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/MessageText.kt | Adds optional onTextLayout callback forwarding; minor refactor of imports/comments. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/MessageQuote.kt | Adds quote click handling and introduces QuoteMessageData with quotedMessageId. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/MessageMedia.kt | Switches media items to ImmutableList and updates previews/highlight plumbing. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/MessageLink.kt | Makes link previews clickable and routes open-url via ConversationCommand. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/MessageEffects.kt | Adds rememberHighlightAlpha() and refactors highlight modifier to accept alpha directly. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/ExpandableMessageText.kt | Adds expandable “read more” message text with measurement-based expansion delta. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/DocumentMessage.kt | Updates previews to use persistent immutable lists. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/message/BaseMessage.kt | Thread-wide refactor: immutable state models, highlight plumbing, quote/link commands, expandable text wiring. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/conversation/ConversationV3Dialogs.kt | Adds a ConversationV3 dialogs layer for URL, delete, emoji clear, community join, etc. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/conversation/ConversationScreen.kt | Adds list controller integration, scroll-to-bottom button, scroll state reporting, and dialogs hookup. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/conversation/ConversationListState.kt | Introduces a single controller for paging-aware scroll-to-message and highlight targeting. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/compose/conversation/ConversationElements.kt | Adds scroll-to-bottom UI component and adjusts unread marker spacing. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationV3ViewModel.kt | Introduces scroll events, unread badge logic, dialogs state, and unifies commands under ConversationCommand. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationV3NavHost.kt | Adds support for a pending “scroll to message” request on entry, with consume callback. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationUIRules.kt | Adds centralized UI rules for clustering/date breaks/author labels. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationPagingSource.kt | Adds overlap-window paging to compute correct UI state across page boundaries. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationModels.kt | Adds new ConversationV3 models for scroll state and dialog state. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationDataMapper.kt | Uses ConversationUIRules, emits immutable content groups, and maps quoted message IDs for scrolling. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationCommand.kt | Adds a unified command contract for navigation/screen/message/dialog actions. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v3/ConversationActivityV3.kt | Adds pending-scroll handling via intent extras and forwards to nav host. |
| app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt | Updates InputbarViewModel constructor call to use context parameter. |
| app/src/main/java/org/thoughtcrime/securesms/InputbarViewModel.kt | Switches dependency from Application to Context for string/resource access. |
| .gitignore | Fixes script negation line formatting and ignores /.claude. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as resolved.
This comment was marked as resolved.
| if (lastLoadedIndex < 0) return false | ||
|
|
||
| val previousItemCount = itemCount | ||
| this[lastLoadedIndex] |
Collaborator
There was a problem hiding this comment.
What does this statement do?
Collaborator
Author
There was a problem hiding this comment.
Had to figure this out but with LazyPagingItems, get(index) notifies Paging that this item was accessed, which can trigger prefetch/append work.
Here that is being used on purpose to “touch” the end of the loaded snapshot so Paging loads the next older page.
Collaborator
There was a problem hiding this comment.
A comment here would prevent an accidental removal of this statement in the future.
| animateToPositioned(index) | ||
| } else { | ||
| lazyListState.scrollToItem(index) | ||
| yield() |
Collaborator
There was a problem hiding this comment.
Interesting...what does this one do? And how reliable is to say after yielding, the list state is at the final destination?
Collaborator
Author
There was a problem hiding this comment.
Its the coroutine yield. It can help assure the items are laid out though there can't be any guarantee.
Collaborator
Author
There was a problem hiding this comment.
It should give Compose a chance to re-measure after the instant scroll
Comment on lines
+319
to
+324
| for (i in snapshot.indices) { | ||
| if ((snapshot[i] as? ConversationItem.Message)?.data?.id == messageId) { | ||
| return i | ||
| } | ||
| } | ||
| return null |
Collaborator
There was a problem hiding this comment.
Isn't there a indexOfFirstOrNull for this kind of thing?
Collaborator
Author
There was a problem hiding this comment.
I don't think so. Is there a particular one you're thinking of?
Collaborator
There was a problem hiding this comment.
I thought there is a snapshot.indexOfFirstOrNull Kotlin official function you can call to find out the first index that satisfies a lambda. But perhaps the snapshot here is not a List?
| /** | ||
| * Helper class for UI rules in the conversation | ||
| */ | ||
| internal object ConversationUIRules { |
Collaborator
There was a problem hiding this comment.
I don't think we need to use internal - we are in the app module and it doesnt make sense
SessionHero01
approved these changes
Mar 11, 2026
ThomasSession
enabled auto-merge
March 11, 2026 23:40
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
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.
More ConversationV3 logic.
Added dialogs
Scroll logic - Including to specific quoted messages
Highlight logic
Expandable "read more" messages
"Jump to bottom" button and logic
Cleaned up logic around UI rules, pagination and message access
Immutable lists to cater for Stable compose state