Closes #25809: tapping on search engine triggers engine selection

This commit is contained in:
mike a 2022-11-09 11:27:29 -08:00 committed by mergify[bot]
parent 74ff3f4c51
commit 7c4050eb08
16 changed files with 204 additions and 24 deletions

View File

@ -28,6 +28,8 @@ import androidx.constraintlayout.widget.ConstraintSet.PARENT_ID
import androidx.constraintlayout.widget.ConstraintSet.TOP
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.content.ContextCompat
import androidx.core.graphics.drawable.toDrawable
import androidx.core.view.isGone
import androidx.core.view.isVisible
import androidx.core.view.updateLayoutParams
import androidx.fragment.app.Fragment
@ -49,12 +51,17 @@ import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch
import mozilla.components.browser.menu.view.MenuButton
import mozilla.components.browser.state.search.SearchEngine
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.normalTabs
import mozilla.components.browser.state.selector.privateTabs
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.searchEngines
import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.menu.Orientation
import mozilla.components.concept.menu.candidate.DrawableMenuIcon
import mozilla.components.concept.menu.candidate.TextMenuCandidate
import mozilla.components.concept.storage.FrecencyThresholdOption
import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.AuthType
@ -73,6 +80,7 @@ import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
import org.mozilla.fenix.Config
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.HomeScreen
import org.mozilla.fenix.GleanMetrics.UnifiedSearch
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.addons.showSnackBar
@ -114,6 +122,7 @@ import org.mozilla.fenix.nimbus.FxNimbus
import org.mozilla.fenix.onboarding.FenixOnboarding
import org.mozilla.fenix.perf.MarkersFragmentLifecycleCallbacks
import org.mozilla.fenix.perf.runBlockingIncrement
import org.mozilla.fenix.search.toolbar.SearchSelectorMenu
import org.mozilla.fenix.tabstray.TabsTrayAccessPoint
import org.mozilla.fenix.utils.Settings.Companion.TOP_SITES_PROVIDER_MAX_THRESHOLD
import org.mozilla.fenix.utils.ToolbarPopupWindow
@ -142,6 +151,13 @@ class HomeFragment : Fragment() {
ToolbarPosition.TOP -> null
}
private val searchSelectorMenu by lazy {
SearchSelectorMenu(
context = requireContext(),
interactor = sessionControlInteractor,
)
}
private val browsingModeManager get() = (activity as HomeActivity).browsingModeManager
private val collectionStorageObserver = object : TabCollectionStorage.Observer {
@ -330,6 +346,24 @@ class HomeFragment : Fragment() {
)
}
requireContext().settings().showUnifiedSearchFeature.let {
binding.searchSelector.isVisible = it
binding.searchEngineIcon.isGone = it
}
binding.searchSelector.apply {
setOnClickListener {
val orientation = if (context.settings().shouldUseBottomToolbar) {
Orientation.UP
} else {
Orientation.DOWN
}
UnifiedSearch.searchMenuTapped.record(NoExtras())
searchSelectorMenu.menuController.show(anchor = it, orientation = orientation, forceOrientation = true)
}
}
_sessionControlInteractor = SessionControlInteractor(
controller = DefaultSessionControlController(
activity = activity,
@ -594,6 +628,14 @@ class HomeFragment : Fragment() {
}
}
consumeFlow(requireComponents.core.store) { flow ->
flow.map { state -> state.search }
.ifChanged()
.collect { search ->
updateSearchSelectorMenu(search.searchEngines)
}
}
// DO NOT MOVE ANYTHING BELOW THIS addMarker CALL!
requireComponents.core.engine.profiler?.addMarker(
MarkersFragmentLifecycleCallbacks.MARKER_NAME,
@ -602,20 +644,42 @@ class HomeFragment : Fragment() {
)
}
private fun updateSearchSelectorMenu(searchEngines: List<SearchEngine>) {
val searchEngineList = searchEngines
.map {
TextMenuCandidate(
text = it.name,
start = DrawableMenuIcon(
drawable = it.icon.toDrawable(resources),
),
) {
sessionControlInteractor.onMenuItemTapped(SearchSelectorMenu.Item.SearchEngine(it))
}
}
searchSelectorMenu.menuController.submitList(searchSelectorMenu.menuItems(searchEngineList))
}
private fun observeSearchEngineChanges() {
consumeFlow(store) { flow ->
flow.map { state -> state.search.selectedOrDefaultSearchEngine }
.ifChanged()
.collect { searchEngine ->
if (searchEngine != null) {
val name = searchEngine?.name
val icon = searchEngine?.let {
// Changing dimensions doesn't not affect the icon size, not sure what the
// code is doing: https://github.com/mozilla-mobile/fenix/issues/27763
val iconSize =
requireContext().resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size)
val searchIcon =
BitmapDrawable(requireContext().resources, searchEngine.icon)
searchIcon.setBounds(0, 0, iconSize, iconSize)
binding.searchEngineIcon.setImageDrawable(searchIcon)
BitmapDrawable(requireContext().resources, searchEngine.icon).apply {
setBounds(0, 0, iconSize, iconSize)
}
}
if (requireContext().settings().showUnifiedSearchFeature) {
binding.searchSelector.setIcon(icon, name)
} else {
binding.searchEngineIcon.setImageDrawable(null)
binding.searchEngineIcon.setImageDrawable(icon)
}
}
}

View File

@ -40,6 +40,7 @@ import org.mozilla.fenix.GleanMetrics.RecentTabs
import org.mozilla.fenix.GleanMetrics.TopSites
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.BrowserAnimator
import org.mozilla.fenix.browser.BrowserFragmentDirections
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.collections.SaveCollectionStep
@ -57,6 +58,8 @@ import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.home.HomeFragmentDirections
import org.mozilla.fenix.home.Mode
import org.mozilla.fenix.onboarding.WallpaperOnboardingDialogFragment.Companion.THUMBNAILS_SELECTION_COUNT
import org.mozilla.fenix.search.toolbar.SearchSelectorInteractor
import org.mozilla.fenix.search.toolbar.SearchSelectorMenu
import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.settings.SupportUtils.SumoTopic.PRIVATE_BROWSING_MYTHS
import org.mozilla.fenix.utils.Settings
@ -209,6 +212,11 @@ interface SessionControlController {
* @see [SessionControlInteractor.reportSessionMetrics]
*/
fun handleReportSessionMetrics(state: AppState)
/**
* @see [SearchSelectorInteractor.onMenuItemTapped]
*/
fun handleMenuItemTapped(item: SearchSelectorMenu.Item)
}
@Suppress("TooManyFunctions", "LargeClass", "LongParameterList")
@ -660,4 +668,26 @@ class DefaultSessionControlController(
RecentBookmarks.recentBookmarksCount.set(state.recentBookmarks.size.toLong())
}
override fun handleMenuItemTapped(item: SearchSelectorMenu.Item) {
when (item) {
SearchSelectorMenu.Item.SearchSettings -> {
navController.nav(
R.id.homeFragment,
HomeFragmentDirections.actionGlobalSearchEngineFragment(),
)
}
is SearchSelectorMenu.Item.SearchEngine -> {
val directions = HomeFragmentDirections.actionGlobalSearchDialog(
sessionId = null,
searchEngine = item.searchEngine.id,
)
navController.nav(
R.id.homeFragment,
directions,
BrowserAnimator.getToolbarNavOptions(activity),
)
}
}
}
}

View File

@ -27,6 +27,8 @@ import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryGrou
import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryHighlight
import org.mozilla.fenix.home.recentvisits.controller.RecentVisitsController
import org.mozilla.fenix.home.recentvisits.interactor.RecentVisitsInteractor
import org.mozilla.fenix.search.toolbar.SearchSelectorInteractor
import org.mozilla.fenix.search.toolbar.SearchSelectorMenu
import org.mozilla.fenix.wallpapers.WallpaperState
/**
@ -270,7 +272,8 @@ class SessionControlInteractor(
RecentBookmarksInteractor,
RecentVisitsInteractor,
CustomizeHomeIteractor,
PocketStoriesInteractor {
PocketStoriesInteractor,
SearchSelectorInteractor {
override fun onCollectionAddTabTapped(collection: TabCollection) {
controller.handleCollectionAddTabTapped(collection)
@ -485,4 +488,8 @@ class SessionControlInteractor(
override fun onMessageClosedClicked(message: Message) {
controller.handleMessageClosed(message)
}
override fun onMenuItemTapped(item: SearchSelectorMenu.Item) {
controller.handleMenuItemTapped(item)
}
}

View File

@ -32,6 +32,7 @@ import org.mozilla.fenix.components.metrics.MetricsUtils
import org.mozilla.fenix.crashes.CrashListActivity
import org.mozilla.fenix.ext.navigateSafe
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.search.toolbar.SearchSelectorInteractor
import org.mozilla.fenix.search.toolbar.SearchSelectorMenu
import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.utils.Settings
@ -54,7 +55,7 @@ interface SearchController {
fun handleSearchEngineSuggestionClicked(searchEngine: SearchEngine)
/**
* @see [ToolbarInteractor.onMenuItemTapped]
* @see [SearchSelectorInteractor.onMenuItemTapped]
*/
fun handleMenuItemTapped(item: SearchSelectorMenu.Item)
}

