For #5567: Removes search shortcuts button (#5739)

* For #5567: Removes search shortcut button

* No issue: Simplifies logic for displaying shortcuts

app/metrics.yaml

@@ -133,28 +133,6 @@ events:
     expires: "2020-03-01"
 
 search_shortcuts:
-  opened:
-    type: event
-    description: >
-      A user opened the search shortcut menu in the search view by pressing the shortcuts button
-    bugs:
-      - 793
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
-    notification_emails:
-      - fenix-core@mozilla.com
-    expires: "2020-03-01"
-  closed:
-    type: event
-    description: >
-      A user closed the search shortcut menu in the search view by pressing the shortcuts button
-    bugs:
-      - 793
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
-    notification_emails:
-      - fenix-core@mozilla.com
-    expires: "2020-03-01"
   selected:
     type: event
     description: >

app/src/androidTest/java/org/mozilla/fenix/ui/SearchTest.kt

@@ -32,7 +32,6 @@ class SearchTest {
             verifySearchView()
             verifyBrowserToolbar()
             verifyScanButton()
-            verifyShortcutsButton()
         }
     }
 

app/src/androidTest/java/org/mozilla/fenix/ui/robots/SearchRobot.kt

@@ -37,7 +37,6 @@ class SearchRobot {
     fun verifySearchView() = assertSearchView()
     fun verifyBrowserToolbar() = assertBrowserToolbarEditView()
     fun verifyScanButton() = assertScanButton()
-    fun verifyShortcutsButton() = assertShortcutsButton()
     fun verifySearchWithText() = assertSearchWithText()
     fun verifyDuckDuckGoResults() = assertDuckDuckGoResults()
     fun verifyDuckDuckGoURL() = assertDuckDuckGoURL()
@@ -56,10 +55,6 @@ class SearchRobot {
         allowPermissionButton().click()
     }
 
-    fun clickShortcutsButton() {
-        shortcutsButton().perform(click())
-    }
-
     fun typeSearch(searchTerm: String) {
         browserToolbarEditView().perform(typeText(searchTerm))
     }
@@ -124,11 +119,6 @@ private fun scanButton(): ViewInteraction {
     return onView(allOf(withId(R.id.searchScanButton)))
 }
 
-private fun shortcutsButton(): ViewInteraction {
-    mDevice.wait(Until.findObjects(By.res("R.id.search_shortcuts_button")), TestAssetHelper.waitingTime)
-    return onView(withId(R.id.searchShortcutsButton))
-}
-
 private fun clearButton() = onView(withId(R.id.mozac_browser_toolbar_clear_view))
 
 private fun assertDuckDuckGoURL() {
@@ -153,10 +143,6 @@ private fun assertScanButton() =
     onView(allOf(withText("Scan")))
         .check(matches(ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))
 
-private fun assertShortcutsButton() =
-    onView(allOf(withText("Shortcuts")))
-        .check(matches(ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))
-
 private fun assertSearchWithText() =
     onView(allOf(withText("SEARCH WITH")))
         .check(matches(ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

@@ -84,12 +84,6 @@ private val Event.wrapper: EventWrapper<*>?
             },
             { Events.performedSearchKeys.valueOf(it) }
         )
-        is Event.SearchShortcutMenuOpened -> EventWrapper<NoExtraKeys>(
-            { SearchShortcuts.opened.record(it) }
-        )
-        is Event.SearchShortcutMenuClosed -> EventWrapper<NoExtraKeys>(
-            { SearchShortcuts.closed.record(it) }
-        )
         is Event.SearchShortcutSelected -> EventWrapper(
             { SearchShortcuts.selected.record(it) },
             { SearchShortcuts.selectedKeys.valueOf(it) }

app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt

@@ -36,8 +36,6 @@ sealed class Event {
     object InteractWithSearchURLArea : Event()
     object DismissedOnboarding : Event()
     object ClearedPrivateData : Event()
-    object SearchShortcutMenuOpened : Event()
-    object SearchShortcutMenuClosed : Event()
     object AddBookmark : Event()
     object RemoveBookmark : Event()
     object OpenedBookmark : Event()

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -251,8 +251,7 @@ class HomeFragment : Fragment() {
             invokePendingDeleteJobs()
             onboarding.finish()
             val directions = HomeFragmentDirections.actionHomeFragmentToSearchFragment(
-                sessionId = null,
-                showShortcutEnginePicker = true
+                sessionId = null
             )
             val extras =
                 FragmentNavigator.Extras.Builder()
@@ -265,8 +264,7 @@ class HomeFragment : Fragment() {
         view.add_tab_button.setOnClickListener {
             invokePendingDeleteJobs()
             val directions = HomeFragmentDirections.actionHomeFragmentToSearchFragment(
-                sessionId = null,
-                showShortcutEnginePicker = true
+                sessionId = null
             )
             nav(R.id.homeFragment, directions)
         }

app/src/main/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessor.kt

@@ -33,8 +33,7 @@ class StartSearchIntentProcessor(
             out.removeExtra(HomeActivity.OPEN_TO_SEARCH)
 
             val directions = NavGraphDirections.actionGlobalSearch(
-                sessionId = null,
-                showShortcutEnginePicker = true
+                sessionId = null
             )
             navController.nav(null, directions)
             true

app/src/main/java/org/mozilla/fenix/search/SearchController.kt

@@ -18,6 +18,7 @@ import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.searchEngineManager
+import org.mozilla.fenix.ext.settings
 
 /**
  * An interface that handles the view manipulation of the Search, triggered by the Interactor
@@ -30,7 +31,6 @@ interface SearchController {
     fun handleSearchTermsTapped(searchTerms: String)
     fun handleSearchShortcutEngineSelected(searchEngine: SearchEngine)
     fun handleClickSearchEngineSettings()
-    fun handleTurnOnStartedTyping()
     fun handleExistingSessionSelected(session: Session)
 }
 
@@ -40,10 +40,6 @@ class DefaultSearchController(
     private val navController: NavController
 ) : SearchController {
 
-    data class UserTypingCheck(var ranOnTextChanged: Boolean, var userHasTyped: Boolean)
-
-    internal val userTypingCheck = UserTypingCheck(false, !store.state.showShortcutEnginePicker)
-
     override fun handleUrlCommitted(url: String) {
         if (url.isNotBlank()) {
             (context as HomeActivity).openToBrowserAndLoad(
@@ -69,13 +65,9 @@ class DefaultSearchController(
 
     override fun handleTextChanged(text: String) {
         store.dispatch(SearchFragmentAction.UpdateQuery(text))
-
-        if (userTypingCheck.ranOnTextChanged && !userTypingCheck.userHasTyped) {
-            store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(false))
-            handleTurnOnStartedTyping()
-        }
-
-        userTypingCheck.ranOnTextChanged = true
+        store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(
+            text.isEmpty() && context.settings().shouldShowSearchShortcuts
+        ))
     }
 
     override fun handleUrlTapped(url: String) {
@@ -111,11 +103,6 @@ class DefaultSearchController(
         navController.navigate(directions)
     }
 
-    override fun handleTurnOnStartedTyping() {
-        userTypingCheck.ranOnTextChanged = true
-        userTypingCheck.userHasTyped = true
-    }
-
     override fun handleExistingSessionSelected(session: Session) {
         val directions = SearchFragmentDirections.actionSearchFragmentToBrowserFragment(null)
         navController.nav(R.id.searchFragment, directions)

app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt

@@ -38,7 +38,6 @@ import org.mozilla.fenix.R
 import org.mozilla.fenix.components.StoreProvider
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.components
-import org.mozilla.fenix.ext.getColorFromAttr
 import org.mozilla.fenix.ext.getSpannable
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.settings
@@ -79,10 +78,6 @@ class SearchFragment : Fragment(), BackHandler {
             ?.let(SearchFragmentArgs.Companion::fromBundle)
             ?.let { it.pastedText }
 
-        val displayShortcutEnginePicker = arguments
-            ?.let(SearchFragmentArgs.Companion::fromBundle)
-            ?.let { it.showShortcutEnginePicker } ?: false
-
         val view = inflater.inflate(R.layout.fragment_search, container, false)
         val url = session?.url.orEmpty()
         val currentSearchEngine = SearchEngineSource.Default(
@@ -93,10 +88,10 @@ class SearchFragment : Fragment(), BackHandler {
             SearchFragmentStore(
                 SearchFragmentState(
                     query = url,
-                    showShortcutEnginePicker = displayShortcutEnginePicker,
                     searchEngineSource = currentSearchEngine,
                     defaultEngineSource = currentSearchEngine,
                     showSearchSuggestions = requireContext().settings().shouldShowSearchSuggestions,
+                    showSearchShortcuts = requireContext().settings().shouldShowSearchShortcuts && url.isEmpty(),
                     showClipboardSuggestions = requireContext().settings().shouldShowClipboardSuggestions,
                     showHistorySuggestions = requireContext().settings().shouldShowHistorySuggestions,
                     showBookmarkSuggestions = requireContext().settings().shouldShowBookmarkSuggestions,
@@ -188,19 +183,6 @@ class SearchFragment : Fragment(), BackHandler {
 
         view.toolbar_wrapper.clipToOutline = false
 
-        searchShortcutsButton.setOnClickListener {
-            val isOpen = searchStore.state.showShortcutEnginePicker
-            searchStore.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(!isOpen))
-
-            if (isOpen) {
-                requireComponents.analytics.metrics.track(Event.SearchShortcutMenuClosed)
-            } else {
-                requireComponents.analytics.metrics.track(Event.SearchShortcutMenuOpened)
-            }
-
-            searchInteractor.turnOnStartedTyping()
-        }
-
         fill_link_from_clipboard.setOnClickListener {
             (activity as HomeActivity)
                 .openToBrowserAndLoad(
@@ -214,7 +196,6 @@ class SearchFragment : Fragment(), BackHandler {
             awesomeBarView.update(it)
             toolbarView.update(it)
             updateSearchEngineIcon(it)
-            updateSearchShortuctsIcon(it)
             updateSearchWithLabel(it)
             updateClipboardSuggestion(it, requireContext().components.clipboardHandler.url)
         }
@@ -276,30 +257,19 @@ class SearchFragment : Fragment(), BackHandler {
 
     private fun updateSearchWithLabel(searchState: SearchFragmentState) {
         search_with_shortcuts.visibility =
-            if (searchState.showShortcutEnginePicker) View.VISIBLE else View.GONE
+            if (searchState.showSearchShortcuts) View.VISIBLE else View.GONE
     }
 
     private fun updateClipboardSuggestion(searchState: SearchFragmentState, clipboardUrl: String?) {
-        val shouldBeVisible =
+        val visibility =
             if (searchState.showClipboardSuggestions && searchState.query.isEmpty() && !clipboardUrl.isNullOrEmpty())
             View.VISIBLE else View.GONE
 
-        fill_link_from_clipboard.visibility = shouldBeVisible
-        divider_line.visibility = shouldBeVisible
+        fill_link_from_clipboard.visibility = visibility
+        divider_line.visibility = visibility
         clipboard_url.text = clipboardUrl
     }
 
-    private fun updateSearchShortuctsIcon(searchState: SearchFragmentState) {
-        with(requireContext()) {
-            val showShortcuts = searchState.showShortcutEnginePicker
-            searchShortcutsButton?.isChecked = showShortcuts
-
-            val color = if (showShortcuts) R.attr.contrastText else R.attr.primaryText
-
-            searchShortcutsButton.compoundDrawables[0]?.setTint(getColorFromAttr(color))
-        }
-    }
-
     override fun onRequestPermissionsResult(
         requestCode: Int,
         permissions: Array<String>,

app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt

@@ -33,10 +33,10 @@ sealed class SearchEngineSource {
 /**
  * The state for the Search Screen
  * @property query The current search query string
- * @property showShortcutEnginePicker Whether or not to show the available search engine view
  * @property searchEngineSource The current selected search engine with the context of how it was selected
  * @property defaultEngineSource The current default search engine source
  * @property showSearchSuggestions Whether or not to show search suggestions from the search engine in the AwesomeBar
+ * @property showSearchShortcuts Whether or not to show search shortcuts in the AwesomeBar
  * @property showClipboardSuggestions Whether or not to show clipboard suggestion in the AwesomeBar
  * @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
@@ -45,10 +45,10 @@ sealed class SearchEngineSource {
  */
 data class SearchFragmentState(
     val query: String,
-    val showShortcutEnginePicker: Boolean,
     val searchEngineSource: SearchEngineSource,
     val defaultEngineSource: SearchEngineSource.Default,
     val showSearchSuggestions: Boolean,
+    val showSearchShortcuts: Boolean,
     val showClipboardSuggestions: Boolean,
     val showHistorySuggestions: Boolean,
     val showBookmarkSuggestions: Boolean,
@@ -74,16 +74,15 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
         is SearchFragmentAction.SearchShortcutEngineSelected ->
             state.copy(
                 searchEngineSource = SearchEngineSource.Shortcut(action.engine),
-                showShortcutEnginePicker = false
+                showSearchShortcuts = false
             )
         is SearchFragmentAction.ShowSearchShortcutEnginePicker ->
-            state.copy(showShortcutEnginePicker = action.show)
+            state.copy(showSearchShortcuts = action.show)
         is SearchFragmentAction.UpdateQuery ->
             state.copy(query = action.query)
         is SearchFragmentAction.SelectNewDefaultSearchEngine ->
             state.copy(
-                searchEngineSource = SearchEngineSource.Default(action.engine),
-                showShortcutEnginePicker = false
+                searchEngineSource = SearchEngineSource.Default(action.engine)
             )
     }
 }

app/src/main/java/org/mozilla/fenix/search/SearchInteractor.kt

@@ -45,10 +45,6 @@ class SearchInteractor(
         searchController.handleClickSearchEngineSettings()
     }
 
-    fun turnOnStartedTyping() {
-        searchController.handleTurnOnStartedTyping()
-    }
-
     override fun onExistingSessionSelected(session: Session) {
         searchController.handleExistingSessionSelected(session)
     }

app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt

@@ -166,7 +166,7 @@ class AwesomeBarView(
     fun update(state: SearchFragmentState) {
         view.removeAllProviders()
 
-        if (state.showShortcutEnginePicker) {
+        if (state.showSearchShortcuts) {
             view.addProviders(shortcutsEnginePickerProvider)
         } else {
             if (state.showSearchSuggestions) {

app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt

@@ -16,7 +16,7 @@ import org.mozilla.fenix.settings.SharedPreferenceUpdater
 class SearchEngineFragment : PreferenceFragmentCompat() {
 
     override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
-        setPreferencesFromResource(R.xml.search_engine_preferences, rootKey)
+        setPreferencesFromResource(R.xml.search_preferences, rootKey)
     }
 
     override fun onResume() {
@@ -29,7 +29,10 @@ class SearchEngineFragment : PreferenceFragmentCompat() {
                 isChecked = context.settings().shouldShowSearchSuggestions
             }
 
-        searchSuggestionsPreference?.onPreferenceChangeListener = SharedPreferenceUpdater()
+        val showSearchShortcuts =
+            findPreference<SwitchPreference>(getPreferenceKey(R.string.pref_key_show_search_shortcuts))?.apply {
+                isChecked = context.settings().shouldShowSearchShortcuts
+            }
 
         val showHistorySuggestions =
             findPreference<SwitchPreference>(getPreferenceKey(R.string.pref_key_search_browsing_history))?.apply {
@@ -46,6 +49,8 @@ class SearchEngineFragment : PreferenceFragmentCompat() {
                 isChecked = context.settings().shouldShowClipboardSuggestions
             }
 
+        searchSuggestionsPreference?.onPreferenceChangeListener = SharedPreferenceUpdater()
+        showSearchShortcuts?.onPreferenceChangeListener = SharedPreferenceUpdater()
         showHistorySuggestions?.onPreferenceChangeListener = SharedPreferenceUpdater()
         showBookmarkSuggestions?.onPreferenceChangeListener = SharedPreferenceUpdater()
         showClipboardSuggestions?.onPreferenceChangeListener = SharedPreferenceUpdater()

app/src/main/java/org/mozilla/fenix/utils/Settings.kt

@@ -163,6 +163,11 @@ class Settings private constructor(
         default = true
     )
 
+    val shouldShowSearchShortcuts by booleanPreference(
+        appContext.getPreferenceKey(R.string.pref_key_show_search_shortcuts),
+        default = true
+    )
+
     val shouldUseDarkTheme by booleanPreference(
         appContext.getPreferenceKey(R.string.pref_key_dark_theme),
         default = false