Skip to content

Commit 9b0fd36

Browse files
committed
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.
1 parent 9cc74c0 commit 9b0fd36

9 files changed

Lines changed: 460 additions & 297 deletions

stream-chat-android-compose/api/stream-chat-android-compose.api

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1504,7 +1504,11 @@ public final class io/getstream/chat/android/compose/ui/components/messages/Comp
15041504
public final class io/getstream/chat/android/compose/ui/components/messages/ComposableSingletons$MessageTextKt {
15051505
public static final field INSTANCE Lio/getstream/chat/android/compose/ui/components/messages/ComposableSingletons$MessageTextKt;
15061506
public fun <init> ()V
1507-
public final fun getLambda$-1569101361$stream_chat_android_compose_release ()Lkotlin/jvm/functions/Function2;
1507+
public final fun getLambda$-261803629$stream_chat_android_compose_release ()Lkotlin/jvm/functions/Function2;
1508+
public final fun getLambda$-399958751$stream_chat_android_compose_release ()Lkotlin/jvm/functions/Function2;
1509+
public final fun getLambda$1734337819$stream_chat_android_compose_release ()Lkotlin/jvm/functions/Function2;
1510+
public final fun getLambda$1973807821$stream_chat_android_compose_release ()Lkotlin/jvm/functions/Function2;
1511+
public final fun getLambda$552887520$stream_chat_android_compose_release ()Lkotlin/jvm/functions/Function2;
15081512
}
15091513

