SES-2145 - Fix re-scroll to bottom after clicking on original message in a reply (#961)

* Re-scroll to bottom after click on OG msg of reply fixed & message highlighting added to match iOS

* Fixing UI issues

Making sure the glow isn't clipped
Scrolling past the quoted view so that it isn't right at the top

* Consolitate recycler view scroll & highlight functionality into a single scrollListener

* Fix comment typo

* Scroll past targeted messages by a given offset in pixels rather than in messages

* Made the linearSmoothScroller lazy and tidied up

* Removed unused property

* Precalculate scroll offset for scroll-target messages

---------

Signed-off-by: alansley <aclansley@gmail.com>
Co-authored-by: alansley <aclansley@gmail.com>
Co-authored-by: ThomasSession <thomas.r@getsession.org>

app/build.gradle

@@ -269,7 +269,7 @@ dependencies {
 
     implementation("com.google.dagger:hilt-android:$daggerHiltVersion")
     implementation "androidx.appcompat:appcompat:$appcompatVersion"
-    implementation 'androidx.recyclerview:recyclerview:1.2.1'
+    implementation 'androidx.recyclerview:recyclerview:1.3.2'
     implementation "com.google.android.material:material:$materialVersion"
     implementation 'com.google.android.flexbox:flexbox:3.0.0'
     implementation 'androidx.legacy:legacy-support-v13:1.0.0'

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt

@@ -51,6 +51,7 @@ import androidx.lifecycle.repeatOnLifecycle
 import androidx.loader.app.LoaderManager
 import androidx.loader.content.Loader
 import androidx.recyclerview.widget.LinearLayoutManager
+import androidx.recyclerview.widget.LinearSmoothScroller
 import androidx.recyclerview.widget.RecyclerView
 import com.annimon.stream.Stream
 import com.bumptech.glide.Glide
@@ -102,10 +103,10 @@ import org.session.libsession.utilities.getColorFromAttr
 import org.session.libsession.utilities.recipients.Recipient
 import org.session.libsession.utilities.recipients.RecipientModifiedListener
 import org.session.libsignal.crypto.MnemonicCodec
+import org.session.libsignal.utilities.AccountId
 import org.session.libsignal.utilities.IdPrefix
 import org.session.libsignal.utilities.ListenableFuture
 import org.session.libsignal.utilities.Log
-import org.session.libsignal.utilities.AccountId
 import org.session.libsignal.utilities.guava.Optional
 import org.session.libsignal.utilities.hexEncodedPrivateKey
 import org.thoughtcrime.securesms.ApplicationContext
@@ -118,7 +119,7 @@ import org.thoughtcrime.securesms.conversation.ConversationActionBarDelegate
 import org.thoughtcrime.securesms.conversation.disappearingmessages.DisappearingMessagesActivity
 import org.thoughtcrime.securesms.conversation.v2.ConversationReactionOverlay.OnActionSelectedListener
 import org.thoughtcrime.securesms.conversation.v2.ConversationReactionOverlay.OnReactionSelectedListener
-import org.thoughtcrime.securesms.conversation.v2.ConversationViewModel.Commands.*
+import org.thoughtcrime.securesms.conversation.v2.ConversationViewModel.Commands.ShowOpenUrlDialog
 import org.thoughtcrime.securesms.conversation.v2.MessageDetailActivity.Companion.MESSAGE_TIMESTAMP
 import org.thoughtcrime.securesms.conversation.v2.MessageDetailActivity.Companion.ON_COPY
 import org.thoughtcrime.securesms.conversation.v2.MessageDetailActivity.Companion.ON_DELETE
@@ -312,12 +313,6 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     private val EMOJI_REACTIONS_ALLOWED_PER_MINUTE = 20
     private val ONE_MINUTE_IN_MILLISECONDS = 1.minutes.inWholeMilliseconds
 
-    private val isScrolledToBottom: Boolean
-        get() = binding.conversationRecyclerView.isScrolledToBottom
-
-    private val isScrolledToWithin30dpOfBottom: Boolean
-        get() = binding.conversationRecyclerView.isScrolledToWithin30dpOfBottom
-
     private val layoutManager: LinearLayoutManager?
         get() { return binding.conversationRecyclerView.layoutManager as LinearLayoutManager? }
 
@@ -429,11 +424,30 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         }
     }
 
