[SES-3536] - Unable to navigate back on search screen (#1031)

app/src/main/java/org/thoughtcrime/securesms/home/HomeActivity.kt

@@ -167,8 +167,7 @@ class HomeActivity : ScreenLockActionBarActivity(),
         // Set up toolbar buttons
         binding.profileButton.setOnClickListener { openSettings() }
         binding.searchViewContainer.setOnClickListener {
-            globalSearchViewModel.refresh()
-            binding.globalSearchInputLayout.requestFocus()
+            homeViewModel.onSearchClicked()
         }
         binding.sessionToolbar.disableClipping()
         // Set up seed reminder view
@@ -177,6 +176,7 @@ class HomeActivity : ScreenLockActionBarActivity(),
                 if (!textSecurePreferences.getHasViewedSeed()) SeedReminder { start<RecoveryPasswordActivity>() }
             }
         }
+
         // Set up recycler view
         binding.globalSearchInputLayout.listener = this
         homeAdapter.setHasStableIds(true)
@@ -251,11 +251,12 @@ class HomeActivity : ScreenLockActionBarActivity(),
                 }
             }
 
-            // monitor the global search VM query
+            // sync view -> viewModel
             launch {
-                binding.globalSearchInputLayout.query
+                binding.globalSearchInputLayout.query()
                     .collect(globalSearchViewModel::setQuery)
             }
+
             // Get group results and display them
             launch {
                 globalSearchViewModel.result.map { result ->
@@ -308,6 +309,17 @@ class HomeActivity : ScreenLockActionBarActivity(),
                 }
             }
         }
+
+        // Set up search layout
+        lifecycleScope.launch {
+            homeViewModel.isSearchOpen.collect { open ->
+                setSearchShown(open)
+            }
+        }
+    }
+
+    override fun onCancelClicked() {
+        homeViewModel.onCancelSearchClicked()
     }
 
     private val GlobalSearchResult.groupedContacts: List<GlobalSearchAdapter.Model> get() {
@@ -358,11 +370,12 @@ class HomeActivity : ScreenLockActionBarActivity(),
         }
     }
 
-    override fun onInputFocusChanged(hasFocus: Boolean) {
-        setSearchShown(hasFocus || binding.globalSearchInputLayout.query.value.isNotEmpty())
-    }
-
     private fun setSearchShown(isShown: Boolean) {
+        // Request focus immediately so the user can start typing
+        if (isShown) {
+            binding.globalSearchInputLayout.requestFocus()
+        }
+
         binding.searchToolbar.isVisible = isShown
         binding.sessionToolbar.isVisible = !isShown
         binding.recyclerView.isVisible = !isShown
@@ -387,11 +400,6 @@ class HomeActivity : ScreenLockActionBarActivity(),
             binding.seedReminderView.isVisible = false
         }
 
-        // refresh search on resume, in case we a conversation was deleted
-        if (binding.globalSearchRecycler.isVisible){
-            globalSearchViewModel.refresh()
-        }
-
         updateLegacyConfigView()
     }
 
