From 15aa91bae9886dbe53e3b3f656e8f4a918a41ec6 Mon Sep 17 00:00:00 2001 From: SessionHero01 <180888785+SessionHero01@users.noreply.github.com> Date: Wed, 19 Mar 2025 15:33:23 +1100 Subject: [PATCH] [SES-3536] - Unable to navigate back on search screen (#1031) --- .../securesms/home/HomeActivity.kt | 43 ++++++---- .../securesms/home/HomeViewModel.kt | 22 +++++ .../home/search/GlobalSearchInputLayout.kt | 46 +++++++---- .../home/search/GlobalSearchViewModel.kt | 81 ++++++++++--------- 4 files changed, 127 insertions(+), 65 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeActivity.kt index 4d243bc325..112717a384 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeActivity.kt +++ b/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() } } } + // 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 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) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt index c891813d79..800cce3ce1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeViewModel.kt +++ b/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 get() = mutableIsSearchOpen + val callBanner: StateFlow = 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, ) diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchInputLayout.kt b/app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchInputLayout.kt index 442e6159fd..9ec65983d7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchInputLayout.kt +++ b/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("") - val query: StateFlow = _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(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() } } \ No newline at end of file diff --git a/app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchViewModel.kt index 3e99134506..dd94bf04d7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/home/search/GlobalSearchViewModel.kt +++ b/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() - private val _queryText = MutableStateFlow("") - 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("") + + 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 Flow.reEmit(refreshes: Flow) = flatMapLatest { query -> merge(flowOf(query), refreshes.map { query }) }