Fix long click not working for media timeline items (#3879)

element-hq · Nov 18, 2024 · 2405542 · 2405542
1 parent 7222a1f
commit 2405542
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 29 deletions.
diff --git a/...in/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListView.kt b/...in/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/...in/io/element/android/features/messages/impl/timeline/components/ATimelineItemEventRow.kt b/...in/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/...lin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt b/...lin/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/...lement/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt b/...lement/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/...n/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemRow.kt b/...n/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/...o/element/android/features/messages/impl/timeline/components/TimelineItemStateEventRow.kt b/...o/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/.../android/features/messages/impl/timeline/components/event/TimelineItemEventContentView.kt b/.../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,13 +85,15 @@ fun TimelineItemEventContentView(
             content = content,
             hideMediaContent = hideMediaContent,
             onContentClick = onContentClick,
+            onLongClick = onLongClick,
             onShowClick = onShowContentClick,
             modifier = modifier,
         )
         is TimelineItemVideoContent -> TimelineItemVideoView(
             content = content,
             hideMediaContent = hideMediaContent,
             onContentClick = onContentClick,
+            onLongClick = onLongClick,
             onShowContentClick = onShowContentClick,
             onLinkClick = onLinkClick,
             onContentLayoutChange = onContentLayoutChange,

diff --git a/...element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt b/...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/...ment/android/features/messages/impl/timeline/components/event/TimelineItemLocationView.kt b/...ment/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/...ement/android/features/messages/impl/timeline/components/event/TimelineItemStickerView.kt b/...ement/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/...element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt b/...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 = {},
     )