diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 54da14ec38ae..d0e5ef7f8b14 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -955,8 +955,10 @@ class BrowserTabFragment : } private fun onOmnibarClearTextButtonPressed() { - viewModel.onClearOmnibarTextInput() - omnibar.setText("") + if (!changeOmnibarPositionFeature.refactor().isEnabled()) { + viewModel.onClearOmnibarTextInput() + omnibar.setText("") + } } private fun configureCustomTab() { diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt index 68c39bbcd93d..c5c903f7d016 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt @@ -296,11 +296,15 @@ class OmnibarLayout @JvmOverloads constructor( omnibarTextInput.replaceTextChangedListener( object : TextChangedWatcher() { + var clearQuery = false + var deleteLastCharacter = false override fun afterTextChanged(editable: Editable) { if (isAttachedToWindow) { viewModel.onInputStateChanged( omnibarTextInput.text.toString(), omnibarTextInput.hasFocus(), + clearQuery, + deleteLastCharacter, ) } omnibarTextListener?.onOmnibarTextChanged( @@ -310,6 +314,12 @@ class OmnibarLayout @JvmOverloads constructor( ), ) } + + override fun beforeTextChanged(s: CharSequence, start: Int, count: Int, after: Int) { + Timber.d("Omnibar: $count characters beginning at $start are about to be replaced by new text with length $after") + clearQuery = start == 0 && after == 0 + deleteLastCharacter = count == 1 && clearQuery + } }, ) @@ -445,7 +455,6 @@ class OmnibarLayout @JvmOverloads constructor( } private fun renderBrowserMode(viewState: ViewState) { - Timber.d("Omnibar: render browserMode $viewState") renderOutline(viewState.hasFocus) if (viewState.updateOmnibarText) { omnibarTextInput.setText(viewState.omnibarText) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt index e90254972835..17d7a650cdb4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt @@ -23,6 +23,7 @@ import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesViewModel import com.duckduckgo.app.browser.DuckDuckGoUrlDetector import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode +import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.Browser import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.CustomTab import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.Error import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode.NewTab @@ -90,6 +91,7 @@ class OmnibarLayoutViewModel @Inject constructor( val leadingIconState: LeadingIconState = LeadingIconState.SEARCH, val privacyShield: PrivacyShield = PrivacyShield.UNKNOWN, val hasFocus: Boolean = false, + val query: String = "", val omnibarText: String = "", val url: String = "", val expanded: Boolean = false, @@ -171,6 +173,20 @@ class OmnibarLayoutViewModel @Inject constructor( } } else { _viewState.update { + val shouldUpdateOmnibarText = it.viewMode is Browser + Timber.d("Omnibar: lost focus in Browser mode $shouldUpdateOmnibarText") + val omnibarText = if (shouldUpdateOmnibarText) { + if (duckDuckGoUrlDetector.isDuckDuckGoQueryUrl(it.url)) { + Timber.d("Omnibar: is DDG url, showing query ${it.query}") + it.query + } else { + Timber.d("Omnibar: is url, showing URL ${it.url}") + it.url + } + } else { + Timber.d("Omnibar: not browser mode, not changing omnibar text") + it.omnibarText + } it.copy( hasFocus = false, expanded = false, @@ -187,6 +203,8 @@ class OmnibarLayoutViewModel @Inject constructor( urlLoaded = _viewState.value.url, ), shouldMoveCaretToStart = true, + updateOmnibarText = shouldUpdateOmnibarText, + omnibarText = omnibarText, ) } } @@ -386,13 +404,28 @@ class OmnibarLayoutViewModel @Inject constructor( fun onInputStateChanged( query: String, hasFocus: Boolean, + clearQuery: Boolean, + deleteLastCharacter: Boolean, ) { - Timber.d("Omnibar: onInputStateChanged") val showClearButton = hasFocus && query.isNotBlank() val showControls = !hasFocus || query.isBlank() + Timber.d("Omnibar: onInputStateChanged query $query hasFocus $hasFocus clearQuery $clearQuery deleteLastCharacter $deleteLastCharacter") + _viewState.update { + val updatedQuery = if (deleteLastCharacter) { + Timber.d("Omnibar: deleting last character, old query ${it.query} also deleted") + it.url + } else if (clearQuery) { + Timber.d("Omnibar: clearing old query ${it.query}, we keep it as reference") + it.query + } else { + Timber.d("Omnibar: not clearing or deleting old query ${it.query}, updating query to $query") + query + } + it.copy( + query = updatedQuery, omnibarText = query, updateOmnibarText = false, hasFocus = hasFocus, @@ -501,6 +534,7 @@ class OmnibarLayoutViewModel @Inject constructor( } fun onUserTouchedOmnibarTextInput(touchAction: Int) { + Timber.d("Omnibar: onUserTouchedOmnibarTextInput") if (touchAction == ACTION_UP) { firePixelBasedOnCurrentUrl( AppPixelName.ADDRESS_BAR_NEW_TAB_PAGE_CLICKED, @@ -517,6 +551,12 @@ class OmnibarLayoutViewModel @Inject constructor( AppPixelName.ADDRESS_BAR_SERP_CANCELLED, AppPixelName.ADDRESS_BAR_WEBSITE_CANCELLED, ) + _viewState.update { + it.copy( + omnibarText = it.url, + updateOmnibarText = true, + ) + } } fun onEnterKeyPressed() { diff --git a/app/src/test/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModelTest.kt index 89db5b7374e3..efcd28b692d6 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModelTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModelTest.kt @@ -627,7 +627,9 @@ class OmnibarLayoutViewModelTest { fun whenInputStateChangedAndQueryEmptyThenViewStateCorrect() = runTest { val query = "" val hasFocus = true - testee.onInputStateChanged(query, hasFocus) + val clearQuery = true + val deleteLastCharacter = false + testee.onInputStateChanged(query, hasFocus, clearQuery, deleteLastCharacter) testee.viewState.test { val viewState = awaitItem() @@ -645,7 +647,9 @@ class OmnibarLayoutViewModelTest { fun whenInputStateChangedAndQueryNotEmptyThenViewStateCorrect() = runTest { val query = "query" val hasFocus = true - testee.onInputStateChanged(query, hasFocus) + val clearQuery = true + val deleteLastCharacter = false + testee.onInputStateChanged(query, hasFocus, clearQuery, deleteLastCharacter) testee.viewState.test { val viewState = awaitItem() @@ -815,6 +819,92 @@ class OmnibarLayoutViewModelTest { } } + @Test + fun whenOmnibarTextClearedAndBackPressedThenUrlIsShown() = runTest { + givenSiteLoaded(RANDOM_URL) + testee.onClearTextButtonPressed() + testee.onBackKeyPressed() + + testee.viewState.test { + val viewState = awaitItem() + assertTrue(viewState.omnibarText == RANDOM_URL) + } + } + + @Test + fun whenHidingKeyboardAfterClearingInputWhileInSiteThenURLisShown() = runTest { + givenSiteLoaded(RANDOM_URL) + testee.onClearTextButtonPressed() + val hasFocus = true + val clearQuery = true + val deleteLastCharacter = false + testee.onInputStateChanged("", hasFocus, clearQuery, deleteLastCharacter) + testee.onOmnibarFocusChanged(false, "") + testee.onInputStateChanged(RANDOM_URL, false, false, false) + + testee.viewState.test { + val viewState = awaitItem() + assertTrue(viewState.omnibarText == RANDOM_URL) + } + } + + @Test + fun whenHidingKeyboardAfterClearingInputWhileInSERPThenURLisShown() = runTest { + givenSiteLoaded(SERP_URL) + + val hasFocus = true + val clearQuery = true + val deleteLastCharacter = false + + testee.onClearTextButtonPressed() + testee.onInputStateChanged("", hasFocus, clearQuery, deleteLastCharacter) + testee.onOmnibarFocusChanged(false, "") + testee.onInputStateChanged(SERP_URL, false, false, false) + + testee.viewState.test { + val viewState = awaitItem() + assertTrue(viewState.omnibarText == SERP_URL) + } + } + + @Test + fun whenClosingKeyboardAfterDeletingLastCharacterFromOmnibaWhileInSERPThenURLisShown() = runTest { + givenSiteLoaded(SERP_URL) + + val hasFocus = true + val clearQuery = true + val deleteLastCharacter = true + + testee.onClearTextButtonPressed() + testee.onInputStateChanged("", hasFocus, clearQuery, deleteLastCharacter) + testee.onOmnibarFocusChanged(false, "") + testee.onInputStateChanged(SERP_URL, false, false, deleteLastCharacter) + + testee.viewState.test { + val viewState = awaitItem() + assertTrue(viewState.omnibarText == SERP_URL) + } + } + + @Test + fun whenClosingKeyboardAfterDeletingLastCharacterFromOmnibaWhileInSitehenURLisShown() = runTest { + givenSiteLoaded(RANDOM_URL) + + val hasFocus = true + val clearQuery = true + val deleteLastCharacter = true + + testee.onClearTextButtonPressed() + testee.onInputStateChanged("", hasFocus, clearQuery, deleteLastCharacter) + testee.onOmnibarFocusChanged(false, "") + testee.onInputStateChanged(RANDOM_URL, false, false, deleteLastCharacter) + + testee.viewState.test { + val viewState = awaitItem() + assertTrue(viewState.omnibarText == RANDOM_URL) + } + } + private fun givenSiteLoaded(loadedUrl: String) { testee.onViewModeChanged(ViewMode.Browser(loadedUrl)) testee.onExternalStateChange(