-    // Properties for what message indices are visible previously & now, as well as the scroll state
+    // Properties related to the conversation recycler view's scroll state and position
     private var previousLastVisibleRecyclerViewIndex: Int = RecyclerView.NO_POSITION
     private var currentLastVisibleRecyclerViewIndex:  Int = RecyclerView.NO_POSITION
     private var recyclerScrollState: Int = RecyclerView.SCROLL_STATE_IDLE
 
+    private val isScrolledToBottom: Boolean
+        get() = binding.conversationRecyclerView.isScrolledToBottom
+
+    // When the user clicks on the original message in a reply then we scroll to and highlight that original
+    // message. To do this we keep track of the replied-to message's location in the recycler view.
+    private var pendingHighlightMessagePosition: Int? = null
+
+    // Used to target a specific message and scroll to it with some breathing room above (offset) for all messages but the first
+    private var currentTargetedScrollOffsetPx: Int = 0
+    private val nonFirstMessageOffsetPx by lazy { resources.getDimensionPixelSize(R.dimen.massive_spacing) * -1 }
+    private val linearSmoothScroller by lazy {
+        object : LinearSmoothScroller(binding.conversationRecyclerView.context) {
+            override fun getVerticalSnapPreference(): Int = SNAP_TO_START
+            override fun calculateDyToMakeVisible(view: View, snapPreference: Int): Int {
+                return super.calculateDyToMakeVisible(view, snapPreference) - currentTargetedScrollOffsetPx
+            }
+        }
+    }
+
     // region Settings
     companion object {
         // Extras
@@ -452,9 +466,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
     // endregion
 
-    fun showOpenUrlDialog(url: String){
+    fun showOpenUrlDialog(url: String) = viewModel.onCommand(ShowOpenUrlDialog(url))
-        viewModel.onCommand(ShowOpenUrlDialog(url))
-    }
 
     // region Lifecycle
     override fun onCreate(savedInstanceState: Bundle?, isReady: Boolean) {
@@ -494,23 +506,12 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
             val layoutManager = binding.conversationRecyclerView.layoutManager as LinearLayoutManager
             val targetPosition = if (reverseMessageList) 0 else adapter.itemCount
 
+            // If we are currently in the process of smooth scrolling then we'll use `scrollToPosition` to quick-jump..
             if (layoutManager.isSmoothScrolling) {
                 binding.conversationRecyclerView.scrollToPosition(targetPosition)
             } else {
-                // It looks like 'smoothScrollToPosition' will actually load all intermediate items in
+                // ..otherwise we'll use the animated `smoothScrollToPosition` to scroll to our target position.
-                // order to do the scroll, this can be very slow if there are a lot of messages so
+                binding.conversationRecyclerView.smoothScrollToPosition(targetPosition)
-                // instead we check the current position and if there are more than 10 items to scroll
-                // we jump instantly to the 10th item and scroll from there (this should happen quick
-                // enough to give a similar scroll effect without having to load everything)
-//                val position = if (reverseMessageList) layoutManager.findFirstVisibleItemPosition() else layoutManager.findLastVisibleItemPosition()
-//                val targetBuffer = if (reverseMessageList) 10 else Math.max(0, (adapter.itemCount - 1) - 10)
-//                if (position > targetBuffer) {
-//                    binding.conversationRecyclerView?.scrollToPosition(targetBuffer)
-//                }
-
-                binding.conversationRecyclerView.post {
-                    binding.conversationRecyclerView.smoothScrollToPosition(targetPosition)
-                }
             }
         }
 
@@ -533,7 +534,12 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
             // by triggering 'jumpToMessage' using these values
             val messageTimestamp = messageToScrollTimestamp.get()
             val author = messageToScrollAuthor.get()
-            val targetPosition = if (author != null && messageTimestamp >= 0) mmsSmsDb.getMessagePositionInConversation(viewModel.threadId, messageTimestamp, author, reverseMessageList) else -1
+
+            val targetPosition = if (author != null && messageTimestamp >= 0) {
+                mmsSmsDb.getMessagePositionInConversation(viewModel.threadId, messageTimestamp, author, reverseMessageList)
+            } else {
+                -1
+            }
 
             withContext(Dispatchers.Main) {
                 setUpRecyclerView()
@@ -635,10 +641,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     override fun getSystemService(name: String): Any? {
-        if (name == ActivityDispatcher.SERVICE) {
+        return if (name == ActivityDispatcher.SERVICE) { this } else { super.getSystemService(name) }
-            return this
-        }
-        return super.getSystemService(name)
     }
 
     override fun dispatchIntent(body: (Context) -> Intent?) {
@@ -686,9 +689,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         }
     }
 
-    override fun onLoaderReset(cursor: Loader<Cursor>) {
+    override fun onLoaderReset(cursor: Loader<Cursor>) = adapter.changeCursor(null)
-        adapter.changeCursor(null)
-    }
 
     // called from onCreate
     private fun setUpRecyclerView() {
@@ -700,7 +701,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         binding.conversationRecyclerView.addOnScrollListener(object : RecyclerView.OnScrollListener() {
 
             override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) {
-                // The unreadCount check is to prevent us scrolling to the bottom when we first enter a conversation
+                // The unreadCount check is to prevent us scrolling to the bottom when we first enter a conversation.
                 if (recyclerScrollState == RecyclerView.SCROLL_STATE_IDLE && unreadCount != Int.MAX_VALUE) {
                     scrollToMostRecentMessageIfWeShould()
                 }
@@ -709,6 +710,17 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
 
             override fun onScrollStateChanged(recyclerView: RecyclerView, newState: Int) {
                 recyclerScrollState = newState
+
+                // If we were scrolling towards a specific message to highlight when scrolling stops then do so
+                if (newState == RecyclerView.SCROLL_STATE_IDLE) {
+                    pendingHighlightMessagePosition?.let { position ->
+                        recyclerView.findViewHolderForLayoutPosition(position)?.let { viewHolder ->
+                            (viewHolder.itemView as? VisibleMessageView)?.playHighlight()
+                                ?: Log.w(TAG, "View at position $position is not a VisibleMessageView - cannot highlight.")
+                        } ?: Log.w(TAG, "ViewHolder at position $position is null - cannot highlight.")
+                        pendingHighlightMessagePosition = null
+                    }
+                }
             }
         })
 
@@ -720,13 +732,15 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     private fun scrollToMostRecentMessageIfWeShould() {
+        val lm = layoutManager ?: return Log.w(TAG, "Cannot scroll recycler view without a layout manager - bailing.")
+
         // Grab an initial 'previous' last visible message..
         if (previousLastVisibleRecyclerViewIndex == RecyclerView.NO_POSITION) {
-            previousLastVisibleRecyclerViewIndex = layoutManager?.findLastVisibleItemPosition()!!
+            previousLastVisibleRecyclerViewIndex = lm.findLastVisibleItemPosition()
         }
 
         // ..and grab the 'current' last visible message.
-        currentLastVisibleRecyclerViewIndex = layoutManager?.findLastVisibleItemPosition()!!
+        currentLastVisibleRecyclerViewIndex = lm.findLastVisibleItemPosition()
 
         // If the current last visible message index is less than the previous one (i.e. we've
         // lost visibility of one or more messages due to showing the IME keyboard) AND we're
@@ -737,12 +751,9 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         // ..OR we're at the last message or have received a new message..
         val atLastOrReceivedNewMessage = currentLastVisibleRecyclerViewIndex == (adapter.itemCount - 1)
 
-        // ..then scroll the recycler view to the last message on resize. Note: We cannot just call
+        // ..then scroll the recycler view to the last message on resize.
-        // scroll/smoothScroll - we have to `post` it or nothing happens!
         if (atBottomAndTrueLastNoLongerVisible || atLastOrReceivedNewMessage) {
-            binding.conversationRecyclerView.post {
+            binding.conversationRecyclerView.smoothScrollToPosition(adapter.itemCount)
-                binding.conversationRecyclerView.smoothScrollToPosition(adapter.itemCount)
-            }
         }
 
         // Update our previous last visible view index to the current one
@@ -846,13 +857,8 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         }
     }
 
-    private fun setUpRecipientObserver() {
+    private fun setUpRecipientObserver()    = viewModel.recipient?.addListener(this)
-        viewModel.recipient?.addListener(this)
+    private fun tearDownRecipientObserver() = viewModel.recipient?.removeListener(this)
-    }
-
-    private fun tearDownRecipientObserver() {
-        viewModel.recipient?.removeListener(this)
-    }
 
     private fun getLatestOpenGroupInfoIfNeeded() {
         val openGroup = viewModel.openGroup ?: return
@@ -1340,11 +1346,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     override fun onOptionsItemSelected(item: MenuItem): Boolean {
-        if (item.itemId == android.R.id.home) {
+        return if (item.itemId == android.R.id.home) false else viewModel.onOptionItemSelected(this, item)
-            return false
-        }
-
-        return viewModel.onOptionItemSelected(this, item)
     }
 
     override fun block(deleteThread: Boolean) {
@@ -1536,9 +1538,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         reactionDelegate.show(this, message, selectedConversationModel, viewModel.blindedPublicKey)
     }
 
-    override fun dispatchTouchEvent(ev: MotionEvent): Boolean {
+    override fun dispatchTouchEvent(ev: MotionEvent): Boolean = reactionDelegate.applyTouchEvent(ev) || super.dispatchTouchEvent(ev)
-        return reactionDelegate.applyTouchEvent(ev) || super.dispatchTouchEvent(ev)
-    }
 
     override fun onReactionSelected(messageRecord: MessageRecord, emoji: String) {
         reactionDelegate.hide()
@@ -1684,9 +1684,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         }
     }
 
-    override fun onReactWithAnyEmojiDialogDismissed() {
+    override fun onReactWithAnyEmojiDialogDismissed() = reactionDelegate.hide()
-        reactionDelegate.hide()
-    }
 
     override fun onReactWithAnyEmojiSelected(emoji: String, messageId: MessageId) {
         reactionDelegate.hide()
@@ -1713,7 +1711,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
     }
 
     // Called when the user is attempting to clear all instance of a specific emoji
-    override fun onClearAll(emoji: String, messageId: MessageId) { viewModel.onEmojiClear(emoji, messageId) }
+    override fun onClearAll(emoji: String, messageId: MessageId) = viewModel.onEmojiClear(emoji, messageId)
 
     override fun onMicrophoneButtonMove(event: MotionEvent) {
         val rawX = event.rawX
@@ -1745,9 +1743,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         }
     }
 