@@ -437,8 +445,13 @@ class HomeActivity : ScreenLockActionBarActivity(),
     // region Interaction
     @Deprecated("Deprecated in Java")
     override fun onBackPressed() {
-        if (binding.globalSearchRecycler.isVisible) binding.globalSearchInputLayout.clearSearch(true)
-        else super.onBackPressed()
+        if (homeViewModel.isSearchOpen.value && binding.globalSearchInputLayout.handleBackPressed()) {
+            return
+        }
+
+        if (!homeViewModel.onBackPressed()) {
+            super.onBackPressed()
+        }
     }
 
     override fun onConversationClick(thread: ThreadRecord) {

app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt

@@ -13,6 +13,7 @@ import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.channels.BufferOverflow
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
@@ -59,6 +60,10 @@ class HomeViewModel @Inject constructor(
             onBufferOverflow = BufferOverflow.DROP_OLDEST
     )
 
+    private val mutableIsSearchOpen = MutableStateFlow(false)
+
+    val isSearchOpen: StateFlow<Boolean> get() = mutableIsSearchOpen
+
     val callBanner: StateFlow<String?> = callManager.currentConnectionStateFlow.map {
         // a call is in progress if it isn't idle nor disconnected
         if(it !is State.Idle && it !is State.Disconnected){
@@ -145,6 +150,23 @@ class HomeViewModel @Inject constructor(
 
     fun tryReload() = manualReloadTrigger.tryEmit(Unit)
 
+    fun onSearchClicked() {
+        mutableIsSearchOpen.value = true
+    }
+
+    fun onCancelSearchClicked() {
+        mutableIsSearchOpen.value = false
+    }
+
+    fun onBackPressed(): Boolean {
+        if (mutableIsSearchOpen.value) {
+            mutableIsSearchOpen.value = false
+            return true
+        }
+
+        return false
+    }
+
     data class Data(
         val items: List<Item>,
     )

app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchInputLayout.kt

@@ -13,8 +13,12 @@ import android.view.inputmethod.EditorInfo
 import android.view.inputmethod.InputMethodManager
 import android.widget.LinearLayout
 import android.widget.TextView
+import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.channelFlow
+import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.flow.stateIn
 import network.loki.messenger.databinding.ViewGlobalSearchInputBinding
 import org.thoughtcrime.securesms.util.SimpleTextWatcher
 import org.thoughtcrime.securesms.util.addTextChangedListener
@@ -29,17 +33,31 @@ class GlobalSearchInputLayout @JvmOverloads constructor(
 
     var listener: GlobalSearchInputLayoutListener? = null
 
-    private val _query = MutableStateFlow<CharSequence>("")
-    val query: StateFlow<CharSequence> = _query
+    fun query() = channelFlow {
+        val watcher = object : SimpleTextWatcher() {
+            override fun onTextChanged(text: String?) {
+                trySend(text.orEmpty())
+            }
+        }
+
+        send(binding.searchInput.text.toString())
+        binding.searchInput.addTextChangedListener(watcher)
+
+        awaitClose {
+            binding.searchInput.removeTextChangedListener(watcher)
+        }
+    }
 
     override fun onAttachedToWindow() {
         super.onAttachedToWindow()
         binding.searchInput.onFocusChangeListener = this
-        binding.searchInput.addTextChangedListener(::setQuery)
         binding.searchInput.setOnEditorActionListener(this)
         binding.searchInput.filters = arrayOf<InputFilter>(LengthFilter(100)) // 100 char search limit
-        binding.searchCancel.setOnClickListener { clearSearch(true) }
-        binding.searchClear.setOnClickListener { clearSearch(false) }
+        binding.searchCancel.setOnClickListener {
+            clearSearch()
+            listener?.onCancelClicked()
+        }
+        binding.searchClear.setOnClickListener { clearSearch() }
     }
 
     override fun onFocusChange(v: View?, hasFocus: Boolean) {
@@ -48,7 +66,6 @@ class GlobalSearchInputLayout @JvmOverloads constructor(
                 if (hasFocus) showSoftInput(v, 0)
                 else hideSoftInputFromWindow(windowToken, 0)
             }
-            listener?.onInputFocusChanged(hasFocus)
         }
     }
 
@@ -60,20 +77,21 @@ class GlobalSearchInputLayout @JvmOverloads constructor(
         return false
     }
 
-    fun clearSearch(clearFocus: Boolean) {
+    fun clearSearch() {
         binding.searchInput.text = null
-        setQuery("")
-        if (clearFocus) {
-            binding.searchInput.clearFocus()
-        }
     }
 
-    private fun setQuery(query: String) {
-        _query.value = query
+    fun handleBackPressed(): Boolean {
+        if (binding.searchInput.length() > 0) {
+            clearSearch()
+            return true
+        }
+
+        return false
     }
 
     interface GlobalSearchInputLayoutListener {
-        fun onInputFocusChanged(hasFocus: Boolean)
+        fun onCancelClicked()
     }
 
 }

app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchViewModel.kt

@@ -1,5 +1,6 @@
 package org.thoughtcrime.securesms.home.search
 
+import android.app.Application
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
@@ -11,18 +12,28 @@ import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.WhileSubscribed
 import kotlinx.coroutines.flow.buffer
+import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapLatest
 import kotlinx.coroutines.flow.merge
+import kotlinx.coroutines.flow.onStart
+import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.plus
 import kotlinx.coroutines.withContext
+import org.session.libsignal.utilities.Log
+import org.thoughtcrime.securesms.database.DatabaseContentProviders
+import org.thoughtcrime.securesms.dependencies.ConfigFactory
 import org.thoughtcrime.securesms.search.SearchRepository
 import org.thoughtcrime.securesms.search.model.SearchResult
+import org.thoughtcrime.securesms.util.observeChanges
 import javax.inject.Inject
 import kotlin.coroutines.resume
 import kotlin.coroutines.suspendCoroutine
@@ -30,45 +41,49 @@ import kotlin.coroutines.suspendCoroutine
 @OptIn(ExperimentalCoroutinesApi::class)
 @HiltViewModel
 class GlobalSearchViewModel @Inject constructor(
+    private val application: Application,
     private val searchRepository: SearchRepository,
+    private val configFactory: ConfigFactory,
 ) : ViewModel() {
-    private val scope = viewModelScope + SupervisorJob()
-    private val refreshes = MutableSharedFlow<Unit>()
-    private val _queryText = MutableStateFlow<CharSequence>("")
 
-    val result = _queryText
-        .reEmit(refreshes)
-        .buffer(onBufferOverflow = BufferOverflow.DROP_OLDEST)
+    // The query text here is not the source of truth due to the limitation of Android view system
+    // Currently it's only set by the user input: if you try to set it programmatically, it won't
+    // be reflected in the UI and could be overwritten by the user input.
+    private val _queryText = MutableStateFlow<String>("")
+
+    private fun observeChangesAffectingSearch(): Flow<*> = merge(
+        application.contentResolver.observeChanges(DatabaseContentProviders.ConversationList.CONTENT_URI),
+        configFactory.configUpdateNotifications
+    )
+
+    val result = combine(
+        _queryText,
+        observeChangesAffectingSearch().onStart { emit(Unit) }
+    ) { query, _ -> query }
+        .debounce(300L)
         .mapLatest { query ->
-            if (query.trim().isEmpty()) {
-                withContext(Dispatchers.Default) {
-                    // searching for 05 as contactDb#getAllContacts was not returning contacts
-                    // without a nickname/name who haven't approved us.
-                    GlobalSearchResult(
-                        query.toString(),
-                        searchRepository.queryContacts("05").first.toList()
-                    )
-                }
-            } else {
-                // User input delay in case we get a new query within a few hundred ms this
-                // coroutine will be cancelled and the expensive query will not be run.
-                delay(300)
-                try {
-                    searchRepository.suspendQuery(query.toString()).toGlobalSearchResult()
-                } catch (e: Exception) {
-                    GlobalSearchResult(query.toString())
+            try {
+                if (query.isBlank()) {
+                    withContext(Dispatchers.Default) {
+                        // searching for 05 as contactDb#getAllContacts was not returning contacts
+                        // without a nickname/name who haven't approved us.
+                        GlobalSearchResult(
+                            query,
+                            searchRepository.queryContacts("05").first.toList()
+                        )
+                    }
+                } else {
+                    searchRepository.suspendQuery(query).toGlobalSearchResult()
                 }
+            } catch (e: Exception) {
+                Log.e("GlobalSearchViewModel", "Error searching len = ${query.length}", e)
+                GlobalSearchResult(query)
             }
         }
+        .shareIn(viewModelScope, SharingStarted.WhileSubscribed(), 0)
 
     fun setQuery(charSequence: CharSequence) {
-        _queryText.value = charSequence
-    }
-
-    fun refresh() {
-        viewModelScope.launch {
-            refreshes.emit(Unit)
-        }
+        _queryText.value = charSequence.toString()
     }
 }
 
@@ -77,9 +92,3 @@ private suspend fun SearchRepository.suspendQuery(query: String): SearchResult {
         query(query, cont::resume)
     }
 }
-
-/**
- * Re-emit whenever refreshes emits.
- * */
-@OptIn(ExperimentalCoroutinesApi::class)
-private fun <T> Flow<T>.reEmit(refreshes: Flow<Unit>) = flatMapLatest { query -> merge(flowOf(query), refreshes.map { query }) }