View File

@ -178,6 +178,9 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
tabId = args.sessionId,
pastedText = args.pastedText,
searchAccessPoint = args.searchAccessPoint,
searchEngine = requireComponents.core.store.state.search.searchEngines.firstOrNull {
it.id == args.searchEngine
},
),
)

View File

@ -117,12 +117,14 @@ data class SearchFragmentState(
/**
* Creates the initial state for the search fragment.
*/
@Suppress("LongParameterList")
fun createInitialSearchFragmentState(
activity: HomeActivity,
components: Components,
tabId: String?,
pastedText: String?,
searchAccessPoint: MetricsUtils.Source,
searchEngine: SearchEngine? = null,
): SearchFragmentState {
val settings = components.settings
val tab = tabId?.let { components.core.store.state.findTab(it) }
@ -134,11 +136,17 @@ fun createInitialSearchFragmentState(
settings.shouldShowSearchSuggestions && settings.shouldShowSearchSuggestionsInPrivate
}
val searchEngineSource = if (searchEngine != null) {
SearchEngineSource.Shortcut(searchEngine)
} else {
SearchEngineSource.None
}
return SearchFragmentState(
query = url,
url = url,
searchTerms = tab?.content?.searchTerms.orEmpty(),
searchEngineSource = SearchEngineSource.None,
searchEngineSource = searchEngineSource,
defaultEngine = null,
showSearchSuggestions = shouldShowSearchSuggestions,
showSearchSuggestionsHint = false,

View File

@ -8,6 +8,7 @@ import android.content.Context
import android.graphics.drawable.Drawable
import android.util.AttributeSet
import android.view.LayoutInflater
import android.view.ViewGroup
import android.widget.RelativeLayout
import org.mozilla.fenix.databinding.SearchSelectorBinding
@ -21,9 +22,21 @@ internal class SearchSelector @JvmOverloads constructor(
) : RelativeLayout(context, attrs, defStyle) {
private val binding = SearchSelectorBinding.inflate(LayoutInflater.from(context), this)
private var marginTop: Int = 0
fun setIcon(icon: Drawable, contentDescription: String) {
override fun setLayoutParams(params: ViewGroup.LayoutParams?) {
if (params is MarginLayoutParams) {
params.topMargin = marginTop
}
super.setLayoutParams(params)
}
fun setIcon(icon: Drawable?, contentDescription: String?) {
binding.icon.setImageDrawable(icon)
binding.icon.contentDescription = contentDescription
}
fun setTopMargin(margin: Int) {
marginTop = margin
}
}

View File

@ -0,0 +1,19 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.search.toolbar
/**
* Interface for search selector menu. This interface is implemented by objects that want
* to respond to user interaction with items inside [SearchSelectorMenu].
*/
interface SearchSelectorInteractor {
/**
* Called when an user taps on a search selector menu item.
*
* @param item The [SearchSelectorMenu.Item] that was tapped.
*/
fun onMenuItemTapped(item: SearchSelectorMenu.Item)
}

View File

@ -26,7 +26,7 @@ typealias MozSearchEngine = SearchEngine
*/
class SearchSelectorMenu(
private val context: Context,
private val interactor: ToolbarInteractor,
private val interactor: SearchSelectorInteractor,
) {
/**

View File

@ -55,6 +55,7 @@ class SearchSelectorToolbarAction(
menu.menuController.show(anchor = it, orientation = orientation, forceOrientation = true)
}
setTopMargin(resources.getDimensionPixelSize(R.dimen.search_engine_engine_icon_top_margin))
setBackgroundResource(
context.theme.resolveAttribute(android.R.attr.selectableItemBackgroundBorderless),
)

View File

@ -24,7 +24,7 @@ import org.mozilla.fenix.utils.Settings
* Interface for the Toolbar Interactor. This interface is implemented by objects that want
* to respond to user interaction on the [ToolbarView].
*/
interface ToolbarInteractor {
interface ToolbarInteractor : SearchSelectorInteractor {
/**
* Called when a user hits the return key while [ToolbarView] has focus.
@ -45,13 +45,6 @@ interface ToolbarInteractor {
* @param text The current text displayed by [ToolbarView].
*/
fun onTextChanged(text: String)
/**
* Called when an user taps on a search selector menu item.
*
* @param item The [SearchSelectorMenu.Item] that was tapped.
*/
fun onMenuItemTapped(item: SearchSelectorMenu.Item)
}
/**

View File

@ -119,7 +119,7 @@
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent" />
<FrameLayout
<LinearLayout
android:id="@+id/toolbar_wrapper"
android:layout_width="0dp"
android:layout_height="40dp"
@ -140,16 +140,23 @@
android:layout_height="24dp"
android:layout_gravity="start|center_vertical"
android:layout_marginStart="8dp"
android:layout_marginEnd="12dp"
android:clickable="false"
android:focusable="false"
android:importantForAccessibility="no" />
<org.mozilla.fenix.search.toolbar.SearchSelector
android:id="@+id/search_selector"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="start|center_vertical"
android:background="?selectableItemBackgroundBorderless" />
<TextView
android:id="@+id/toolbar"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="start|center_vertical"
android:layout_marginStart="44dp"
android:clickable="false"
android:ellipsize="end"
android:focusable="false"
@ -158,7 +165,7 @@
android:text="@string/search_hint"
android:textColor="?attr/textPrimary"
android:textSize="15sp" />
</FrameLayout>
</LinearLayout>
<androidx.constraintlayout.widget.Barrier
android:id="@+id/accessory_button_barrier"

View File

@ -12,7 +12,6 @@
android:id="@+id/search_selector"
android:layout_width="40dp"
android:layout_height="28dp"
android:layout_marginTop="14dp"
android:layout_marginHorizontal="8dp"
app:cardBackgroundColor="@color/photonWhite"
app:cardCornerRadius="4dp"
@ -30,7 +29,6 @@
android:id="@+id/icon"
android:layout_width="24dp"
android:layout_height="24dp"
android:scaleType="center"
android:layout_gravity="center"
app:shapeAppearanceOverlay="@style/SearchSelectorIconStyle"
app:layout_constraintStart_toStartOf="parent"

View File

@ -214,6 +214,11 @@
android:name="search_access_point"
android:defaultValue="NONE"
app:argType="org.mozilla.fenix.components.metrics.MetricsUtils$Source" />
<argument
android:name="search_engine"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
</dialog>
<fragment

View File

@ -19,6 +19,7 @@
<dimen name="search_bar_search_engine_icon_padding">12dp</dimen>
<dimen name="search_bar_search_icon_margin">28dp</dimen>
<dimen name="search_engine_engine_icon_margin">12dp</dimen>
<dimen name="search_engine_engine_icon_top_margin">14dp</dimen>
<dimen name="search_engine_radio_button_height">48dp</dimen>
<dimen name="search_engine_engine_icon_height">24dp</dimen>
<dimen name="radio_button_padding_horizontal">16dp</dimen>

View File

@ -6,6 +6,7 @@ package org.mozilla.fenix.home
import androidx.navigation.NavController
import androidx.navigation.NavDirections
import androidx.navigation.NavOptions
import io.mockk.Runs
import io.mockk.every
import io.mockk.just
@ -72,6 +73,7 @@ import org.mozilla.fenix.home.recentbookmarks.RecentBookmark
import org.mozilla.fenix.home.recenttabs.RecentTab
import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController
import org.mozilla.fenix.onboarding.WallpaperOnboardingDialogFragment.Companion.THUMBNAILS_SELECTION_COUNT
import org.mozilla.fenix.search.toolbar.SearchSelectorMenu
import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.utils.Settings
import org.mozilla.fenix.wallpapers.Wallpaper
@ -1304,6 +1306,34 @@ class DefaultSessionControlControllerTest {
}
}
@Test
fun `WHEN handleMenuItemTapped is called with SearchSettings item THEN navigate to SearchEngineFragment`() {
createController().handleMenuItemTapped(SearchSelectorMenu.Item.SearchSettings)
verify {
navController.navigate(
match<NavDirections> { it.actionId == R.id.action_global_searchEngineFragment },
null,
)
}
}
@Test
fun `WHEN handleMenuItemTapped is called with SearchEngine item THEN navigate to SearchDialogFragment`() {
val item = mockk<SearchSelectorMenu.Item.SearchEngine>()
every { item.searchEngine.id } returns "DuckDuckGo"
createController().handleMenuItemTapped(item)
val expectedDirections = HomeFragmentDirections.actionGlobalSearchDialog(
sessionId = null,
searchEngine = item.searchEngine.id,
)
verify {
navController.navigate(expectedDirections, any<NavOptions>())
}
}
private fun createController(
hideOnboarding: () -> Unit = { },
registerCollectionStorageObserver: () -> Unit = { },