From 4ded05bbd1a241deba393cf2b076cc6917c3ecff Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Thu, 31 Aug 2023 16:31:49 -0300 Subject: [PATCH] Implement groundwork for proper ConversationItemV2 payload processing. --- .../components/LoggingAdapterDataObserver.kt | 41 +++++++++++++++++++ .../conversation/ConversationUpdateTick.kt | 2 +- .../conversation/v2/ConversationAdapterV2.kt | 17 +++++--- .../conversation/v2/ConversationFragment.kt | 2 +- .../V2ConversationItemMediaViewHolder.kt | 6 +++ .../V2ConversationItemTextOnlyViewHolder.kt | 31 +++++++++++++- .../conversation/v2/items/V2Payload.kt | 16 ++++++++ .../ConversationUpdateTickTest.kt | 20 ++++----- 8 files changed, 116 insertions(+), 19 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/components/LoggingAdapterDataObserver.kt create mode 100644 app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2Payload.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/LoggingAdapterDataObserver.kt b/app/src/main/java/org/thoughtcrime/securesms/components/LoggingAdapterDataObserver.kt new file mode 100644 index 0000000000..b81f782393 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/components/LoggingAdapterDataObserver.kt @@ -0,0 +1,41 @@ +/* + * Copyright 2023 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.components + +import androidx.recyclerview.widget.RecyclerView.AdapterDataObserver +import org.signal.core.util.logging.Log + +class LoggingAdapterDataObserver( + private val tag: String +) : AdapterDataObserver() { + override fun onChanged() { + Log.d(tag, "onChanged() called") + } + + override fun onItemRangeChanged(positionStart: Int, itemCount: Int) { + Log.d(tag, "onItemRangeChanged() called with: positionStart = $positionStart, itemCount = $itemCount") + } + + override fun onItemRangeChanged(positionStart: Int, itemCount: Int, payload: Any?) { + Log.d(tag, "onItemRangeChanged() called with: positionStart = $positionStart, itemCount = $itemCount, payload = $payload") + } + + override fun onItemRangeInserted(positionStart: Int, itemCount: Int) { + Log.d(tag, "onItemRangeInserted() called with: positionStart = $positionStart, itemCount = $itemCount") + } + + override fun onItemRangeRemoved(positionStart: Int, itemCount: Int) { + Log.d(tag, "onItemRangeRemoved() called with: positionStart = $positionStart, itemCount = $itemCount") + } + + override fun onItemRangeMoved(fromPosition: Int, toPosition: Int, itemCount: Int) { + Log.d(tag, "onItemRangeMoved() called with: fromPosition = $fromPosition, toPosition = $toPosition, itemCount = $itemCount") + } + + override fun onStateRestorationPolicyChanged() { + Log.d(tag, "onStateRestorationPolicyChanged() called") + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTick.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTick.kt index 092f8ec7f8..500774b484 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTick.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTick.kt @@ -28,7 +28,7 @@ class ConversationUpdateTick( isResumed = true handler.removeCallbacksAndMessages(null) - onTick() + handler.postDelayed(this::onTick, TIMEOUT) } override fun onPause(owner: LifecycleOwner) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt index 05b0f5b011..aa0374a717 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapterV2.kt @@ -38,6 +38,7 @@ import org.thoughtcrime.securesms.conversation.v2.data.ThreadHeader import org.thoughtcrime.securesms.conversation.v2.items.V2ConversationContext import org.thoughtcrime.securesms.conversation.v2.items.V2ConversationItemMediaViewHolder import org.thoughtcrime.securesms.conversation.v2.items.V2ConversationItemTextOnlyViewHolder +import org.thoughtcrime.securesms.conversation.v2.items.V2Payload import org.thoughtcrime.securesms.conversation.v2.items.bridge import org.thoughtcrime.securesms.database.model.MessageRecord import org.thoughtcrime.securesms.databinding.V2ConversationItemMediaIncomingBinding @@ -78,7 +79,7 @@ class ConversationAdapterV2( override val selectedItems: Set get() = _selected.toSet() - override var searchQuery: String? = null + override var searchQuery: String = "" private var inlineContent: ConversationMessage? = null private var recordToPulse: ConversationMessage? = null @@ -195,9 +196,13 @@ class ConversationAdapterV2( return getConversationMessage(adapterPosition + 1)?.messageRecord } - fun updateSearchQuery(searchQuery: String?) { + fun updateSearchQuery(searchQuery: String) { + val oldQuery = this.searchQuery this.searchQuery = searchQuery - notifyItemRangeChanged(0, itemCount) + + if (oldQuery != this.searchQuery) { + notifyItemRangeChanged(0, itemCount, V2Payload.SEARCH_QUERY_UPDATED) + } } fun getLastVisibleConversationMessage(position: Int): ConversationMessage? { @@ -230,7 +235,7 @@ class ConversationAdapterV2( fun playInlineContent(conversationMessage: ConversationMessage?) { if (this.inlineContent !== conversationMessage) { this.inlineContent = conversationMessage - notifyItemRangeChanged(0, itemCount) + notifyItemRangeChanged(0, itemCount, V2Payload.PLAY_INLINE_CONTENT) } } @@ -270,7 +275,7 @@ class ConversationAdapterV2( return if (this.hasWallpaper != hasWallpaper) { Log.d(TAG, "Resetting adapter due to wallpaper change.") this.hasWallpaper = hasWallpaper - notifyItemRangeChanged(0, itemCount) + notifyItemRangeChanged(0, itemCount, V2Payload.WALLPAPER) true } else { false @@ -282,7 +287,7 @@ class ConversationAdapterV2( this.isMessageRequestAccepted = isMessageRequestAccepted if (oldState != isMessageRequestAccepted) { - notifyItemRangeChanged(0, itemCount) + notifyItemRangeChanged(0, itemCount, V2Payload.MESSAGE_REQUEST_STATE) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt index 32755f45e4..32a64b667b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt @@ -1035,7 +1035,7 @@ class ConversationFragment : getVoiceNoteMediaController().voiceNotePlaybackState.observe(viewLifecycleOwner, inputPanel.playbackStateObserver) - val conversationUpdateTick = ConversationUpdateTick { viewModel.pagingController.onDataInvalidated() } + val conversationUpdateTick = ConversationUpdateTick { adapter.updateTimestamps() } viewLifecycleOwner.lifecycle.addObserver(conversationUpdateTick) if (args.conversationScreenType.isInPopup) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemMediaViewHolder.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemMediaViewHolder.kt index 9536dfbb9b..9f33c8031b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemMediaViewHolder.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemMediaViewHolder.kt @@ -47,6 +47,12 @@ class V2ConversationItemMediaViewHolder>( override fun bind(model: Model) { conversationMessage = (model as ConversationMessageElement).conversationMessage + + if (payload.isNotEmpty()) { + super.bind(model) + return + } + presentThumbnail() super.bind(model) presentQuote() diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt index eb585792de..84a16d7704 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2ConversationItemTextOnlyViewHolder.kt @@ -27,6 +27,7 @@ import org.signal.core.util.dp import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.components.mention.MentionAnnotation import org.thoughtcrime.securesms.conversation.BodyBubbleLayoutTransition +import org.thoughtcrime.securesms.conversation.ConversationAdapterBridge import org.thoughtcrime.securesms.conversation.ConversationItemDisplayMode import org.thoughtcrime.securesms.conversation.ConversationMessage import org.thoughtcrime.securesms.conversation.colors.ChatColors @@ -161,6 +162,26 @@ open class V2ConversationItemTextOnlyViewHolder>( adapterPosition = bindingAdapterPosition ) + var hasProcessedSupportedPayload = false + if (ConversationAdapterBridge.PAYLOAD_TIMESTAMP in payload) { + presentDate(shape) + hasProcessedSupportedPayload = true + } + + if (ConversationAdapterBridge.PAYLOAD_NAME_COLORS in payload) { + presentSenderNameColor() + hasProcessedSupportedPayload = true + } + + if (V2Payload.SEARCH_QUERY_UPDATED in payload) { + presentBody() + hasProcessedSupportedPayload = true + } + + if (hasProcessedSupportedPayload) { + return + } + presentBody() presentDate() presentDeliveryStatus() @@ -448,7 +469,6 @@ open class V2ConversationItemTextOnlyViewHolder>( binding.senderBadge.visibility = photoVisibility binding.senderName.text = sender.getDisplayName(context) - binding.senderName.setTextColor(conversationContext.getColorizer().getIncomingGroupSenderColor(context, sender)) binding.senderPhoto.setAvatar(conversationContext.glideRequests, sender, false) binding.senderBadge.setBadgeFromRecipient(sender, conversationContext.glideRequests) binding.senderPhoto.setOnClickListener { @@ -464,6 +484,15 @@ open class V2ConversationItemTextOnlyViewHolder>( } } + private fun presentSenderNameColor() { + if (binding.senderName == null || !conversationMessage.threadRecipient.isGroup) { + return + } + + val sender = conversationMessage.messageRecord.fromRecipient + binding.senderName.setTextColor(conversationContext.getColorizer().getIncomingGroupSenderColor(context, sender)) + } + private fun presentAlert() { val record = conversationMessage.messageRecord binding.conversationItemBody.setCompoundDrawablesWithIntrinsicBounds( diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2Payload.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2Payload.kt new file mode 100644 index 0000000000..35fffbcfa1 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/items/V2Payload.kt @@ -0,0 +1,16 @@ +/* + * Copyright 2023 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.conversation.v2.items + +/** + * Describes the different V2-Specific payloads that can be emitted. + */ +enum class V2Payload { + SEARCH_QUERY_UPDATED, + PLAY_INLINE_CONTENT, + WALLPAPER, + MESSAGE_REQUEST_STATE +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTickTest.kt b/app/src/test/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTickTest.kt index 540cc4a604..6dea15735a 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTickTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/conversation/ConversationUpdateTickTest.kt @@ -30,27 +30,27 @@ class ConversationUpdateTickTest { } @Test - fun `Given no time has passed after onResume is invoked, then I expect one invocations of onTick`() { + fun `Given no time has passed after onResume is invoked, then I expect no invocations of onTick`() { // GIVEN ShadowLooper.pauseMainLooper() testSubject.onResume(lifecycleOwner) // THEN - verify(listener, times(1)).onTick() + verify(listener, never()).onTick() } @Test - fun `Given onResume is invoked, when half timeout passes, then I expect one invocations of onTick`() { + fun `Given onResume is invoked, when half timeout passes, then I expect no invocations of onTick`() { // GIVEN testSubject.onResume(lifecycleOwner) ShadowLooper.idleMainLooper(timeoutMillis / 2, TimeUnit.MILLISECONDS) // THEN - verify(listener, times(1)).onTick() + verify(listener, never()).onTick() } @Test - fun `Given onResume is invoked, when timeout passes, then I expect two invocations of onTick`() { + fun `Given onResume is invoked, when timeout passes, then I expect one invocation of onTick`() { // GIVEN testSubject.onResume(lifecycleOwner) @@ -58,11 +58,11 @@ class ConversationUpdateTickTest { ShadowLooper.idleMainLooper(timeoutMillis, TimeUnit.MILLISECONDS) // THEN - verify(listener, times(2)).onTick() + verify(listener, times(1)).onTick() } @Test - fun `Given onResume is invoked, when timeout passes five times, then I expect six invocations of onTick`() { + fun `Given onResume is invoked, when timeout passes five times, then I expect five invocations of onTick`() { // GIVEN testSubject.onResume(lifecycleOwner) @@ -70,11 +70,11 @@ class ConversationUpdateTickTest { ShadowLooper.idleMainLooper(timeoutMillis * 5, TimeUnit.MILLISECONDS) // THEN - verify(listener, times(6)).onTick() + verify(listener, times(5)).onTick() } @Test - fun `Given onResume then onPause is invoked, when timeout passes, then I expect one invocation of onTick`() { + fun `Given onResume then onPause is invoked, when timeout passes, then I expect no invocation of onTick`() { // GIVEN testSubject.onResume(lifecycleOwner) testSubject.onPause(lifecycleOwner) @@ -83,6 +83,6 @@ class ConversationUpdateTickTest { ShadowLooper.idleMainLooper(timeoutMillis, TimeUnit.MILLISECONDS) // THEN - verify(listener, times(1)).onTick() + verify(listener, never()).onTick() } }