From 57d1de165e78445d2c64137ce613c8bb865b9bd5 Mon Sep 17 00:00:00 2001 From: AL-Session <160798022+AL-Session@users.noreply.github.com> Date: Wed, 17 Apr 2024 17:25:21 +1000 Subject: [PATCH] SES1718 - Message Sending Status (#1462) * Investigation in progress * Initial push for PR * Fixes #1461 * Removed leftover debug comments * Added minor optimisation to showMessageStatus method (bail early if the message isn't one we care about displaying details of to the user) * Minor cleanup * Tiny cleanup * Addressed PR feedback * Removed forgotten debug log line & forced delivery status elements to be removed on non-visible messages just in case * Minor refactor to simplify 'VisibleMessageView.showStatusMessage' --------- Co-authored-by: alansley --- .../disappearingmessages/State.kt | 2 +- .../v2/messages/VisibleMessageView.kt | 133 +++++++++++++----- .../database/model/DisplayRecord.java | 5 + 3 files changed, 105 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/disappearingmessages/State.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/disappearingmessages/State.kt index cc968df398..ced4cc0035 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/disappearingmessages/State.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/disappearingmessages/State.kt @@ -68,7 +68,7 @@ enum class ExpiryType( AFTER_SEND( ExpiryMode::AfterSend, R.string.expiration_type_disappear_after_send, - R.string.expiration_type_disappear_after_read_description, + R.string.expiration_type_disappear_after_send_description, R.string.AccessibilityId_disappear_after_send_option ); diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt index a59a102d1c..f22de40c10 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt @@ -37,6 +37,7 @@ import org.session.libsession.utilities.ViewUtil import org.session.libsession.utilities.getColorFromAttr import org.session.libsession.utilities.modifyLayoutParams import org.session.libsignal.utilities.IdPrefix +import org.session.libsignal.utilities.Log import org.thoughtcrime.securesms.conversation.v2.ConversationActivityV2 import org.thoughtcrime.securesms.database.LokiAPIDatabase import org.thoughtcrime.securesms.database.LokiThreadDatabase @@ -206,7 +207,7 @@ class VisibleMessageView : LinearLayout { binding.dateBreakTextView.text = if (showDateBreak) DateUtils.getDisplayFormattedTimeSpanString(context, Locale.getDefault(), message.timestamp) else null binding.dateBreakTextView.isVisible = showDateBreak - // Message status indicator + // Update message status indicator showStatusMessage(message) // Emoji Reactions @@ -243,44 +244,101 @@ class VisibleMessageView : LinearLayout { onDoubleTap = { binding.messageContentView.root.onContentDoubleTap?.invoke() } } + // Method to display or hide the status of a message. + // Note: Although most commonly used to display the delivery status of a message, we also use the + // message status area to display the disappearing messages state - so in this latter case we'll + // be displaying the "Sent" and the animating clock icon for outgoing messages or "Read" and the + // animated clock icon for incoming messages. private fun showStatusMessage(message: MessageRecord) { + // We'll start by hiding everything and then only make visible what we need + binding.messageStatusTextView.isVisible = false + binding.messageStatusImageView.isVisible = false + binding.expirationTimerView.isVisible = false - val scheduledToDisappear = message.expiresIn > 0 + // Get details regarding how we should display the message (it's delivery icon, icon tint colour, and + // the resource string for what text to display (R.string.delivery_status_sent etc.). + val (iconID, iconColor, textId) = getMessageStatusInfo(message) + + // If we get any nulls then a message isn't one with a state that we care about (i.e., control messages + // etc.) - so bail. See: `DisplayRecord.is` for the full suite of message state methods. + // Also: We set all delivery status elements visibility to false just to make sure we don't display any + // stale data. + if (textId == null) return binding.messageInnerLayout.modifyLayoutParams { gravity = if (message.isOutgoing) Gravity.END else Gravity.START } - binding.statusContainer.modifyLayoutParams { horizontalBias = if (message.isOutgoing) 1f else 0f } - binding.expirationTimerView.isGone = true + // If the message is incoming AND it is not scheduled to disappear then don't show any status or timer details + val scheduledToDisappear = message.expiresIn > 0 + if (message.isIncoming && !scheduledToDisappear) return - if (message.isOutgoing || scheduledToDisappear) { - val (iconID, iconColor, textId) = getMessageStatusImage(message) - textId?.let(binding.messageStatusTextView::setText) - iconColor?.let(binding.messageStatusTextView::setTextColor) - iconID?.let { ContextCompat.getDrawable(context, it) } - ?.run { iconColor?.let { mutate().apply { setTint(it) } } ?: this } - ?.let(binding.messageStatusImageView::setImageDrawable) + // Set text & icons as appropriate for the message state. Note: Possible message states we care + // about are: isFailed, isSyncFailed, isPending, isSyncing, isResyncing, isRead, and isSent. + textId.let(binding.messageStatusTextView::setText) + iconColor?.let(binding.messageStatusTextView::setTextColor) + iconID?.let { ContextCompat.getDrawable(context, it) } + ?.run { iconColor?.let { mutate().apply { setTint(it) } } ?: this } + ?.let(binding.messageStatusImageView::setImageDrawable) - // Always show the delivery status of the last sent message - val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context) - val lastSentMessageId = mmsSmsDb.getLastSentMessageFromSender(message.threadId, thisUsersSessionId) - val isLastSentMessage = lastSentMessageId == message.id + // Potential options at this point are that the message is: + // i.) incoming AND scheduled to disappear. + // ii.) outgoing but NOT scheduled to disappear, or + // iii.) outgoing AND scheduled to disappear. - binding.messageStatusTextView.isVisible = textId != null && (isLastSentMessage || scheduledToDisappear) - val showTimer = scheduledToDisappear && !message.isPending - binding.messageStatusImageView.isVisible = iconID != null && !showTimer && (!message.isSent || isLastSentMessage) - - binding.messageStatusImageView.bringToFront() + // ----- Case i..) Message is incoming and scheduled to disappear ----- + if (message.isIncoming && scheduledToDisappear) { + // Display the status ('Read') and the show the timer only (no delivery icon) + binding.messageStatusTextView.isVisible = true + binding.expirationTimerView.isVisible = true binding.expirationTimerView.bringToFront() - binding.expirationTimerView.isVisible = showTimer - if (showTimer) updateExpirationTimer(message) - } else { - binding.messageStatusTextView.isVisible = false - binding.messageStatusImageView.isVisible = false + updateExpirationTimer(message) + return + } + + // --- If we got here then we know the message is outgoing --- + + val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context) + val lastSentMessageId = mmsSmsDb.getLastSentMessageFromSender(message.threadId, thisUsersSessionId) + val isLastSentMessage = lastSentMessageId == message.id + + // ----- Case ii.) Message is outgoing but NOT scheduled to disappear ----- + if (!scheduledToDisappear) { + // If this isn't a disappearing message then we never show the timer + + // If the message has NOT been successfully sent then always show the delivery status text and icon.. + val neitherSentNorRead = !(message.isSent || message.isRead) + if (neitherSentNorRead) { + binding.messageStatusTextView.isVisible = true + binding.messageStatusImageView.isVisible = true + } else { + // ..but if the message HAS been successfully sent or read then only display the delivery status + // text and image if this is the last sent message. + binding.messageStatusTextView.isVisible = isLastSentMessage + binding.messageStatusImageView.isVisible = isLastSentMessage + if (isLastSentMessage) { binding.messageStatusImageView.bringToFront() } + } + } + else // ----- Case iii.) Message is outgoing AND scheduled to disappear ----- + { + // Always display the delivery status text on all outgoing disappearing messages + binding.messageStatusTextView.isVisible = true + + // If the message is sent or has been read.. + val sentOrRead = message.isSent || message.isRead + if (sentOrRead) { + // ..then display the timer icon for this disappearing message (but keep the message status icon hidden) + binding.expirationTimerView.isVisible = true + binding.expirationTimerView.bringToFront() + updateExpirationTimer(message) + } else { + // If the message has NOT been sent or read (or it has failed) then show the delivery status icon rather than the timer icon + binding.messageStatusImageView.isVisible = true + binding.messageStatusImageView.bringToFront() + } } } @@ -302,10 +360,9 @@ class VisibleMessageView : LinearLayout { @ColorInt val iconTint: Int?, @StringRes val messageText: Int?) - private fun getMessageStatusImage(message: MessageRecord): MessageStatusInfo = when { + private fun getMessageStatusInfo(message: MessageRecord): MessageStatusInfo = when { message.isFailed -> - MessageStatusInfo( - R.drawable.ic_delivery_status_failed, + MessageStatusInfo(R.drawable.ic_delivery_status_failed, resources.getColor(R.color.destructive, context.theme), R.string.delivery_status_failed ) @@ -318,24 +375,32 @@ class VisibleMessageView : LinearLayout { message.isPending -> MessageStatusInfo( R.drawable.ic_delivery_status_sending, - context.getColorFromAttr(R.attr.message_status_color), R.string.delivery_status_sending + context.getColorFromAttr(R.attr.message_status_color), + R.string.delivery_status_sending ) - message.isResyncing -> + message.isSyncing || message.isResyncing -> MessageStatusInfo( R.drawable.ic_delivery_status_sending, - context.getColor(R.color.accent_orange), R.string.delivery_status_syncing + context.getColorFromAttr(R.attr.message_status_color), + R.string.delivery_status_sending // We COULD tell the user that we're `syncing` (R.string.delivery_status_syncing) but it will likely make more sense to them if we say "Sending" ) - message.isRead || !message.isOutgoing -> + message.isRead || message.isIncoming -> MessageStatusInfo( R.drawable.ic_delivery_status_read, - context.getColorFromAttr(R.attr.message_status_color), R.string.delivery_status_read + context.getColorFromAttr(R.attr.message_status_color), + R.string.delivery_status_read ) - else -> + message.isSent -> MessageStatusInfo( R.drawable.ic_delivery_status_sent, context.getColorFromAttr(R.attr.message_status_color), R.string.delivery_status_sent ) + else -> { + // The message isn't one we care about for message statuses we display to the user (i.e., + // control messages etc. - see the `DisplayRecord.is` suite of methods for options). + MessageStatusInfo(null, null, null) + } } private fun updateExpirationTimer(message: MessageRecord) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java b/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java index 605118f3cc..db1076e472 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java @@ -114,6 +114,11 @@ public abstract class DisplayRecord { public boolean isOutgoing() { return MmsSmsColumns.Types.isOutgoingMessageType(type); } + + public boolean isIncoming() { + return !MmsSmsColumns.Types.isOutgoingMessageType(type); + } + public boolean isGroupUpdateMessage() { return SmsDatabase.Types.isGroupUpdateMessage(type); }