From b239d7d934329edcdbd6d50d900d62ac29462b96 Mon Sep 17 00:00:00 2001 From: Sebastian Kaspari Date: Thu, 16 Sep 2021 18:46:21 +0200 Subject: [PATCH] [fenix] Issue https://github.com/mozilla-mobile/fenix/issues/21291: SearchDialogFragment: Get URL from clipboard once and not for every state update --- .../fenix/search/SearchDialogFragment.kt | 29 +++++++++++++++---- .../fenix/search/SearchFragmentStore.kt | 10 ++++++- .../fenix/search/SearchFragmentStoreTest.kt | 18 ++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index 3169efc7ae..a087704eac 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -44,6 +44,7 @@ import mozilla.components.concept.storage.HistoryStorage import mozilla.components.feature.qr.QrFeature import mozilla.components.lib.state.ext.consumeFlow import mozilla.components.lib.state.ext.consumeFrom +import mozilla.components.support.base.coroutines.Dispatchers import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import mozilla.components.support.ktx.android.content.getColorFromAttr @@ -349,7 +350,7 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { if (it.url != it.query) firstUpdate = false binding.awesomeBar.visibility = if (shouldShowAwesomebar(it)) View.VISIBLE else View.INVISIBLE updateSearchSuggestionsHintVisibility(it) - updateClipboardSuggestion(it, requireContext().components.clipboardHandler.url) + updateClipboardSuggestion(it) updateToolbarContentDescription(it) updateSearchShortcutsIcon(it) toolbarView.update(it) @@ -374,6 +375,22 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { } } + override fun onResume() { + super.onResume() + + view?.post { + // We delay querying the clipboard by posting this code to the main thread message queue, + // because ClipboardManager will return null if the does app not have input focus yet. + lifecycleScope.launch(Dispatchers.Cached) { + store.dispatch( + SearchFragmentAction.UpdateClipboardUrl( + requireContext().components.clipboardHandler.url + ) + ) + } + } + } + override fun onPause() { super.onPause() view?.hideKeyboard() @@ -585,10 +602,10 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { private fun isSpeechAvailable(): Boolean = speechIntent.resolveActivity(requireContext().packageManager) != null - private fun updateClipboardSuggestion(searchState: SearchFragmentState, clipboardUrl: String?) { + private fun updateClipboardSuggestion(searchState: SearchFragmentState) { val shouldShowView = searchState.showClipboardSuggestions && searchState.query.isEmpty() && - !clipboardUrl.isNullOrEmpty() && !searchState.showSearchShortcuts + !searchState.clipboardUrl.isNullOrEmpty() && !searchState.showSearchShortcuts binding.fillLinkFromClipboard.isVisible = shouldShowView binding.fillLinkDivider.isVisible = shouldShowView @@ -598,13 +615,13 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { binding.clipboardTitle.isVisible = shouldShowView binding.linkIcon.isVisible = shouldShowView - binding.clipboardUrl.text = clipboardUrl + binding.clipboardUrl.text = searchState.clipboardUrl binding.fillLinkFromClipboard.contentDescription = "${binding.clipboardTitle.text}, ${binding.clipboardUrl.text}." - if (clipboardUrl != null && !((activity as HomeActivity).browsingModeManager.mode.isPrivate)) { - requireComponents.core.engine.speculativeConnect(clipboardUrl) + if (searchState.clipboardUrl != null && !((activity as HomeActivity).browsingModeManager.mode.isPrivate)) { + requireComponents.core.engine.speculativeConnect(searchState.clipboardUrl) } } diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt index 1c6570fe91..0db9b845fa 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt @@ -61,6 +61,7 @@ sealed class SearchEngineSource { * @property showHistorySuggestions Whether or not to show history suggestions in the AwesomeBar * @property showBookmarkSuggestions Whether or not to show the bookmark suggestion in the AwesomeBar * @property pastedText The text pasted from the long press toolbar menu + * @property clipboardUrl The URL in the clipboard of the user - if there's any; otherwise `null`. */ data class SearchFragmentState( val query: String, @@ -79,7 +80,8 @@ data class SearchFragmentState( val showSyncedTabsSuggestions: Boolean, val tabId: String?, val pastedText: String? = null, - val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint? + val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint?, + val clipboardUrl: String? = null ) : State fun createInitialSearchFragmentState( @@ -129,6 +131,7 @@ sealed class SearchFragmentAction : Action { data class ShowSearchShortcutEnginePicker(val show: Boolean) : SearchFragmentAction() data class AllowSearchSuggestionsInPrivateModePrompt(val show: Boolean) : SearchFragmentAction() data class UpdateQuery(val query: String) : SearchFragmentAction() + data class UpdateClipboardUrl(val url: String?) : SearchFragmentAction() /** * Updates the local `SearchFragmentState` from the global `SearchState` in `BrowserStore`. @@ -169,5 +172,10 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen } ) } + is SearchFragmentAction.UpdateClipboardUrl -> { + state.copy( + clipboardUrl = action.url + ) + } } } diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt index 43245dafa1..5aa34c6436 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt @@ -16,6 +16,7 @@ import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ContentState import mozilla.components.browser.state.state.SearchState import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.support.test.ext.joinBlocking import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull @@ -199,6 +200,23 @@ class SearchFragmentStoreTest { assertFalse(store.state.showSearchSuggestionsHint) } + @Test + fun updatingClipboardUrl() { + val initialState = emptyDefaultState() + val store = SearchFragmentStore(initialState) + + assertNull(store.state.clipboardUrl) + + store.dispatch( + SearchFragmentAction.UpdateClipboardUrl("https://www.mozilla.org") + ).joinBlocking() + + assertEquals( + "https://www.mozilla.org", + store.state.clipboardUrl + ) + } + @Test fun `Updating SearchFragmentState from SearchState`() = runBlocking { val store = SearchFragmentStore(