Improve message list TalkBack accessibility#6488
Open
andremion wants to merge 6 commits into
Open
Conversation
Contributor
Author
|
@CodeRabbit review |
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughThis PR enhances accessibility in the Compose UI by expanding quoted message reply context naming and refactoring message container interaction modifiers. Quoted message accessibility now selects localized strings based on reply ownership, supported by new localized string resources across ten languages. Message container interactions are refactored to separate gesture handling for thread vs. action scenarios with improved accessibility semantics. ChangesCompose Accessibility Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
Contributor
SDK Size Comparison 📏
|
andremion
force-pushed
the
fix/compose-message-list-a11y
branch
from
a3142a9 to
fe87078
Compare
auto-merge was automatically disabled
June 3, 2026 16:36
Pull request was converted to draft
When TalkBack focused on the quoted preview inside a reply, the bubble
announced only the quoted message's sender and text. Screen-reader
users had no signal about who replied to whom — they had to swipe to
the reply row to piece the relationship together.
Extend the `accessibilityName` override in `QuotedMessageUserName` to
handle the in-list case (`replyMessage != null`). The quoted bubble's
sender label now reads as one of:
- `"You replied to your message"` (both the replier and the quoted
message are the current user — e.g. a self-thread)
- `"You replied to {name}'s message"` (current user replied to someone
else)
- `"{name} replied to your message"` (someone else replied to the
current user)
- `"{name} replied to {other name}'s message"` (someone else replied
to a third party's message)
TalkBack then reads the bubble as "{relationship}, {quoted text},
double-tap to activate" — context first, content second.
Two new strings (`stream_compose_quoted_message_replied_to_your_message`
and `stream_compose_quoted_message_replied_to_their_message`) translated
across the 7 supported locales. The composer-banner case
(`replyMessage == null`) is unchanged.
The row's `combinedClickable` was always enabled when either
`canOpenThread` or `canOpenActions` was true, with `onClick` guarded to
fire `onThreadClick` only when a thread existed. For regular messages
(no thread started yet, common case) `onClick` was a no-op but the
`combinedClickable` still exposed a click action to the accessibility
tree, so TalkBack announced "double-tap to activate" — promising an
action that didn't fire. The long-press path worked fine.
Refactor the modifier into three branches by row capability, so the
accessibility tree only carries the actions the row actually performs:
- `canOpenThread`: keep `combinedClickable` with both `onClick` (open
thread) and `onLongClick` (open actions, if available).
- `canOpenActions` only: use `Modifier.pointerInput { detectTapGestures
(onLongPress = ...) }` plus `Modifier.semantics(mergeDescendants =
true) { onLongClick(label) }` — long-press still opens the menu,
TalkBack reads only "double-tap and hold to show message options",
and descendants merge into a single row focus.
- Neither (deleted, uploading): empty `Modifier.semantics
(mergeDescendants = true) {}` so the row stays a single focus
target for TalkBack with no interaction hints.
Trade-off: the `canOpenActions`-only case loses the press-down ripple
that `combinedClickable` applies through its default `indication`.
Sighted users still get feedback from the actions menu opening; SR
users get a TalkBack hint that matches what tapping actually does. The
ripple loss is the smaller of the two regressions to keep.
Public API unchanged; the modifier is a local val in `MessageContainer`.
The new `accessibilityName` logic in `QuotedMessageUserName` only fires
when `replyMessage != null`, but every existing snapshot test
constructs the composable with `replyMessage = null`. Sonar reported 6
uncovered lines and 2 uncovered conditions on the file as a result.
Add two preview composables (`QuotedMessageReplyByMeToOther`,
`QuotedMessageReplyByOtherToMe`) plus matching Paparazzi tests. The
pair is chosen so each new conditional branch is hit in both
directions:
- Case A (me replies to other): `replyMessage.isMine = true`,
`message.isMine = false` -> "You replied to {other}'s message".
- Case B (other replies to me): `replyMessage.isMine = false`,
`message.isMine = true` -> "{other} replied to your message".
The file crossed the detekt `TooManyFunctions` threshold (21/20) once
the two preview wrappers were added, so apply the existing project
pattern of `@file:Suppress("TooManyFunctions")` (already used on
`ChannelItem`, `MessageInput`, etc.).
CI's clean-cache `apiCheck` picked up two new `ComposableSingletons$QuotedMessageKt` lambda accessors emitted for the `@Preview` wrappers added in the previous commit. Local `apiCheck` had served a cached `apiBuild` output and missed the diff.
The previous commit keyed the gesture detector on `message.id`:
Modifier.pointerInput(message.id) {
detectTapGestures(onLongPress = { onLongItemClick(message) })
}
`pointerInput` re-launches its block only when its key changes, so for
the lifetime of a message row (id stable) the captured `message` and
`onLongItemClick` references stay frozen to the values from the first
composition. When the message updates (for example, after the user
adds a reaction), the long-press still fires with the pre-reaction
message — the actions menu opens against stale state, and the menu's
reaction-toggle decides "add" instead of "remove" because the captured
message has no reaction yet. The user-driven add->remove E2E flows
(`test_deletesReaction`, `test_removesReaction_whenUnReactingToParticipantsMessage`)
broke for that reason.
`combinedClickable` (used in the `canOpenThread` branch) does not have
this problem because its handler lambdas are reinstalled on every
recomposition.
Capture the message and the long-click handler with
`rememberUpdatedState` and key `pointerInput` on `Unit`. The gesture
detector survives recomposition, but every fire reads the current
values, so the menu always opens against the latest message state.
andremion
force-pushed
the
fix/compose-message-list-a11y
branch
from
fe87078 to
4cb4e92
Compare
|
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.
AND-1180
Goal
Two TalkBack issues on the message list, bundled because they share a screen and a verification path.
combinedClickableexposes a click action to the accessibility tree even thoughonClickis guarded to a no-op. TalkBack promises an action that doesn't fire.Implementation
Quoted reply context (
QuotedMessage.kt)Extend the existing
accessibilityNameoverride onQuotedMessageUserNameto cover the in-list case (replyMessage != null). The quoted bubble's sender label now reads as one of:"You replied to your message"— replier and quoted are both the current user (self-thread)."You replied to {name}'s message"— current user replied to someone else."{name} replied to your message"— someone else replied to the current user."{name} replied to {other name}'s message"— someone else replied to a third party.TalkBack then reads the bubble as
"{relationship}, {quoted text}, double-tap to activate"— context first, content second. The composer-banner case (replyMessage == null) is unchanged. Two new string keys (stream_compose_quoted_message_replied_to_your_message,stream_compose_quoted_message_replied_to_their_message) translated across the 7 supported locales.Row click hint (
MessageContainer.kt)Split the row's modifier into three branches by capability so the accessibility tree only carries the actions the row actually performs:
canOpenThread: keepcombinedClickablewith bothonClick(open thread) andonLongClick(open actions, when available).canOpenActionsonly:Modifier.pointerInput { detectTapGestures(onLongPress = …) }+Modifier.semantics(mergeDescendants = true) { onLongClick(label) }. Long-press still opens the actions menu; TalkBack reads only"double-tap and hold to show message options"; descendants merge into a single row focus.Modifier.semantics(mergeDescendants = true) {}— the row stays a single focus target with no interaction hints.Trade-off: the
canOpenActions-only branch loses the press-down ripple thatcombinedClickableapplied through its defaultindication. Sighted users still get feedback from the actions menu opening; the ripple loss is the smaller of the two regressions to keep.Public API unchanged. The modifier is a local
valinMessageContainer.Testing
Enable TalkBack on the device and open a channel in the Compose sample.
Quoted reply announcement
"{A} replied to your message, {quoted text}, double-tap to activate"."You replied to your message, {quoted text}, …"."You replied to {B}'s message, {quoted text}, …"."{A} replied to {B}'s message, {quoted text}, …".Row click hint
"double-tap and hold to show message options". No"double-tap to activate"hint."double-tap to open thread"and the long-press hint.Localization smoke test
Switch the device language to one of the supported locales (es, fr, it, ja, ko, hi, in) and repeat the four quoted-reply cases. Verify the translated strings render with the names substituted in the correct positions.
Summary by CodeRabbit
New Features
Localization