-    override fun onMicrophoneButtonCancel(event: MotionEvent) {
+    override fun onMicrophoneButtonCancel(event: MotionEvent) = hideVoiceMessageUI()
-        hideVoiceMessageUI()
-    }
 
     override fun onMicrophoneButtonUp(event: MotionEvent) {
         if(binding.inputBar.voiceRecorderState != VoiceRecorderState.Recording){
@@ -1801,9 +1797,27 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate,
         return hitRect.contains(x, y)
     }
 
-    override fun scrollToMessageIfPossible(timestamp: Long) {
+    override fun highlightMessageFromTimestamp(timestamp: Long) {
-        val lastSeenItemPosition = adapter.getItemPositionForTimestamp(timestamp) ?: return
+        // Try to find the message with the given timestamp
-        binding.conversationRecyclerView?.scrollToPosition(lastSeenItemPosition)
+        adapter.getItemPositionForTimestamp(timestamp)?.let { targetMessagePosition ->
+
+            // If the view is already visible then we don't have to scroll before highlighting it..
+            binding.conversationRecyclerView.findViewHolderForLayoutPosition(targetMessagePosition)?.let { viewHolder ->
+                if (viewHolder.itemView is VisibleMessageView) {
+                    (viewHolder.itemView as VisibleMessageView).playHighlight()
+                    return
+                }
+            }
+
+            // ..otherwise, set the pending highlight target and trigger a scroll.
+            // Note: If the targeted message isn't the very first one then we scroll slightly past it to give it some breathing room.
+            // Also: The offset must be negative to provide room above it.
+            pendingHighlightMessagePosition = targetMessagePosition
+            currentTargetedScrollOffsetPx = if (targetMessagePosition > 0) nonFirstMessageOffsetPx else 0
+            linearSmoothScroller.targetPosition = targetMessagePosition
+            (binding.conversationRecyclerView.layoutManager as? LinearLayoutManager)?.startSmoothScroll(linearSmoothScroller)
+            
+        } ?: Log.i(TAG, "Could not find message with timestamp: $timestamp")
     }
 
     override fun onReactionClicked(emoji: String, messageId: MessageId, userWasSender: Boolean) {

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapter.kt

@@ -13,6 +13,8 @@ import androidx.core.util.set
 import androidx.lifecycle.LifecycleCoroutineScope
 import androidx.recyclerview.widget.RecyclerView.ViewHolder
 import com.bumptech.glide.RequestManager
+import java.util.concurrent.atomic.AtomicLong
+import kotlin.math.min
 import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.channels.BufferOverflow
 import kotlinx.coroutines.channels.Channel
@@ -28,8 +30,6 @@ import org.thoughtcrime.securesms.conversation.v2.messages.VisibleMessageViewDel
 import org.thoughtcrime.securesms.database.CursorRecyclerViewAdapter
 import org.thoughtcrime.securesms.database.model.MessageRecord
 import org.thoughtcrime.securesms.dependencies.DatabaseComponent
-import java.util.concurrent.atomic.AtomicLong
-import kotlin.math.min
 
 class ConversationAdapter(
     context: Context,
@@ -73,9 +73,7 @@ class ConversationAdapter(
     }
 
     @WorkerThread
-    private fun getSenderInfo(sender: String): Contact? {
+    private fun getSenderInfo(sender: String): Contact? = contactDB.getContactWithAccountID(sender)
-        return contactDB.getContactWithAccountID(sender)
-    }
 
     sealed class ViewType(val rawValue: Int) {
         object Visible : ViewType(0)
@@ -193,9 +191,7 @@ class ConversationAdapter(
         super.onItemViewRecycled(viewHolder)
     }
 
-    private fun getMessage(cursor: Cursor): MessageRecord? {
+    private fun getMessage(cursor: Cursor): MessageRecord? = messageDB.readerFor(cursor).current
-        return messageDB.readerFor(cursor).current
-    }
 
     private fun getMessageBefore(position: Int, cursor: Cursor): MessageRecord? {
         // The message that's visually before the current one is actually after the current

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageContentView.kt

@@ -11,6 +11,7 @@ import android.text.util.Linkify
 import android.util.AttributeSet
 import android.view.MotionEvent
 import android.view.View
+import android.view.ViewGroup
 import androidx.annotation.ColorInt
 import androidx.constraintlayout.widget.ConstraintLayout
 import androidx.core.graphics.ColorUtils
@@ -123,7 +124,7 @@ class VisibleMessageContentView : ConstraintLayout {
                 val r = Rect()
                 binding.quoteView.root.getGlobalVisibleRect(r)
                 if (r.contains(event.rawX.roundToInt(), event.rawY.roundToInt())) {
-                    delegate?.scrollToMessageIfPossible(quote.id)
+                    delegate?.highlightMessageFromTimestamp(quote.id)
                 }
             }
         }

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt

@@ -164,6 +164,9 @@ class VisibleMessageView : FrameLayout {
         delegate: VisibleMessageViewDelegate? = null,
         onAttachmentNeedsDownload: (DatabaseAttachment) -> Unit
     ) {
+        clipToPadding = false
+        clipChildren = false
+
         isOutgoing = message.isOutgoing
         replyDisabled = message.isOpenGroupInvitation
         val threadID = message.threadId

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageViewDelegate.kt

@@ -3,13 +3,8 @@ package org.thoughtcrime.securesms.conversation.v2.messages
 import org.thoughtcrime.securesms.database.model.MessageId
 
 interface VisibleMessageViewDelegate {
-
     fun playVoiceMessageAtIndexIfPossible(indexInAdapter: Int)
-
+    fun highlightMessageFromTimestamp(timestamp: Long)
-    fun scrollToMessageIfPossible(timestamp: Long)
-
     fun onReactionClicked(emoji: String, messageId: MessageId, userWasSender: Boolean)
-
     fun onReactionLongClicked(messageId: MessageId, emoji: String?)
-
 }

app/src/main/java/org/thoughtcrime/securesms/groups/BaseGroupMembersViewModel.kt

@@ -5,6 +5,7 @@ import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.assisted.AssistedFactory
 import dagger.hilt.android.qualifiers.ApplicationContext
+import java.util.EnumSet
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
@@ -22,8 +23,6 @@ import org.session.libsession.utilities.ConfigFactoryProtocol
 import org.session.libsession.utilities.ConfigUpdateNotification
 import org.session.libsession.utilities.UsernameUtils
 import org.session.libsignal.utilities.AccountId
-import java.util.EnumSet
-
 
 abstract class BaseGroupMembersViewModel (
     private val groupId: AccountId,

app/src/main/java/org/thoughtcrime/securesms/util/GlowView.kt

@@ -11,8 +11,8 @@ import android.view.animation.AccelerateDecelerateInterpolator
 import android.widget.LinearLayout
 import android.widget.RelativeLayout
 import androidx.annotation.ColorInt
-import network.loki.messenger.R
 import kotlin.math.roundToInt
+import network.loki.messenger.R
 
 interface GlowView {
     var mainColor: Int