compose: Announce message sender to screen readers#6504
Draft
andremion wants to merge 1 commit into
Draft
Conversation
Add per-message sender attribution for TalkBack so every message announces who sent it, independent of the visual grouping that hides the author name on consecutive messages from the same user.
The sender is announced once per message:
- Text messages prefix the text with "You said" / "{sender} said".
- Attachment messages (image, video, multiple media, file, voice message, giphy) announce the sender on the attachment label, with the attachment card still individually focusable.
- Poll and deleted messages are attributed too.
- Reply messages announce "replied" instead of "said".
A shared senderAwareContentDescription helper builds the label, and an additive AttachmentState.announceSender flag marks the single attachment that carries the sender. Adds translations for the new strings in all supported locales.
Contributor
Author
|
@CodeRabbit review |
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
✅ Action performedReview finished.
|
WalkthroughAdds sender-aware TalkBack accessibility content descriptions across all Compose message UI components. A new ChangesSender-Aware TalkBack Accessibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileAttachmentContent.kt (1)
111-119: ⚡ Quick winMove
modifierfrom the innerColumnto the outerBox.Lines 111-119 currently apply the public modifier to a nested node; applying it at the wrapper level avoids inconsistent padding/background/semantics behavior.
Suggested refactor
- Box( - modifier = Modifier.semantics { + Box( + modifier = modifier.semantics { contentDescription = rowDescription isTraversalGroup = true }, ) { Column( - modifier = modifier + modifier = Modifier .combinedClickable(Based on learnings: "In Compose UI components within the stream-chat-android-compose module, always apply modifiers to the outermost composable in the hierarchy."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileAttachmentContent.kt` around lines 111 - 119, The modifier parameter is currently applied to the nested Column composable instead of the outer Box wrapper. Move the modifier = modifier assignment from the Column (which is inside the Box with semantics) to the Box itself, and remove it from the Column. This ensures the modifier is applied at the outermost composable level in the hierarchy, which provides consistent behavior for padding, background, and semantics properties.Source: Learnings
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/AudioRecordAttachmentContent.kt (1)
112-120: ⚡ Quick winApply the incoming
modifierto the outermost wrapper.Lines 112-120 place
modifieron the innerColumn; moving it to the outerBoxkeeps behavior consistent with module conventions for Compose containers.Suggested refactor
- Box( - modifier = Modifier.semantics { + Box( + modifier = modifier.semantics { contentDescription = rowDescription isTraversalGroup = true }, ) { Column( - modifier = modifier.applyIf(!shouldBeFullSize) { padding(MessageStyling.messageSectionPadding) }, + modifier = Modifier.applyIf(!shouldBeFullSize) { padding(MessageStyling.messageSectionPadding) }, verticalArrangement = Arrangement.spacedBy(MessageStyling.sectionsDistance), ) {Based on learnings: "In Compose UI components within the stream-chat-android-compose module, always apply modifiers to the outermost composable in the hierarchy."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/AudioRecordAttachmentContent.kt` around lines 112 - 120, The modifier parameter is currently being applied to the inner Column composable instead of the outermost Box wrapper. Move the modifier application from the Column (which currently has the applyIf logic with MessageStyling.messageSectionPadding) to the outer Box composable to align with module conventions. The Column should be updated to remove the modifier parameter entirely, while the Box should receive the same modifier with the conditional padding logic preserved.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.kt`:
- Line 121: The `@Suppress`("LongMethod") annotation on line 121 lacks
documentation and violates the coding guideline that suppressions must be
explicitly documented. Either remove this suppression and refactor the method
into smaller helper functions to reduce its length, or keep the suppression but
add a documented comment explaining the rationale for why this specific method
needs to be long and cannot be reasonably broken down, potentially with tracking
context or issue references.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageContent.kt`:
- Around line 148-153: The senderAwareContentDescription function call when
building deletedDescription is missing the isReply parameter, which causes
deleted replies to announce incorrectly as "said" instead of "replied". Locate
where deletedDescription is assigned using senderAwareContentDescription and add
the isReply state as a parameter to this function call, ensuring it matches how
isReply is passed to other senderAwareContentDescription calls for non-deleted
messages in the same composable.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/PollMessageContent.kt`:
- Around line 221-226: The senderAwareContentDescription function call in the
poll message announcement block is missing the isReply parameter, which causes
reply context to be lost in accessibility announcements. Add the isReply
parameter to the senderAwareContentDescription function call alongside the
existing isMine, senderName, and content parameters to ensure that poll replies
are properly announced as "replied" rather than just "said" for screen reader
users.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/list/MessageContainerSenderAttributionTest.kt`:
- Around line 71-72: The test assertions use hardcoded English accessibility
strings like "You said, Hello" in onNodeWithContentDescription calls, which
makes the tests fragile to locale changes and copy updates. Replace all
instances of hardcoded accessibility strings (at lines 71-72, 88-89, 99-100,
113-114, 126-127, 139-140, and 219-220) with dynamically loaded strings from
stringResource IDs. This will require fetching the actual string resources used
in the composable being tested and using those values in the assertions instead
of hardcoding them, ensuring the tests remain stable as translations and copy
text evolve.
---
Nitpick comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/AudioRecordAttachmentContent.kt`:
- Around line 112-120: The modifier parameter is currently being applied to the
inner Column composable instead of the outermost Box wrapper. Move the modifier
application from the Column (which currently has the applyIf logic with
MessageStyling.messageSectionPadding) to the outer Box composable to align with
module conventions. The Column should be updated to remove the modifier
parameter entirely, while the Box should receive the same modifier with the
conditional padding logic preserved.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileAttachmentContent.kt`:
- Around line 111-119: The modifier parameter is currently applied to the nested
Column composable instead of the outer Box wrapper. Move the modifier = modifier
assignment from the Column (which is inside the Box with semantics) to the Box
itself, and remove it from the Column. This ensures the modifier is applied at
the outermost composable level in the hierarchy, which provides consistent
behavior for padding, background, and semantics properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0e6f3660-f564-4687-824c-593c0a0fb934
📒 Files selected for processing (19)
stream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/state/messages/attachments/AttachmentState.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/AudioRecordAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/GiphyAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageText.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/PollMessageContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageSenderDescription.ktstream-chat-android-compose/src/main/res/values-es/strings.xmlstream-chat-android-compose/src/main/res/values-fr/strings.xmlstream-chat-android-compose/src/main/res/values-hi/strings.xmlstream-chat-android-compose/src/main/res/values-in/strings.xmlstream-chat-android-compose/src/main/res/values-it/strings.xmlstream-chat-android-compose/src/main/res/values-ja/strings.xmlstream-chat-android-compose/src/main/res/values-ko/strings.xmlstream-chat-android-compose/src/main/res/values/strings.xmlstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/list/MessageContainerSenderAttributionTest.kt
| * By default it is used to display a play button over video previews. | ||
| */ | ||
| @OptIn(ExperimentalFoundationApi::class) | ||
| @Suppress("LongMethod") |
There was a problem hiding this comment.
Document or remove the new LongMethod suppression.
Line 121 introduces @Suppress("LongMethod") without rationale. Please either extract helper blocks and drop the suppression, or add a short documented reason (ideally with tracking context).
As per coding guidelines: "**/*.kt: Use @OptIn annotations explicitly in Kotlin code; avoid suppressions unless documented."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.kt`
at line 121, The `@Suppress`("LongMethod") annotation on line 121 lacks
documentation and violates the coding guideline that suppressions must be
explicitly documented. Either remove this suppression and refactor the method
into smaller helper functions to reduce its length, or keep the suppression but
add a documented comment explaining the rationale for why this specific method
needs to be long and cannot be reasonably broken down, potentially with tracking
context or issue references.
Source: Coding guidelines
Comment on lines
+148
to
153
| val isMine = currentUser?.id == message.user.id | ||
| val contentColor = MessageStyling.textColor(outgoing = isMine) | ||
| val deletedText = stringResource(id = R.string.stream_compose_message_deleted) | ||
| // Attribute the sender on deleted messages too, so every message announces who sent it (DS-035). | ||
| val deletedDescription = senderAwareContentDescription(isMine, message.user.name, deletedText) | ||
| Row( |
There was a problem hiding this comment.
Pass reply state into deleted-message sender description.
Line 152 builds deletedDescription without isReply, so deleted replies are announced as “said” instead of “replied”.
Suggested fix
- val deletedDescription = senderAwareContentDescription(isMine, message.user.name, deletedText)
+ val deletedDescription = senderAwareContentDescription(
+ isMine = isMine,
+ senderName = message.user.name,
+ content = deletedText,
+ isReply = message.replyTo != null,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val isMine = currentUser?.id == message.user.id | |
| val contentColor = MessageStyling.textColor(outgoing = isMine) | |
| val deletedText = stringResource(id = R.string.stream_compose_message_deleted) | |
| // Attribute the sender on deleted messages too, so every message announces who sent it (DS-035). | |
| val deletedDescription = senderAwareContentDescription(isMine, message.user.name, deletedText) | |
| Row( | |
| val isMine = currentUser?.id == message.user.id | |
| val contentColor = MessageStyling.textColor(outgoing = isMine) | |
| val deletedText = stringResource(id = R.string.stream_compose_message_deleted) | |
| // Attribute the sender on deleted messages too, so every message announces who sent it (DS-035). | |
| val deletedDescription = senderAwareContentDescription( | |
| isMine = isMine, | |
| senderName = message.user.name, | |
| content = deletedText, | |
| isReply = message.replyTo != null, | |
| ) | |
| Row( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageContent.kt`
around lines 148 - 153, The senderAwareContentDescription function call when
building deletedDescription is missing the isReply parameter, which causes
deleted replies to announce incorrectly as "said" instead of "replied". Locate
where deletedDescription is assigned using senderAwareContentDescription and add
the isReply state as a parameter to this function call, ensuring it matches how
isReply is passed to other senderAwareContentDescription calls for non-deleted
messages in the same composable.
Comment on lines
+221
to
+226
| // Announce the sender on the poll name so screen readers attribute every message (DS-035). | ||
| val senderAwareName = senderAwareContentDescription( | ||
| isMine = isMine, | ||
| senderName = message.user.name, | ||
| content = poll.name, | ||
| ) |
There was a problem hiding this comment.
Include reply context in poll sender announcement.
Lines 222-226 don’t pass isReply, so poll replies are announced with “said” instead of “replied”.
Suggested fix
val senderAwareName = senderAwareContentDescription(
isMine = isMine,
senderName = message.user.name,
content = poll.name,
+ isReply = message.replyTo != null,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Announce the sender on the poll name so screen readers attribute every message (DS-035). | |
| val senderAwareName = senderAwareContentDescription( | |
| isMine = isMine, | |
| senderName = message.user.name, | |
| content = poll.name, | |
| ) | |
| // Announce the sender on the poll name so screen readers attribute every message (DS-035). | |
| val senderAwareName = senderAwareContentDescription( | |
| isMine = isMine, | |
| senderName = message.user.name, | |
| content = poll.name, | |
| isReply = message.replyTo != null, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/PollMessageContent.kt`
around lines 221 - 226, The senderAwareContentDescription function call in the
poll message announcement block is missing the isReply parameter, which causes
reply context to be lost in accessibility announcements. Add the isReply
parameter to the senderAwareContentDescription function call alongside the
existing isMine, senderName, and content parameters to ensure that poll replies
are properly announced as "replied" rather than just "said" for screen reader
users.
Comment on lines
+71
to
+72
| .onNodeWithContentDescription("You said, Hello") | ||
| .assertExists() |
There was a problem hiding this comment.
Avoid hardcoded English accessibility strings in assertions.
These checks are locale- and wording-fragile. Build expected labels from stringResource IDs (or set test locale explicitly) so this suite stays stable as translations/copy evolve.
Proposed test hardening
+import androidx.test.core.app.ApplicationProvider
+import android.content.Context
+import io.getstream.chat.android.compose.R
...
internal class MessageContainerSenderAttributionTest : MockedChatClientTest {
+ private val context: Context = ApplicationProvider.getApplicationContext()
...
- composeTestRule
- .onNodeWithContentDescription("You said, Hello")
- .assertExists()
+ val expected = context.getString(R.string.stream_compose_message_sender_self, "Hello")
+ composeTestRule.onNodeWithContentDescription(expected).assertExists()Also applies to: 88-89, 99-100, 113-114, 126-127, 139-140, 219-220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/messages/list/MessageContainerSenderAttributionTest.kt`
around lines 71 - 72, The test assertions use hardcoded English accessibility
strings like "You said, Hello" in onNodeWithContentDescription calls, which
makes the tests fragile to locale changes and copy updates. Replace all
instances of hardcoded accessibility strings (at lines 71-72, 88-89, 99-100,
113-114, 126-127, 139-140, and 219-220) with dynamically loaded strings from
stringResource IDs. This will require fetching the actual string resources used
in the composable being tested and using those values in the assertions instead
of hardcoding them, ensuring the tests remain stable as translations and copy
text evolve.
Contributor
SDK Size Comparison 📏
|
|
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
1 participant
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.
Goal
TalkBack does not announce who sent a message, so screen reader users cannot tell the author. The author name is shown only visually, and the visual grouping that hides it on consecutive messages from the same user is a sighted-only concept (design decision DS-035). This change announces the sender on every message for screen readers.
Resolves AND-1239.
Implementation
The sender is announced once per message, on a single content leaf, independent of the visual grouping:
Outgoing messages announce "You said"; incoming messages name the sender. A shared
senderAwareContentDescriptionhelper builds the label, and an additiveAttachmentState.announceSenderflag marks the single attachment that carries the sender. The new strings are translated in all supported locales.Custom and unsupported attachments are intentionally left to the integrator, since their content is integrator-defined.
Testing
No visual changes (accessibility semantics only). Covered by unit tests in
MessageContainerSenderAttributionTest.Manual: enable TalkBack, open a channel, and focus the message rows:
Summary by CodeRabbit
Release Notes
New Features
Tests