From 1012272a0831ed9809fc73ecfffa8d160b139d39 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 18 Nov 2024 15:52:16 +0100 Subject: [PATCH] Fix long click not working for media timeline items (#3879) --- .../impl/pinned/list/PinnedMessagesListView.kt | 1 + .../timeline/components/ATimelineItemEventRow.kt | 2 +- .../timeline/components/TimelineItemEventRow.kt | 16 +++++++--------- .../components/TimelineItemGroupedEventsRow.kt | 6 ++++-- .../impl/timeline/components/TimelineItemRow.kt | 3 ++- .../components/TimelineItemStateEventRow.kt | 3 ++- .../event/TimelineItemEventContentView.kt | 7 +++++-- .../components/event/TimelineItemImageView.kt | 11 ++++++++--- .../components/event/TimelineItemLocationView.kt | 5 +---- .../components/event/TimelineItemStickerView.kt | 10 +++++++--- .../components/event/TimelineItemVideoView.kt | 11 ++++++++--- 11 files changed, 46 insertions(+), 29 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListView.kt index 7ae88b9647..35a0a1e893 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListView.kt @@ -265,6 +265,7 @@ private fun TimelineItemEventContentViewWrapper( eventSink = { }, modifier = modifier, onContentClick = onContentClick, + onLongClick = null, onContentLayoutChange = onContentLayoutChange ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/ATimelineItemEventRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/ATimelineItemEventRow.kt index 85e9854919..02ef734ccc 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/ATimelineItemEventRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/ATimelineItemEventRow.kt @@ -30,7 +30,7 @@ internal fun ATimelineItemEventRow( timelineProtectionState = timelineProtectionState, isLastOutgoingMessage = isLastOutgoingMessage, isHighlighted = isHighlighted, - onContentClick = {}, + onEventClick = {}, onLongClick = {}, onLinkClick = {}, onUserDataClick = {}, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt index 0061ebe6df..c358dcdd0a 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt @@ -114,7 +114,7 @@ fun TimelineItemEventRow( renderReadReceipts: Boolean, isLastOutgoingMessage: Boolean, isHighlighted: Boolean, - onContentClick: () -> Unit, + onEventClick: () -> Unit, onLongClick: () -> Unit, onLinkClick: (String) -> Unit, onUserDataClick: (UserId) -> Unit, @@ -127,10 +127,14 @@ fun TimelineItemEventRow( eventSink: (TimelineEvents.EventFromTimelineItem) -> Unit, modifier: Modifier = Modifier, eventContentView: @Composable (Modifier, (ContentAvoidingLayoutData) -> Unit) -> Unit = { contentModifier, onContentLayoutChange -> + // Only pass down a custom clickable lambda if the content can be clicked separately + val onContentClick = onEventClick.takeUnless { event.isWholeContentClickable } + TimelineItemEventContentView( content = event.content, hideMediaContent = timelineProtectionState.hideMediaContent(event.eventId), onContentClick = onContentClick, + onLongClick = onLongClick, onShowContentClick = { timelineProtectionState.eventSink(TimelineProtectionEvent.ShowContent(event.eventId)) }, onLinkClick = onLinkClick, eventSink = eventSink, @@ -151,12 +155,6 @@ fun TimelineItemEventRow( inReplyToClick(inReplyToEventId) } - val onWholeItemClick = if (event.isWholeContentClickable) { - onContentClick - } else { - {} - } - Column(modifier = modifier.fillMaxWidth()) { if (event.groupPosition.isNew()) { Spacer(modifier = Modifier.height(16.dp)) @@ -180,7 +178,7 @@ fun TimelineItemEventRow( isHighlighted = isHighlighted, timelineRoomInfo = timelineRoomInfo, interactionSource = interactionSource, - onContentClick = onWholeItemClick, + onContentClick = onEventClick, onLongClick = onLongClick, inReplyToClick = ::inReplyToClick, onUserDataClick = ::onUserDataClick, @@ -214,7 +212,7 @@ fun TimelineItemEventRow( isHighlighted = isHighlighted, timelineRoomInfo = timelineRoomInfo, interactionSource = interactionSource, - onContentClick = onWholeItemClick, + onContentClick = onEventClick, onLongClick = onLongClick, inReplyToClick = ::inReplyToClick, onUserDataClick = ::onUserDataClick, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt index fe89d6de02..d65c9186aa 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt @@ -61,7 +61,8 @@ fun TimelineItemGroupedEventsRow( onLinkClick = onLinkClick, eventSink = eventSink, modifier = contentModifier, - onContentClick = {}, + onContentClick = null, + onLongClick = null, onContentLayoutChange = onContentLayoutChange ) }, @@ -126,7 +127,8 @@ private fun TimelineItemGroupedEventsRowContent( onLinkClick = onLinkClick, eventSink = eventSink, modifier = contentModifier, - onContentClick = {}, + onContentClick = null, + onLongClick = null, onContentLayoutChange = onContentLayoutChange ) }, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemRow.kt index 047f9b3d04..859c9f3ccf 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemRow.kt @@ -61,6 +61,7 @@ internal fun TimelineItemRow( hideMediaContent = timelineProtectionState.hideMediaContent(event.eventId), onShowContentClick = { timelineProtectionState.eventSink(TimelineProtectionEvent.ShowContent(event.eventId)) }, onContentClick = { onContentClick(event) }, + onLongClick = { onLongClick(event) }, onLinkClick = onLinkClick, eventSink = eventSink, modifier = contentModifier, @@ -118,7 +119,7 @@ internal fun TimelineItemRow( timelineProtectionState = timelineProtectionState, isLastOutgoingMessage = isLastOutgoingMessage, isHighlighted = timelineItem.isEvent(focusedEventId), - onContentClick = { onContentClick(timelineItem) }, + onEventClick = { onContentClick(timelineItem) }, onLongClick = { onLongClick(timelineItem) }, onLinkClick = onLinkClick, onUserDataClick = onUserDataClick, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemStateEventRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemStateEventRow.kt index 6172686016..d523152977 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemStateEventRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemStateEventRow.kt @@ -74,7 +74,8 @@ fun TimelineItemStateEventRow( hideMediaContent = false, onShowContentClick = {}, eventSink = eventSink, - onContentClick = {}, + onContentClick = null, + onLongClick = null, modifier = Modifier.defaultTimelineContentPadding() ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemEventContentView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemEventContentView.kt index 3a3a43a6fc..35a8cda293 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemEventContentView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemEventContentView.kt @@ -36,7 +36,8 @@ import io.element.android.libraries.architecture.Presenter fun TimelineItemEventContentView( content: TimelineItemEventContent, hideMediaContent: Boolean, - onContentClick: () -> Unit, + onContentClick: (() -> Unit)?, + onLongClick: (() -> Unit)?, onShowContentClick: () -> Unit, onLinkClick: (url: String) -> Unit, eventSink: (TimelineEvents.EventFromTimelineItem) -> Unit, @@ -68,13 +69,13 @@ fun TimelineItemEventContentView( ) is TimelineItemLocationContent -> TimelineItemLocationView( content = content, - onContentClick = onContentClick, modifier = modifier ) is TimelineItemImageContent -> TimelineItemImageView( content = content, hideMediaContent = hideMediaContent, onContentClick = onContentClick, + onLongClick = onLongClick, onShowContentClick = onShowContentClick, onLinkClick = onLinkClick, onContentLayoutChange = onContentLayoutChange, @@ -84,6 +85,7 @@ fun TimelineItemEventContentView( content = content, hideMediaContent = hideMediaContent, onContentClick = onContentClick, + onLongClick = onLongClick, onShowClick = onShowContentClick, modifier = modifier, ) @@ -91,6 +93,7 @@ fun TimelineItemEventContentView( content = content, hideMediaContent = hideMediaContent, onContentClick = onContentClick, + onLongClick = onLongClick, onShowContentClick = onShowContentClick, onLinkClick = onLinkClick, onContentLayoutChange = onContentLayoutChange, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt index 123ba195f1..84b7026142 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt @@ -8,8 +8,9 @@ package io.element.android.features.messages.impl.timeline.components.event import android.text.SpannedString +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background -import androidx.compose.foundation.clickable +import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth @@ -55,11 +56,13 @@ import io.element.android.libraries.textcomposer.ElementRichTextEditorStyle import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.wysiwyg.compose.EditorStyledText +@OptIn(ExperimentalFoundationApi::class) @Composable fun TimelineItemImageView( content: TimelineItemImageContent, hideMediaContent: Boolean, - onContentClick: () -> Unit, + onContentClick: (() -> Unit)?, + onLongClick: (() -> Unit)?, onLinkClick: (String) -> Unit, onShowContentClick: () -> Unit, onContentLayoutChange: (ContentAvoidingLayoutData) -> Unit, @@ -87,7 +90,7 @@ fun TimelineItemImageView( modifier = Modifier .fillMaxWidth() .then(if (isLoaded) Modifier.background(Color.White) else Modifier) - .clickable(onClick = onContentClick), + .then(if (onContentClick != null) Modifier.combinedClickable(onClick = onContentClick, onLongClick = onLongClick) else Modifier), model = content.thumbnailMediaRequestData, contentScale = ContentScale.Fit, alignment = Alignment.Center, @@ -132,6 +135,7 @@ internal fun TimelineItemImageViewPreview(@PreviewParameter(TimelineItemImageCon hideMediaContent = false, onShowContentClick = {}, onContentClick = {}, + onLongClick = {}, onLinkClick = {}, onContentLayoutChange = {}, ) @@ -145,6 +149,7 @@ internal fun TimelineItemImageViewHideMediaContentPreview() = ElementPreview { hideMediaContent = true, onShowContentClick = {}, onContentClick = {}, + onLongClick = {}, onLinkClick = {}, onContentLayoutChange = {}, ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemLocationView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemLocationView.kt index 5d402bc8f1..964f3ecb18 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemLocationView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemLocationView.kt @@ -7,7 +7,6 @@ package io.element.android.features.messages.impl.timeline.components.event -import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn @@ -26,10 +25,9 @@ import io.element.android.libraries.designsystem.theme.components.Text @Composable fun TimelineItemLocationView( content: TimelineItemLocationContent, - onContentClick: () -> Unit, modifier: Modifier = Modifier, ) { - Column(modifier = modifier.clickable(onClick = onContentClick).fillMaxWidth()) { + Column(modifier = modifier.fillMaxWidth()) { content.description?.let { Text( text = it, @@ -55,6 +53,5 @@ internal fun TimelineItemLocationViewPreview(@PreviewParameter(TimelineItemLocat ElementPreview { TimelineItemLocationView( content = content, - onContentClick = {}, ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemStickerView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemStickerView.kt index df087769ed..9037d0dffa 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemStickerView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemStickerView.kt @@ -7,8 +7,9 @@ package io.element.android.features.messages.impl.timeline.components.event +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background -import androidx.compose.foundation.clickable +import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.runtime.Composable @@ -37,11 +38,13 @@ import io.element.android.libraries.ui.strings.CommonStrings private const val STICKER_SIZE_IN_DP = 128 +@OptIn(ExperimentalFoundationApi::class) @Composable fun TimelineItemStickerView( content: TimelineItemStickerContent, hideMediaContent: Boolean, - onContentClick: () -> Unit, + onContentClick: (() -> Unit)?, + onLongClick: (() -> Unit)?, onShowClick: () -> Unit, modifier: Modifier = Modifier, ) { @@ -64,7 +67,7 @@ fun TimelineItemStickerView( modifier = Modifier .fillMaxSize() .then(if (isLoaded) Modifier.background(Color.White) else Modifier) - .clickable(onClick = onContentClick), + .then(if (onContentClick != null) Modifier.combinedClickable(onClick = onContentClick, onLongClick = onLongClick) else Modifier), model = MediaRequestData( source = content.preferredMediaSource, kind = MediaRequestData.Kind.File( @@ -89,6 +92,7 @@ internal fun TimelineItemStickerViewPreview(@PreviewParameter(TimelineItemSticke content = content, hideMediaContent = false, onContentClick = {}, + onLongClick = {}, onShowClick = {}, ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt index 65f59cd631..afd45b171a 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt @@ -8,9 +8,10 @@ package io.element.android.features.messages.impl.timeline.components.event import android.text.SpannedString +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.Image import androidx.compose.foundation.background -import androidx.compose.foundation.clickable +import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer @@ -64,11 +65,13 @@ import io.element.android.libraries.textcomposer.ElementRichTextEditorStyle import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.wysiwyg.compose.EditorStyledText +@OptIn(ExperimentalFoundationApi::class) @Composable fun TimelineItemVideoView( content: TimelineItemVideoContent, hideMediaContent: Boolean, - onContentClick: () -> Unit, + onContentClick: (() -> Unit)?, + onLongClick: (() -> Unit)?, onShowContentClick: () -> Unit, onLinkClick: (String) -> Unit, onContentLayoutChange: (ContentAvoidingLayoutData) -> Unit, @@ -99,7 +102,7 @@ fun TimelineItemVideoView( modifier = Modifier .fillMaxWidth() .then(if (isLoaded) Modifier.background(Color.White) else Modifier) - .clickable(onClick = onContentClick), + .then(if (onContentClick != null) Modifier.combinedClickable(onClick = onContentClick, onLongClick = onLongClick) else Modifier), model = MediaRequestData( source = content.thumbnailSource, kind = MediaRequestData.Kind.Thumbnail( @@ -161,6 +164,7 @@ internal fun TimelineItemVideoViewPreview(@PreviewParameter(TimelineItemVideoCon hideMediaContent = false, onShowContentClick = {}, onContentClick = {}, + onLongClick = {}, onLinkClick = {}, onContentLayoutChange = {}, ) @@ -174,6 +178,7 @@ internal fun TimelineItemVideoViewHideMediaContentPreview() = ElementPreview { hideMediaContent = true, onShowContentClick = {}, onContentClick = {}, + onLongClick = {}, onLinkClick = {}, onContentLayoutChange = {}, )