15101514
public final class io/getstream/chat/android/compose/ui/components/messages/ComposableSingletons$PollMessageContentKt {

stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageText.kt

Lines changed: 100 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import androidx.compose.runtime.Composable
2727
import androidx.compose.runtime.getValue
2828
import androidx.compose.runtime.mutableStateOf
2929
import androidx.compose.runtime.remember
30+
import androidx.compose.runtime.rememberUpdatedState
3031
import androidx.compose.ui.Modifier
3132
import androidx.compose.ui.draw.clipToBounds
3233
import androidx.compose.ui.input.pointer.AwaitPointerEventScope
@@ -103,7 +104,9 @@ public fun MessageText(
103104
else -> MessageStyling.textStyle(outgoing = message.isMine(currentUser))
104105
}
105106

106-
val annotations = styledText.getStringAnnotations(0, styledText.lastIndex)
107+
val annotations = remember(styledText) {
108+
styledText.getStringAnnotations(0, styledText.lastIndex)
109+
}
107110
if (annotations.fastAny(AnnotatedString.Range<String>::isInteractiveTag)) {
108111
ClickableText(
109112
modifier = modifier
@@ -143,13 +146,6 @@ public fun MessageText(
143146
* Non-interactive presses are left untouched so the surrounding bubble can render its passive
144147
* ripple and the cell can still fire its click / long-press handler.
145148
*
146-
* Follow-up: migrate to Compose Foundation's `LinkAnnotation` API (`AnnotatedString.Builder.addLink`
147-
* with `LinkAnnotation.Url` / `LinkAnnotation.Clickable`). Native handling propagates non-link
148-
* taps to the parent for free, removing the need for this custom gesture detector and the
149-
* `isInteractiveAt` plumbing. Requires reworking `TextUtils.linkify` / `tagUser` to emit link
150-
* annotations instead of legacy string annotations and updating `MessageTextFormatter` to expose
151-
* a `LinkInteractionListener` hook for click routing.
152-
*
153149
* @param onLongPress Handler called on long press of an interactive character.
154150
* @param isInteractiveAt Returns whether the given character offset has an interactive annotation
155151
* (link, mention, email). When `false`, the gesture is not consumed and propagates to ancestors.
@@ -171,12 +167,20 @@ private fun ClickableText(
171167
onClick: (Int) -> Unit,
172168
) {
173169
val layoutResult = remember { mutableStateOf<TextLayoutResult?>(null) }
174-
val pressIndicator = Modifier.pointerInput(onClick, onLongPress, isInteractiveAt) {
170+
// Capture callbacks behind stable references so the pointerInput block does not restart on
171+
// recomposition — the lambdas allocated by the caller change identity each composition.
172+
val currentOnClick by rememberUpdatedState(onClick)
173+
val currentOnLongPress by rememberUpdatedState(onLongPress)
174+
val currentIsInteractiveAt by rememberUpdatedState(isInteractiveAt)
175+
val pressIndicator = Modifier.pointerInput(Unit) {
175176
awaitEachGesture {
176177
val down = awaitFirstDown()
177178
val layout = layoutResult.value ?: return@awaitEachGesture
178179
val charAt = layout.getOffsetForPosition(down.position)
179-
if (!isInteractiveAt(charAt)) {
180+
if (!currentIsInteractiveAt(charAt)) {
181+
// Non-interactive character: do not consume the down. Outer modifiers (the
182+
// bubble Column's passiveRipple and the surrounding cell's combinedClickable)
183+
// pick up the gesture instead.
180184
return@awaitEachGesture
181185
}
182186
down.consume()
@@ -185,13 +189,16 @@ private fun ClickableText(
185189
waitForUpOrCancellation()
186190
}
187191
} catch (_: PointerEventTimeoutCancellationException) {
188-
onLongPress()
192+
// Long-press fired. Consume the rest of the gesture so the inner click that would
193+
// normally ride the up event after onLongPress (matching detectTapGestures'
194+
// semantics) cannot reach this scope's onClick.
195+
currentOnLongPress()
189196
consumeUntilUp()
190197
return@awaitEachGesture
191198
}
192199
if (up != null) {
193200
up.consume()
194-
onClick(charAt)
201+
currentOnClick(charAt)
195202
}
196203
}
197204
}
@@ -247,7 +254,9 @@ internal fun handleAnnotationClick(
247254
onUserMentionClick: (User) -> Unit,
248255
fallback: (String) -> Unit,
249256
) {
250-
val annotation = annotations.firstOrNull { position in it.start until it.end } ?: return
257+
val annotation = annotations.firstOrNull {
258+
it.isInteractiveTag() && position in it.start until it.end
259+
} ?: return
251260
when (annotation.tag) {
252261
AnnotationTagMention -> {
253262
message.mentionedUsers.getUserByNameOrId(annotation.item)?.let(onUserMentionClick)
@@ -261,14 +270,83 @@ internal fun handleAnnotationClick(
261270
}
262271
}
263272

264-
@Preview
265273
@Composable
266-
private fun MessageTextPreview() {
267-
ChatTheme {
268-
MessageText(
269-
message = Message(text = "Hello World!"),
270-
currentUser = null,
271-
onLongItemClick = {},
272-
)
273-
}
274+
internal fun MessageTextPlain() {
275+
MessageText(
276+
message = Message(text = "Hello, this is a plain message."),
277+
currentUser = null,
278+
onLongItemClick = {},
279+
)
280+
}
281+
282+
@Composable
283+
internal fun MessageTextWithUrl() {
284+
MessageText(
285+
message = Message(text = "Check out https://getstream.io for more details."),
286+
currentUser = null,
287+
onLongItemClick = {},
288+
)
289+
}
290+
291+
@Composable
292+
internal fun MessageTextWithEmail() {
293+
MessageText(
294+
message = Message(text = "Contact us at support@getstream.io anytime."),
295+
currentUser = null,
296+
onLongItemClick = {},
297+
)
298+
}
299+
300+
@Composable
301+
internal fun MessageTextWithMention() {
302+
MessageText(
303+
message = Message(
304+
text = "Welcome @alice to the channel!",
305+
mentionedUsers = listOf(User(id = "alice", name = "alice")),
306+
),
307+
currentUser = null,
308+
onLongItemClick = {},
309+
)
310+
}
311+
312+
@Composable
313+
internal fun MessageTextWithUrlAndMention() {
314+
MessageText(
315+
message = Message(
316+
text = "Hey @alice, the docs are at https://getstream.io/docs",
317+
mentionedUsers = listOf(User(id = "alice", name = "alice")),
318+
),
319+
currentUser = null,
320+
onLongItemClick = {},
321+
)
322+
}
323+
324+
@Preview(showBackground = true)
325+
@Composable
326+
private fun MessageTextPlainPreview() {
327+
ChatTheme { MessageTextPlain() }
328+
}
329+
330+
@Preview(showBackground = true)
331+
@Composable
332+
private fun MessageTextWithUrlPreview() {
333+
ChatTheme { MessageTextWithUrl() }
334+
}
335+
336+
@Preview(showBackground = true)
337+
@Composable
338+
private fun MessageTextWithEmailPreview() {
339+
ChatTheme { MessageTextWithEmail() }
340+
}
341+
342+
@Preview(showBackground = true)
343+
@Composable
344+
private fun MessageTextWithMentionPreview() {
345+
ChatTheme { MessageTextWithMention() }
346+
}
347+
348+
@Preview(showBackground = true)
349+
@Composable
350+
private fun MessageTextWithUrlAndMentionPreview() {
351+
ChatTheme { MessageTextWithUrlAndMention() }
274352
}

0 commit comments

Comments
 (0)