Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces reusable Compose components for rendering conversation messages, refactoring the landing screen to use these new components. The changes create a foundation for consistent message display across the app.
Changes:
- Introduced comprehensive message rendering composables supporting text, media, audio, documents, quotes, and links
- Refactored the landing screen to use the new message composables instead of custom implementations
- Updated theme colors and dimensions to support the new message UI components
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| MessageComposables.kt | New file containing all message rendering composables, data models, and preview functions for various message types |
| MessageMedia.kt | New file with media message composables supporting 1-3 images/videos with different layout patterns |
| Landing.kt | Refactored to use new Message composable instead of custom message rendering logic |
| ThemeColors.kt | Updated text bubble colors for all themes to use theme-specific colors instead of hardcoded black/white |
| Dimensions.kt | Added message-specific dimensions including corner radius, min width, icon status size, and quote icon size |
| Colors.kt | Added new black alpha values (blackAlpha12, blackAlpha06) for message backgrounds |
| Themes.kt | Removed unused imports (AppTextSecurePreferences, dp) |
| view_visible_message.xml | Added tools:tint attribute for preview support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| data object DisappearingMessageIcon: MessageViewStatusIcon | ||
| } | ||
|
|
||
| sealed class MessageType(){ |
There was a problem hiding this comment.
Sealed classes should not have empty parentheses when they don't have a primary constructor. This is inconsistent with Kotlin style guidelines and other sealed classes in the codebase. Remove the empty parentheses.
| ): MessageType() | ||
| } | ||
|
|
||
| sealed class MessageAudioState(){ |
There was a problem hiding this comment.
Sealed classes should not have empty parentheses when they don't have a primary constructor. This is inconsistent with Kotlin style guidelines and other sealed classes in the codebase. Remove the empty parentheses.
| } | ||
| } | ||
|
|
||
| sealed class MessageMediaItem(){ |
There was a problem hiding this comment.
Sealed classes should not have empty parentheses when they don't have a primary constructor. This is inconsistent with Kotlin style guidelines and other sealed classes in the codebase. Remove the empty parentheses.
| sealed class MessageMediaItem(){ | |
| sealed class MessageMediaItem { |
| val messages = remember { | ||
| listOf( | ||
| MessageViewData( | ||
| type = MessageType.Text(text = AnnotatedString( | ||
| Phrase.from(context.getString(R.string.onboardingBubbleWelcomeToSession)) | ||
| .put(APP_NAME_KEY, context.getString(R.string.app_name)) | ||
| .put(EMOJI_KEY, "\uD83D\uDC4B") // this hardcoded emoji might be moved to NonTranslatableConstants eventually | ||
| .format().toString() | ||
| ), outgoing = false), | ||
| author = "Test" | ||
| ), | ||
| MessageViewData( | ||
| type = MessageType.Text(text = AnnotatedString( | ||
| Phrase.from(context.getString(R.string.onboardingBubbleSessionIsEngineered)) | ||
| .put(APP_NAME_KEY, context.getString(R.string.app_name)) | ||
| .format().toString()), outgoing = true), | ||
| author = "Test" | ||
| ), | ||
| MessageViewData( | ||
| type = MessageType.Text(text = AnnotatedString(context.getString(R.string.onboardingBubbleNoPhoneNumber)), outgoing = false), | ||
| author = "Test" | ||
| ), | ||
| MessageViewData( | ||
| type = MessageType.Text(text = AnnotatedString( | ||
| Phrase.from(context.getString(R.string.onboardingBubbleCreatingAnAccountIsEasy)) | ||
| .put(EMOJI_KEY, "\uD83D\uDC47") // this hardcoded emoji might be moved to NonTranslatableConstants eventually | ||
| .format().toString() | ||
| ), outgoing = true), | ||
| author = "Test" | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
The remember block uses context inside but doesn't include it as a key. While context is unlikely to change in onboarding, it's better to be explicit by adding context as a key: remember(context) { ... }. This ensures the messages are recomputed if the context somehow changes.
| avatar = PreviewMessageData.sampleAvatar, | ||
| type = PreviewMessageData.document( | ||
| outgoing = false, | ||
| name = "Document with a really long name that should ellepsize once it reaches the max width" |
There was a problem hiding this comment.
The word "ellepsize" is misspelled. It should be "ellipsize".
| name = "Document with a really long name that should ellepsize once it reaches the max width" | |
| name = "Document with a really long name that should ellipsize once it reaches the max width" |
| type = PreviewMessageData.audio( | ||
| outgoing = false, | ||
| state = MessageAudioState.Loading, | ||
| name = "Audio with a really long name that should ellepsize once it reaches the max width" |
There was a problem hiding this comment.
The word "ellepsize" is misspelled. It should be "ellipsize".
| name = "Audio with a really long name that should ellepsize once it reaches the max width" | |
| name = "Audio with a really long name that should ellipsize once it reaches the max width" |
| subtitle = subtitle | ||
| , |
There was a problem hiding this comment.
The comma is incorrectly placed on its own line. It should be at the end of the previous line (line 1165).
| subtitle = subtitle | |
| , | |
| subtitle = subtitle, |
| val icon: MessageQuoteIcon | ||
| ) | ||
|
|
||
| sealed class MessageQuoteIcon(){ |
There was a problem hiding this comment.
Sealed classes and sealed interfaces should not have empty parentheses when they don't have a primary constructor. This is inconsistent with Kotlin style guidelines and other sealed classes in the codebase. Remove the empty parentheses.
| sealed class MessageQuoteIcon(){ | |
| sealed class MessageQuoteIcon { |
1 participant
I'm bringing the start of the compose content for the conversation screen so we can start using it in other places.