From 2be4d08228fc6e3a2c7df0a60a93d6a330aacac2 Mon Sep 17 00:00:00 2001 From: Mihai Branescu Date: Fri, 18 Oct 2019 14:48:58 +0300 Subject: [PATCH 1/3] For #6018 - Added back shortcuts button in awesomebar Refactored logic for adding providers, since shortcut providers should be alone in the list, while all others can be as list --- .../mozilla/fenix/search/SearchController.kt | 6 +++ .../mozilla/fenix/search/SearchInteractor.kt | 4 ++ .../fenix/search/awesomebar/AwesomeBarView.kt | 51 +++++++++++++++---- app/src/main/res/layout/fragment_search.xml | 7 +++ app/src/main/res/values/strings.xml | 2 + 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchController.kt b/app/src/main/java/org/mozilla/fenix/search/SearchController.kt index 0717a5bd0..8615dcfa1 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchController.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchController.kt @@ -32,6 +32,7 @@ interface SearchController { fun handleSearchShortcutEngineSelected(searchEngine: SearchEngine) fun handleClickSearchEngineSettings() fun handleExistingSessionSelected(session: Session) + fun handleSearchShortcutsButtonClicked() } class DefaultSearchController( @@ -98,6 +99,11 @@ class DefaultSearchController( context.metrics.track(Event.SearchShortcutSelected(searchEngine.name)) } + override fun handleSearchShortcutsButtonClicked() { + val isOpen = store.state.showSearchShortcuts + store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(!isOpen)) + } + override fun handleClickSearchEngineSettings() { val directions = SearchFragmentDirections.actionSearchFragmentToSearchEngineFragment() navController.navigate(directions) diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchInteractor.kt b/app/src/main/java/org/mozilla/fenix/search/SearchInteractor.kt index 2e7e8b96e..e9ffc74ca 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchInteractor.kt @@ -41,6 +41,10 @@ class SearchInteractor( searchController.handleSearchShortcutEngineSelected(searchEngine) } + override fun onSearchShortcutsButtonClicked() { + searchController.handleSearchShortcutsButtonClicked() + } + override fun onClickSearchEngineSettings() { searchController.handleClickSearchEngineSettings() } diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt index bcb0f0630..99acc9bcc 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt @@ -11,6 +11,7 @@ import android.view.View import android.view.ViewGroup import androidx.core.graphics.drawable.toBitmap import kotlinx.android.extensions.LayoutContainer +import kotlinx.android.synthetic.main.fragment_search.* import mozilla.components.browser.awesomebar.BrowserAwesomeBar import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session @@ -64,6 +65,11 @@ interface AwesomeBarInteractor { * Called whenever an existing session is selected from the sessionSuggestionProvider */ fun onExistingSessionSelected(session: Session) + + /** + * Called whenever the Shortcuts button is clicked + */ + fun onSearchShortcutsButtonClicked() } /** @@ -164,10 +170,16 @@ class AwesomeBarView( interactor::onClickSearchEngineSettings ) } + + searchShortcutsButton.setOnClickListener { + interactor.onSearchShortcutsButtonClicked() + } } @SuppressWarnings("ComplexMethod") fun update(state: SearchFragmentState) { + updateSearchShortcutsIcon(state) + // Do not make suggestions based on user's current URL if (state.query == state.session?.url) { return @@ -178,17 +190,29 @@ class AwesomeBarView( view.onInputChanged(state.query) } + private fun updateSearchShortcutsIcon(searchState: SearchFragmentState) { + with(container.context) { + val showShortcuts = searchState.showSearchShortcuts + searchShortcutsButton?.isChecked = showShortcuts + + val color = if (showShortcuts) R.attr.contrastText else R.attr.primaryText + + searchShortcutsButton.compoundDrawables[0]?.setTint(getColorFromAttr(color)) + } + } + @Suppress("ComplexMethod") private fun updateSuggestionProvidersVisibility(state: SearchFragmentState) { val providersToAdd = mutableSetOf() val providersToRemove = mutableSetOf() if (state.showSearchShortcuts) { - providersToAdd.add(shortcutsEnginePickerProvider) - } else { - providersToRemove.add(shortcutsEnginePickerProvider) + handleDisplayShortcutsProviders() + return } + providersToRemove.add(shortcutsEnginePickerProvider) + if (state.showHistorySuggestions) { providersToAdd.add(historyStorageProvider) } else { @@ -211,12 +235,14 @@ class AwesomeBarView( } ) } else { - providersToRemove.add(when (state.searchEngineSource) { - is SearchEngineSource.Default -> defaultSearchSuggestionProvider - is SearchEngineSource.Shortcut -> createSuggestionProviderForEngine( - state.searchEngineSource.searchEngine - ) - }) + providersToRemove.add( + when (state.searchEngineSource) { + is SearchEngineSource.Default -> defaultSearchSuggestionProvider + is SearchEngineSource.Shortcut -> createSuggestionProviderForEngine( + state.searchEngineSource.searchEngine + ) + } + ) } if ((container.context.asActivity() as? HomeActivity)?.browsingModeManager?.mode?.isPrivate == false) { @@ -240,6 +266,13 @@ class AwesomeBarView( } } + private fun handleDisplayShortcutsProviders() { + view.removeAllProviders() + providersInUse.clear() + providersInUse.add(shortcutsEnginePickerProvider) + view.addProviders(shortcutsEnginePickerProvider) + } + private fun createSuggestionProviderForEngine(engine: SearchEngine): SearchSuggestionProvider { return with(container.context) { val draw = getDrawable(R.drawable.ic_search) diff --git a/app/src/main/res/layout/fragment_search.xml b/app/src/main/res/layout/fragment_search.xml index c8962df3a..2e9f0d08d 100644 --- a/app/src/main/res/layout/fragment_search.xml +++ b/app/src/main/res/layout/fragment_search.xml @@ -156,5 +156,12 @@ android:drawableStart="@drawable/ic_qr" android:textOff="@string/search_scan_button" android:textOn="@string/search_scan_button" /> + + \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 9985dd3d5..e7969b993 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -95,6 +95,8 @@ Scan + + Shortcuts Search engine settings From be81a14b04ee4b3fe4b0c0c9d0522d2dc99c3946 Mon Sep 17 00:00:00 2001 From: Mihai Branescu Date: Fri, 18 Oct 2019 15:43:39 +0300 Subject: [PATCH 2/3] For #6018 - Added unit tests for interactor and controller --- .../search/DefaultSearchControllerTest.kt | 18 ++++++++++++++++++ .../fenix/search/SearchInteractorTest.kt | 10 ++++++++++ 2 files changed, 28 insertions(+) diff --git a/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt b/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt index 77f7fd1a3..932f84c7c 100644 --- a/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt @@ -204,6 +204,24 @@ class DefaultSearchControllerTest { verify { navController.navigate(directions) } } + @Test + fun handleSearchShortcutsButtonClicked_alreadyOpen() { + every { store.state.showSearchShortcuts } returns true + + controller.handleSearchShortcutsButtonClicked() + + verify { store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(false)) } + } + + @Test + fun handleSearchShortcutsButtonClicked_notYetOpen() { + every { store.state.showSearchShortcuts } returns false + + controller.handleSearchShortcutsButtonClicked() + + verify { store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(true)) } + } + @Test fun handleExistingSessionSelected() { val session: Session = mockk(relaxed = true) diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt index a64d83432..d33c1f860 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt @@ -189,6 +189,16 @@ class SearchInteractorTest { verify { store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine)) } } + @Test + fun onSearchShortcutsButtonClicked() { + val searchController: SearchController = mockk(relaxed = true) + val interactor = SearchInteractor(searchController) + + interactor.onSearchShortcutsButtonClicked() + + verify { searchController.handleSearchShortcutsButtonClicked() } + } + @Test fun onClickSearchEngineSettings() { val navController: NavController = mockk() From 86217eb105085b5cba848409aee68e6b5df74f03 Mon Sep 17 00:00:00 2001 From: mcarare Date: Fri, 18 Oct 2019 17:00:26 +0300 Subject: [PATCH 3/3] For #5872 & #6075: Set TabHeader buttons to invisible instead of gone. At least one button has to be invisible instead of gone to keep layout height. Tabs overflow button kept gone to avoid empty space on view end in private mode. --- .../home/sessioncontrol/viewholders/TabHeaderViewHolder.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt index bab540d29..354af7f1c 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.home.sessioncontrol.viewholders import android.content.Context import android.view.View import android.widget.PopupWindow +import androidx.core.view.isInvisible import androidx.core.view.isVisible import androidx.recyclerview.widget.RecyclerView import io.reactivex.Observer @@ -81,8 +82,8 @@ class TabHeaderViewHolder( val headerTextResourceId = if (isPrivate) R.string.tabs_header_private_title else R.string.tab_header_label view.header_text.text = view.context.getString(headerTextResourceId) - view.share_tabs_button.isVisible = isPrivate && hasTabs - view.close_tabs_button.isVisible = isPrivate && hasTabs + view.share_tabs_button.isInvisible = !isPrivate || !hasTabs + view.close_tabs_button.isInvisible = !isPrivate || !hasTabs view.tabs_overflow_button.isVisible = !isPrivate && hasTabs }