From 2a29c67b965519b3c575690157261e601652f000 Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Wed, 23 Aug 2023 17:46:55 +0200 Subject: [PATCH] Improve timestamp rendering for poll event content --- ...melineItemEventForTimestampViewProvider.kt | 1 + .../components/TimelineItemEventRow.kt | 167 ++++++++++-------- .../timeline/components/TimestampPosition.kt | 34 ++++ 3 files changed, 132 insertions(+), 70 deletions(-) create mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimestampPosition.kt diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventForTimestampViewProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventForTimestampViewProvider.kt index 7697ccf4a4..c52e49c512 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventForTimestampViewProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventForTimestampViewProvider.kt @@ -35,5 +35,6 @@ class TimelineItemEventForTimestampViewProvider : PreviewParameterProvider Unit, modifier: Modifier = Modifier ) { - val isMediaItem = event.content is TimelineItemImageContent - || event.content is TimelineItemVideoContent - || event.content is TimelineItemLocationContent + val timestampPosition = when (event.content) { + is TimelineItemImageContent, + is TimelineItemVideoContent, + is TimelineItemLocationContent -> TimestampPosition.Above + is TimelineItemPollContent -> TimestampPosition.Below + else -> TimestampPosition.Aligned + } val replyToDetails = event.inReplyTo as? InReplyTo.Ready // Long clicks are not not automatically propagated from a `clickable` @@ -384,96 +390,97 @@ private fun MessageEventBubbleContent( @Composable fun ContentAndTimestampView( - overlayTimestamp: Boolean, + timestampPosition: TimestampPosition, modifier: Modifier = Modifier, contentModifier: Modifier = Modifier, timestampModifier: Modifier = Modifier, ) { - if (overlayTimestamp) { - Box(modifier) { - ContentView(modifier = contentModifier) - TimelineEventTimestampView( - event = event, - onClick = onTimestampClicked, - onLongClick = ::onTimestampLongClick, - modifier = timestampModifier - .padding(horizontal = 4.dp, vertical = 4.dp) // Outer padding - .background(ElementTheme.colors.bgSubtleSecondary, RoundedCornerShape(10.0.dp)) - .align(Alignment.BottomEnd) - .padding(horizontal = 4.dp, vertical = 2.dp) // Inner padding - ) - } - } else { - Box(modifier) { - ContentView(modifier = contentModifier) - TimelineEventTimestampView( - event = event, - onClick = onTimestampClicked, - onLongClick = ::onTimestampLongClick, - modifier = timestampModifier - .align(Alignment.BottomEnd) - .padding(horizontal = 8.dp, vertical = 4.dp) - ) - } + when (timestampPosition) { + TimestampPosition.Above -> + Box(modifier) { + ContentView(modifier = contentModifier) + TimelineEventTimestampView( + event = event, + onClick = onTimestampClicked, + onLongClick = ::onTimestampLongClick, + modifier = timestampModifier + .padding(horizontal = 4.dp, vertical = 4.dp) // Outer padding + .background(ElementTheme.colors.bgSubtleSecondary, RoundedCornerShape(10.0.dp)) + .align(Alignment.BottomEnd) + .padding(horizontal = 4.dp, vertical = 2.dp) // Inner padding + ) + } + TimestampPosition.Aligned -> + Box(modifier) { + ContentView(modifier = contentModifier) + TimelineEventTimestampView( + event = event, + onClick = onTimestampClicked, + onLongClick = ::onTimestampLongClick, + modifier = timestampModifier + .align(Alignment.BottomEnd) + .padding(horizontal = 8.dp, vertical = 4.dp) + ) + } + TimestampPosition.Below -> + Column(modifier) { + ContentView(modifier = contentModifier) + TimelineEventTimestampView( + event = event, + onClick = onTimestampClicked, + onLongClick = ::onTimestampLongClick, + modifier = timestampModifier + .align(Alignment.End) + .padding(horizontal = 8.dp, vertical = 4.dp) + ) + } } } - /** Used only for media items, with no reply to metadata. It displays the contents with no paddings. */ - @Composable - fun SimpleMediaItemLayout(modifier: Modifier = Modifier) { - ContentAndTimestampView(overlayTimestamp = true, modifier = modifier) - } - - /** Used for every other type of message, groups the different components in a Column with some space between them. */ + /** Groups the different components in a Column with some space between them. */ @Composable fun CommonLayout( inReplyToDetails: InReplyTo.Ready?, modifier: Modifier = Modifier ) { + var modifierWithPadding: Modifier = Modifier + var contentModifier: Modifier = Modifier EqualWidthColumn(modifier = modifier, spacing = 8.dp) { - if (inReplyToDetails != null) { - val senderName = inReplyToDetails.senderDisplayName ?: inReplyToDetails.senderId.value - val attachmentThumbnailInfo = attachmentThumbnailInfoForInReplyTo(inReplyToDetails) - val text = textForInReplyTo(inReplyToDetails) - ReplyToContent( - senderName = senderName, - text = text, - attachmentThumbnailInfo = attachmentThumbnailInfo, - modifier = Modifier - .padding(top = 8.dp, start = 8.dp, end = 8.dp) - .clip(RoundedCornerShape(6.dp)) - .clickable(enabled = true, onClick = inReplyToClick), - ) - } - val modifierWithPadding = if (isMediaItem) { - Modifier.padding(start = 8.dp, end = 8.dp, bottom = 8.dp) - } else { - Modifier - } - - val contentModifier = if (isMediaItem) { - Modifier.clip(RoundedCornerShape(12.dp)) - } else { - if (inReplyToDetails != null) { - Modifier.padding(start = 12.dp, end = 12.dp, top = 0.dp, bottom = 8.dp) - } else { - Modifier.padding(start = 12.dp, end = 12.dp, top = 8.dp, bottom = 8.dp) + when { + inReplyToDetails != null -> { + val senderName = inReplyToDetails.senderDisplayName ?: inReplyToDetails.senderId.value + val attachmentThumbnailInfo = attachmentThumbnailInfoForInReplyTo(inReplyToDetails) + val text = textForInReplyTo(inReplyToDetails) + ReplyToContent( + senderName = senderName, + text = text, + attachmentThumbnailInfo = attachmentThumbnailInfo, + modifier = Modifier + .padding(top = 8.dp, start = 8.dp, end = 8.dp) + .clip(RoundedCornerShape(6.dp)) + .clickable(enabled = true, onClick = inReplyToClick), + ) + if (timestampPosition == TimestampPosition.Above) { + modifierWithPadding = Modifier.padding(start = 8.dp, end = 8.dp, bottom = 8.dp) + contentModifier = Modifier.clip(RoundedCornerShape(12.dp)) + } else { + contentModifier = Modifier.padding(start = 12.dp, end = 12.dp, top = 0.dp, bottom = 8.dp) + } + } + timestampPosition != TimestampPosition.Above -> { + contentModifier = Modifier.padding(start = 12.dp, end = 12.dp, top = 8.dp, bottom = 8.dp) } } ContentAndTimestampView( - overlayTimestamp = isMediaItem, + timestampPosition = timestampPosition, contentModifier = contentModifier, modifier = modifierWithPadding, ) } } - if (isMediaItem && replyToDetails == null) { - SimpleMediaItemLayout() - } else { - CommonLayout(inReplyToDetails = replyToDetails, modifier = modifier) - } + CommonLayout(inReplyToDetails = replyToDetails, modifier = modifier) } @Composable @@ -810,3 +817,23 @@ internal fun TimelineItemEventRowLongSenderNamePreview() = ElementPreviewLight { onTimestampClicked = {}, ) } + +// Note: no need for light/dark variant for this preview, we only look at the timestamp position +@Preview +@Composable +internal fun TimelineItemEventTimestampBelowPreview() = ElementPreviewLight { + TimelineItemEventRow( + event = aTimelineItemEvent(content = aTimelineItemPollContent()), + isHighlighted = false, + canReply = true, + onClick = {}, + onLongClick = {}, + onUserDataClick = {}, + inReplyToClick = {}, + onReactionClick = { _, _ -> }, + onReactionLongClick = { _, _ -> }, + onMoreReactionsClick = {}, + onSwipeToReply = {}, + onTimestampClicked = {}, + ) +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimestampPosition.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimestampPosition.kt new file mode 100644 index 0000000000..cb1a201f81 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimestampPosition.kt @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.messages.impl.timeline.components + +enum class TimestampPosition { + /** + * Timestamp should be rendered above the timeline event content (eg. image). + */ + Above, + + /** + * Timestamp should be aligned with the timeline event content if this is possible (eg. text). + */ + Aligned, + + /** + * Timestamp should always be rendered below the timeline event content (eg. poll). + */ + Below, +}