Merge #6102 #6106

6102: For #6018 Re-add Search Shortcuts Button r=boek a=BranescuMihai



6106: For #5872 & #6075: Set TabHeader buttons to invisible instead of gone. r=boek a=mcarare

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.



Co-authored-by: Mihai Branescu <branescu.mihai2@gmail.com>
Co-authored-by: mcarare <mihai.carare.dev@gmail.com>

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.share_tabs_button.isInvisible = !isPrivate || !hasTabs
-        view.close_tabs_button.isVisible = isPrivate && hasTabs
+        view.close_tabs_button.isInvisible = !isPrivate || !hasTabs
         view.tabs_overflow_button.isVisible = !isPrivate && hasTabs
     }
 

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)

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()
     }

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<AwesomeBar.SuggestionProvider>()
         val providersToRemove = mutableSetOf<AwesomeBar.SuggestionProvider>()
 
         if (state.showSearchShortcuts) {
-            providersToAdd.add(shortcutsEnginePickerProvider)
+            handleDisplayShortcutsProviders()
-        } else {
+            return
-            providersToRemove.add(shortcutsEnginePickerProvider)
         }
 
+        providersToRemove.add(shortcutsEnginePickerProvider)
+
         if (state.showHistorySuggestions) {
             providersToAdd.add(historyStorageProvider)
         } else {
@@ -211,12 +235,14 @@ class AwesomeBarView(
                 }
             )
         } else {
-            providersToRemove.add(when (state.searchEngineSource) {
+            providersToRemove.add(
-                is SearchEngineSource.Default -> defaultSearchSuggestionProvider
+                when (state.searchEngineSource) {
-                is SearchEngineSource.Shortcut -> createSuggestionProviderForEngine(
+                    is SearchEngineSource.Default -> defaultSearchSuggestionProvider
-                    state.searchEngineSource.searchEngine
+                    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)

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" />
+
+        <ToggleButton
+            android:id="@+id/searchShortcutsButton"
+            style="@style/search_pill"
+            android:drawableStart="@drawable/ic_search"
+            android:textOff="@string/search_shortcuts_button"
+            android:textOn="@string/search_shortcuts_button" />
     </LinearLayout>
 </androidx.constraintlayout.widget.ConstraintLayout>

app/src/main/res/values/strings.xml

@@ -95,6 +95,8 @@
     <!-- Search Fragment -->
     <!-- Button in the search view that lets a user search by scanning a QR code -->
     <string name="search_scan_button">Scan</string>
+    <!-- Button in the search view that lets a user search by using a shortcut -->
+    <string name="search_shortcuts_button">Shortcuts</string>
     <!-- Button in the search view when shortcuts are displayed that takes a user to the search engine settings -->
     <string name="search_shortcuts_engine_settings">Search engine settings</string>
     <!-- Header displayed when selecting a shortcut search engine -->

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)

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()