Commit b8be9ca
authored
Scope message cell ripple to the bubble shape (#6425)
* Add ripple feedback to giphy, file, link and quoted-message taps
Image attachments already render a ripple on tap via their
combinedClickable. Giphy, file rows, link previews and the quoted-
message preview block had indication = null, leaving them without
visual feedback. Match the image-attachment pattern across all four
so every interactive surface inside a message bubble ripples
consistently on press.
* Move bubble ripple to message-content column-level clickable
Wrap the message-content Column inside DefaultMessageRegularContent
with combinedClickable + ripple(). The Column owns the click + ripple
for the entire bubble interior (text, spacer, and any space around
inner attachments). Inner attachment clickables (image, file, giphy,
link, quoted) still consume their own taps and ripple in their own
bounds.
This replaces the earlier params-based bubble ripple (which had a
position-translation issue between the cell's interaction source and
the bubble's local coords). With the click and the ripple at the
same layout node, press positions are captured in Column-local coords
and the ripple renders correctly regardless of message alignment or
bubble width.
* Plumb cell interaction source through to bubble for avatar-gap ripple
Restore the ripple feedback on the bubble when the user long-presses
in the avatar gap (or any cell area outside the bubble). The
column-level clickable inside DefaultMessageRegularContent only fires
for taps inside the bubble; cell-area presses go to MessageContainer's
combinedClickable, which has indication = null. Without forwarding the
cell's interaction source, those presses had no visual feedback.
Add interactionSource to MessageBubbleParams and MessageContentParams.
The cell hoists its MutableInteractionSource and threads it via
MessageContainer -> factory.MessageContent -> DefaultMessageContent ->
RegularMessageContent / PollMessageContent -> factory.MessageBubble.
The MessageBubble factory default applies clip(shape).indication(
source, ripple(bounded = false)) when the source is non-null,
synchronised with the cell's press state.
Two ripple paths now coexist by design:
- Tap inside the bubble: column's combinedClickable consumes,
column-level bounded ripple fires (position-aware).
- Tap in avatar gap: cell's combinedClickable handles, the cell's
source emits, bubble's params indication renders an unbounded
ripple from the bubble centre.
Both fire on the correct trigger; no double rippling thanks to
gesture consumption rules.
* Forward cell click and long-click intents to bubble Column
The Column-level combinedClickable inside DefaultMessageRegularContent
and the inner PollMessageContent consumed taps with onClick = {} and
fired a raw onLongItemClick(message) without haptic. Two regressions
followed: tapping a thread-start message inside the bubble no longer
opened the thread (the cell's onClick was shadowed), and bubble
long-press lost the haptic feedback that the cell triggered for
avatar-gap presses.
Hoist onItemClick and onItemLongClick as named lambdas in
MessageContainer, both wrapping the canOpenThread / canOpenActions
gates and the haptic call. Plumb them through MessageContentParams,
MessageRegularContentParams, the public DefaultMessageContent /
RegularMessageContent / MessageContent / PollMessageContent
composables, and the factory defaults so the bubble Column can call
them directly.
This keeps canOpenActions in a single place (MessageContainer) and
removes the LocalHapticFeedback usage from MessageContent.kt and
PollMessageContent.kt: the bubble Column no longer needs to know
about action-permission rules or haptic policy. The inner private
PollMessageContent overload also drops its now-unused
onLongItemClick: (Message) -> Unit parameter.
* Rename bubble-mirror callbacks and document why they are hoisted
The previous names (onItemClick / onItemLongClick) collided with the
pre-existing onLongItemClick: (Message) -> Unit, which still exists for
attachment routing. Reading MessageContentParams meant disambiguating
three near-identical names with different shapes and purposes. Rename
the new pair to onBubbleClick / onBubbleLongClick: the names now encode
where the gesture happens and remove the word-swap collision.
Add a one-line comment in MessageContainer near the hoisted lambdas
explaining why they are extracted (shared with the bubble Column via
MessageContentParams so in-bubble gestures mirror the cell).
* Replace bubble-mirror plumbing with non-consuming ripple modifier
Drop the onBubbleClick / onBubbleLongClick callback chain plus the
interactionSource forwarding introduced earlier in this branch. The
bubble Column now uses a single internal Modifier (passiveRipple) that
renders a bounded position-aware ripple via a non-consuming
pointerInput. The cell's combinedClickable stays as the single owner
of click and long-click logic, and inner clickable children
(attachments, quoted-reply previews) keep their own ripples — they
are filtered out at the Column level via awaitFirstDown(requireUnconsumed
= true), so they do not double-fire.
passiveRipple is named after what it does (renders a ripple on press
without claiming the gesture), not where it is used. It lives in
ui/util and is reusable beyond message bubbles.
Behaviour change: avatar-gap presses no longer render an unbounded
ripple inside the bubble (the cell-source-driven indication on
MessageBubble is removed). Cell click and long-press still fire there;
only the visual feedback in that region is dropped, matching the
WhatsApp pattern where the avatar gap is a dead zone visually.
Net public API: MessageBubbleParams loses interactionSource;
MessageContentParams and MessageRegularContentParams lose
onBubbleClick, onBubbleLongClick, and interactionSource;
DefaultMessageContent / RegularMessageContent / MessageContent /
PollMessageContent lose the same trailing params. Surface shrinks
back below the pre-attempt baseline.
* Ripple text-with-link bubbles on non-link character taps
The local ClickableText helper used detectTapGestures, which consumes
the down event on every press inside the text. With the bubble Column's
passiveRipple gating on awaitFirstDown(requireUnconsumed = true), that
consumption blocked the bubble ripple AND the cell's combinedClickable
for any tap on a message containing a link, mention, or email — even
when the touched character was plain text.
Replace detectTapGestures with a custom awaitEachGesture loop that
only consumes when the down position lands on a character carrying
an interactive annotation. Non-link character taps propagate to
ancestors normally: the bubble ripples (passiveRipple sees unconsumed
down) and the cell fires its onClick / onLongClick (thread-open,
haptic, action menu).
Tap and long-press on link characters keep their existing handlers
via the same withTimeoutOrNull + waitForUpOrCancellation flow as
detectTapGestures.
Note: this is a workaround on top of the legacy string-annotation
plumbing. The cleaner direction is migrating to Compose Foundation's
LinkAnnotation API, which handles non-link tap propagation natively
and would let us delete this entire custom detector. Tracked as a
follow-up.
* Fix off-by-one and unreachable long-press path in ClickableText
Two correctness fixes raised in PR review:
- AnnotatedString.Range.end is exclusive, but the membership checks
used ann.start..ann.end (inclusive). A tap on the character
immediately after a link/mention/email was treated as part of the
annotation. Switch both call sites to ann.start until ann.end.
- The long-press branch used kotlinx.coroutines.withTimeoutOrNull,
which returns null on timeout and never throws. The
catch (_: PointerEventTimeoutCancellationException) block was
unreachable, so onLongPress() was never invoked on long-press of a
link/mention/email. Switch to AwaitPointerEventScope.withTimeout
(the throwing variant matching Compose Foundation's own waitForLongPress)
and drop the now-unused kotlinx.coroutines.withTimeoutOrNull import.
* Extract MessageText interactive-annotation predicates and unit-test them
Pull two internal helpers out of inline lambdas in MessageText.kt:
- AnnotatedString.Range<String>.isInteractiveTag() — true for URL,
email, and mention tags.
- List<AnnotatedString.Range<String>>.hasInteractiveAt(offset) —
true when any range in the list both has an interactive tag and
covers the given offset, with exclusive-end semantics matching
AnnotatedString.Range.end.
Replaces the inline lambdas in the public MessageText composable
with member references at the call sites.
Add MessageTextTest covering the predicate matrix: every interactive
tag, non-interactive tags, empty list, inclusive-start and
exclusive-end boundaries (locks in the recent off-by-one fix), and
mixed annotation lists. Pure JUnit 5 + kluent, no Compose runtime.
* Cover passiveRipple and the MessageText click dispatch with tests
Extract the click-dispatch logic out of MessageText.ClickableText's
inline lambda into an internal `handleAnnotationClick` helper. Same
behaviour with one tightening: the original code silently fell through
to URL handling for any non-mention annotation (treating its `item` as
a URL); the helper uses an explicit `when` over the three interactive
tags (Mention, URL, Email) and ignores anything else. The bubble
predicate already restricts the click handler to interactive
positions, so the tightening only affects pathological input.
Add tests:
- PassiveRippleTest (Compose UI tests via createComposeRule +
Robolectric) covers the four reachable branches of
Modifier.passiveRipple(): tap propagates to outer combinedClickable,
long-press propagates to outer onLongClick, an inner consuming
clickable shields the parent, and drag-out-of-bounds exercises the
Cancel branch without crashing.
- MessageTextTest gains eight pure-JUnit cases for handleAnnotationClick
covering URL with and without onLinkClick, email, mention with
resolved user, mention with unknown username, position outside any
annotation, non-interactive tag, and empty annotation item.
Also drop the redundant `requireUnconsumed = true` arguments at the
two awaitFirstDown call sites — `true` is the default. The named
boolean rule applies when a non-default value is being passed.
* Tighten MessageText click dispatch and add snapshot coverage
Apply audit findings from the gesture review:
- handleAnnotationClick now filters firstOrNull by isInteractiveTag.
Previously, when a non-interactive annotation (e.g. a custom
decoration added through AnnotatedMessageTextBuilder) overlapped
a URL/email/mention range, the first-overlapping non-interactive
one was returned and the when fell through silently. The link
never opened. Locked in by a new MessageTextHelpersTest case
covering the overlap.
- ClickableText's pointerInput now keys on Unit and reads its
callbacks through rememberUpdatedState. The block is no longer
cancelled and restarted each composition because the caller
allocates fresh lambdas.
- styledText.getStringAnnotations is wrapped in remember(styledText)
so the list is not reallocated on every recomposition.
- A small WHY comment is added on the two non-obvious gesture
decisions in ClickableText (the early-return for non-interactive
characters, and consumeUntilUp after long-press).
Tests:
- The previous MessageTextTest is renamed to MessageTextHelpersTest
to free the canonical name for the new snapshot test, matching
the codebase convention (e.g. ReactionsMenuContentTest).
- MessageTextTest is the new Paparazzi snapshot suite. It covers
plain text, URL, email, mention, and URL + mention scenarios.
The fixtures live next to the production code as internal
preview-friendly composables (MessageTextPlain, MessageTextWithUrl,
...) so the @Preview composables and the snapshot tests share
the same definitions.
The earlier audit also recommended Compose UI gesture tests
(tap / long-press / drag-out). Those are not included: the
gesture loop relies on withTimeout inside awaitPointerEventScope,
which the Robolectric test environment does not drive reliably
for this case. The behaviour stays under manual QA.
* Fire haptic on long-press of link or mention characters
ClickableText's onLongPress callback used to call onLongItemClick(message)
without haptic feedback. The cell's combinedClickable.onLongClick — which
handles long-press for every other region — does fire haptic. Long-press
on a link / mention / email character was the one path where the action
menu opened silently, because the cell is shielded there (ClickableText
consumes the down).
Capture LocalHapticFeedback at the MessageText composable level and
include the same haptic call in the lambda passed to ClickableText, so
the user feels the press acknowledgement regardless of which character
they long-press.
* Centralize long-press haptic at the message cell entrance
The cell and ClickableText each fired HapticFeedbackType.LongPress
before calling the upstream onLongItemClick. Attachments
(image / file / giphy / link / quoted-reply preview) did not, so
long-press on those felt different. "Long-press anywhere in a
message cell fires haptic before the upstream handler" is one rule;
restating it at every leaf is duplication and easy to forget when
adding a new clickable.
Wrap the onLongItemClick callback once at the top of
MessageContainer via a small rememberHapticLongClick helper. The
wrapped callback is what flows down through every existing
parameter slot — cell combinedClickable, MessageText.ClickableText,
attachment combinedClickables (via AttachmentState), and the
quoted-reply preview — so every leaf calls onLongItemClick(message)
and gets the haptic transparently.
The cell's canOpenActions gate still works because the gate runs
before the wrapped call: a long-press on a deleted or uploading
message neither fires haptic nor dispatches.
Net: one helper, one wrapping line, no duplicated haptic calls in
the leaf gesture handlers. Attachments now have parity with text
and the rest of the cell. No public API change (the helper is
private and the callback contract from outside the cell is
unchanged).
* spotless
* Clip quoted-card ripple to the card's rounded shape
The quoted-message ripple was bleeding past the card's rounded corners
because the click area didn't match the visible card bounds. Two fixes
align them:
- `MessageQuotedContent` factory now applies `messageSectionPadding` as
the outer-most modifier and chains the caller's modifier after it.
Previously the padding was appended after the caller's modifier, so a
caller-supplied `combinedClickable` covered both the card and the
section padding, making the touch and ripple area larger than the
visible card.
- `DefaultMessageRegularContent` now also clips to `RoundedCornerShape`
matching the card's background, so the ripple respects the card's
rounded corners.1 parent 738b087 commit b8be9ca
18 files changed
Lines changed: 849 additions & 60 deletions
File tree
- stream-chat-android-compose
- api
- src
- main/java/io/getstream/chat/android/compose/ui
- attachments/content
- components/messages
- messages/list
- theme
- util
- test
- kotlin/io/getstream/chat/android/compose/ui
- components/messages
- util
- snapshots/images
Lines changed: 5 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1504 | 1504 | | |
1505 | 1505 | | |
1506 | 1506 | | |
1507 | | - | |
| 1507 | + | |
| 1508 | + | |
| 1509 | + | |
| 1510 | + | |
| 1511 | + | |
1508 | 1512 | | |
1509 | 1513 | | |
1510 | 1514 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| |||
111 | 112 | | |
112 | 113 | | |
113 | 114 | | |
114 | | - | |
| 115 | + | |
115 | 116 | | |
116 | 117 | | |
117 | 118 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
123 | 124 | | |
124 | 125 | | |
125 | 126 | | |
126 | | - | |
| 127 | + | |
127 | 128 | | |
128 | 129 | | |
129 | 130 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| 40 | + | |
40 | 41 | | |
41 | 42 | | |
42 | 43 | | |
| |||
111 | 112 | | |
112 | 113 | | |
113 | 114 | | |
114 | | - | |
| 115 | + | |
115 | 116 | | |
116 | 117 | | |
117 | 118 | | |
| |||
Lines changed: 16 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
| 31 | + | |
30 | 32 | | |
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
| 36 | + | |
34 | 37 | | |
35 | 38 | | |
36 | 39 | | |
| |||
61 | 64 | | |
62 | 65 | | |
63 | 66 | | |
| 67 | + | |
64 | 68 | | |
65 | 69 | | |
66 | 70 | | |
| |||
179 | 183 | | |
180 | 184 | | |
181 | 185 | | |
182 | | - | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
183 | 190 | | |
184 | 191 | | |
185 | 192 | | |
186 | 193 | | |
187 | 194 | | |
188 | 195 | | |
189 | 196 | | |
190 | | - | |
191 | | - | |
192 | | - | |
193 | | - | |
194 | | - | |
195 | | - | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
196 | 205 | | |
197 | 206 | | |
198 | 207 | | |
| |||
0 commit comments