Bug 1895153 - Implement "Find in page..." menu functionality r=android-reviewers,petru

Differential Revision: https://phabricator.services.mozilla.com/D210138
This commit is contained in:
Gabriel Luong 2024-07-19 03:20:04 +00:00
parent 5d427361d6
commit 64826cc4d8
18 changed files with 281 additions and 12 deletions

View File

@ -0,0 +1,37 @@
/* 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.bindings
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.map
import mozilla.components.lib.state.helpers.AbstractBinding
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
import org.mozilla.fenix.components.appstate.AppState
/**
* A binding for observing the [AppState.showFindInPage] state in the [AppStore] and displaying
* the find in page feature.
*
* @param appStore The [AppStore] used to observe [AppState.showFindInPage].
* @param onFindInPageLaunch Invoked when the find in page feature should be launched.
*/
class FindInPageBinding(
private val appStore: AppStore,
private val onFindInPageLaunch: () -> Unit,
) : AbstractBinding<AppState>(appStore) {
override suspend fun onState(flow: Flow<AppState>) {
flow.map { state -> state.showFindInPage }
.distinctUntilChanged()
.collect { showFindInPage ->
if (showFindInPage) {
onFindInPageLaunch()
appStore.dispatch(FindInPageAction.FindInPageShown)
}
}
}
}

View File

@ -137,6 +137,7 @@ import org.mozilla.fenix.OnLongPressedListener
import org.mozilla.fenix.OpenInFirefoxBinding
import org.mozilla.fenix.R
import org.mozilla.fenix.ReaderViewBinding
import org.mozilla.fenix.bindings.FindInPageBinding
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.browser.readermode.DefaultReaderModeController
import org.mozilla.fenix.browser.tabstrip.TabStrip
@ -286,6 +287,8 @@ abstract class BaseBrowserFragment :
private val crashContentIntegration = ViewBoundFeatureWrapper<CrashContentIntegration>()
private val readerViewBinding = ViewBoundFeatureWrapper<ReaderViewBinding>()
private val openInFirefoxBinding = ViewBoundFeatureWrapper<OpenInFirefoxBinding>()
private val findInPageBinding = ViewBoundFeatureWrapper<FindInPageBinding>()
private var pipFeature: PictureInPictureFeature? = null
var customTabSessionId: String? = null
@ -585,6 +588,7 @@ abstract class BaseBrowserFragment :
findInPageIntegration.set(
feature = FindInPageIntegration(
store = store,
appStore = requireComponents.appStore,
sessionId = customTabSessionId,
view = binding.findInPageView,
engineView = binding.engineView,
@ -598,6 +602,15 @@ abstract class BaseBrowserFragment :
view = view,
)
findInPageBinding.set(
feature = FindInPageBinding(
appStore = context.components.appStore,
onFindInPageLaunch = { findInPageIntegration.withFeature { it.launch() } },
),
owner = this,
view = view,
)
readerViewBinding.set(
feature = ReaderViewBinding(
appStore = context.components.appStore,

View File

@ -19,11 +19,14 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature
import mozilla.components.support.base.feature.UserInteractionHandler
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FindInPageIntegration.ToolbarInfo
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
import org.mozilla.fenix.components.appstate.AppState
/**
* BrowserFragment delegate to handle all layout updates needed to show or hide the find in page bar.
*
* @param store The [BrowserStore] used to look up the current selected tab.
* @param appStore The [AppStore] used to update the [AppState.showFindInPage] state.
* @param sessionId ID of the [store] session in which the query will be performed.
* @param view The [FindInPageBar] view to display.
* @param engineView the browser in which the queries will be made and which needs to be better positioned
@ -33,6 +36,7 @@ import org.mozilla.fenix.components.FindInPageIntegration.ToolbarInfo
*/
class FindInPageIntegration(
private val store: BrowserStore,
private val appStore: AppStore,
private val sessionId: String? = null,
private val view: FindInPageBar,
private val engineView: EngineView,
@ -47,6 +51,7 @@ class FindInPageIntegration(
override fun stop() {
feature.stop()
appStore.dispatch(FindInPageAction.FindInPageDismissed)
}
override fun onBackPressed(): Boolean {
@ -56,6 +61,7 @@ class FindInPageIntegration(
private fun onClose() {
view.visibility = View.GONE
restorePreviousLayout()
appStore.dispatch(FindInPageAction.FindInPageDismissed)
}
/**

View File

@ -429,4 +429,25 @@ sealed class AppAction : Action {
*/
data object Reset : SnackbarAction()
}
/**
* [AppAction]s related to the find in page feature.
*/
sealed class FindInPageAction : AppAction() {
/**
* [FindInPageAction] dispatched for launching the find in page feature.
*/
data object FindInPageStarted : FindInPageAction()
/**
* [FindInPageAction] dispatched when find in page feature is shown.
*/
data object FindInPageShown : FindInPageAction()
/**
* [FindInPageAction] dispatched when find in page feature is dismissed.
*/
data object FindInPageDismissed : FindInPageAction()
}
}

View File

@ -62,6 +62,7 @@ import org.mozilla.fenix.wallpapers.WallpaperState
* @property standardSnackbarError A snackbar error message to display.
* @property shoppingState Holds state for shopping feature that's required to live the lifetime of a session.
* @property snackbarState The [SnackbarState] to display.
* @property showFindInPage Whether or not to show the find in page feature.
* @property wasLastTabClosedPrivate Whether the last remaining tab that was closed in private mode. This is used to
* display an undo snackbar message relevant to the browsing mode. If null, no snackbar is shown.
*/
@ -93,5 +94,6 @@ data class AppState(
val standardSnackbarError: StandardSnackbarError? = null,
val shoppingState: ShoppingState = ShoppingState(),
val snackbarState: SnackbarState = SnackbarState.None,
val showFindInPage: Boolean = false,
val wasLastTabClosedPrivate: Boolean? = null,
) : State

View File

@ -10,6 +10,7 @@ import mozilla.components.service.pocket.PocketStory.PocketSponsoredStory
import mozilla.components.service.pocket.ext.recordNewImpression
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.reducer.FindInPageStateReducer
import org.mozilla.fenix.components.appstate.shopping.ShoppingStateReducer
import org.mozilla.fenix.components.appstate.snackbar.SnackbarState
import org.mozilla.fenix.components.appstate.snackbar.SnackbarStateReducer
@ -272,6 +273,7 @@ internal object AppStoreReducer {
state.copy(openInFirefoxRequested = false)
}
is AppAction.FindInPageAction -> FindInPageStateReducer.reduce(state, action)
is AppAction.ShortcutAction -> ShortcutStateReducer.reduce(state, action)
is AppAction.ShoppingAction -> ShoppingStateReducer.reduce(state, action)
is AppAction.SnackbarAction -> SnackbarStateReducer.reduce(state, action)

View File

@ -0,0 +1,25 @@
/* 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.components.appstate.reducer
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
import org.mozilla.fenix.components.appstate.AppState
/**
* A [FindInPageAction] reducer that updates [AppState.showFindInPage].
*/
internal object FindInPageStateReducer {
fun reduce(state: AppState, action: FindInPageAction): AppState = when (action) {
FindInPageAction.FindInPageDismissed,
FindInPageAction.FindInPageShown,
-> state.copy(
showFindInPage = false,
)
FindInPageAction.FindInPageStarted -> state.copy(
showFindInPage = true,
)
}
}

View File

@ -233,7 +233,9 @@ class MenuDialogFragment : BottomSheetDialogFragment() {
store.dispatch(MenuAction.Navigate.NewPrivateTab)
},
onSwitchToDesktopSiteMenuClick = {},
onFindInPageMenuClick = {},
onFindInPageMenuClick = {
store.dispatch(MenuAction.FindInPage)
},
onToolsMenuClick = {
store.dispatch(MenuAction.Navigate.Tools)
},
@ -378,7 +380,9 @@ class MenuDialogFragment : BottomSheetDialogFragment() {
composable(route = CUSTOM_TAB_MENU_ROUTE) {
CustomTabMenu(
onSwitchToDesktopSiteMenuClick = {},
onFindInPageMenuClick = {},
onFindInPageMenuClick = {
store.dispatch(MenuAction.FindInPage)
},
onOpenInFirefoxMenuClick = {
store.dispatch(MenuAction.OpenInFirefox)
},

View File

@ -25,6 +25,7 @@ import mozilla.components.support.base.log.logger.Logger
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction
import org.mozilla.fenix.components.appstate.AppAction.BookmarkAction
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
import org.mozilla.fenix.components.bookmarks.BookmarksUseCase
import org.mozilla.fenix.components.menu.store.BookmarkState
import org.mozilla.fenix.components.menu.store.MenuAction
@ -84,6 +85,7 @@ class MenuDialogMiddleware(
is MenuAction.AddShortcut -> addShortcut(context.store)
is MenuAction.RemoveShortcut -> removeShortcut(context.store)
is MenuAction.DeleteBrowsingDataAndQuit -> deleteBrowsingDataAndQuit()
is MenuAction.FindInPage -> launchFindInPage()
is MenuAction.OpenInApp -> openInApp(context.store)
is MenuAction.OpenInFirefox -> openInFirefox()
is MenuAction.InstallAddon -> installAddon(action.addon)
@ -293,6 +295,11 @@ class MenuDialogMiddleware(
onDismiss()
}
private fun launchFindInPage() = scope.launch {
appStore.dispatch(FindInPageAction.FindInPageStarted)
onDismiss()
}
companion object {
private const val NUMBER_OF_RECOMMENDED_ADDONS_TO_SHOW = 4
}

View File

@ -160,6 +160,12 @@ class MenuTelemetryMiddleware(
),
)
MenuAction.FindInPage -> Events.browserMenuAction.record(
Events.BrowserMenuActionExtra(
item = "find_in_page",
),
)
MenuAction.InitAction,
MenuAction.ToggleReaderView,
MenuAction.OpenInFirefox,

View File

@ -71,6 +71,11 @@ sealed class MenuAction : Action {
*/
data object OpenInFirefox : MenuAction()
/**
* [MenuAction] dispatched to launch find in page feature for the current site.
*/
data object FindInPage : MenuAction()
/**
* [MenuAction] dispatched when the extension state is updated.
*

View File

@ -31,6 +31,7 @@ private fun reducer(state: MenuState, action: MenuAction): MenuState {
is MenuAction.AddShortcut,
is MenuAction.RemoveShortcut,
is MenuAction.DeleteBrowsingDataAndQuit,
is MenuAction.FindInPage,
is MenuAction.OpenInApp,
is MenuAction.OpenInFirefox,
is MenuAction.InstallAddon,

View File

@ -0,0 +1,44 @@
/* 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.bindings
import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.rule.MainCoroutineRule
import mozilla.components.support.test.rule.runTestOnMain
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
class FindInPageBindingTest {
@get:Rule
val coroutinesTestRule = MainCoroutineRule()
@Test
fun `WHEN find in page started action is dispatched THEN launch find in page feature`() = runTestOnMain {
val appStore = AppStore()
var onFindInPageLaunchCalled = false
val binding = FindInPageBinding(
appStore = appStore,
onFindInPageLaunch = { onFindInPageLaunchCalled = true },
)
binding.start()
appStore.dispatch(FindInPageAction.FindInPageStarted)
// Wait for FindInPageAction.FindInPageStarted
appStore.waitUntilIdle()
// Wait for FindInPageAction.FindInPageShown
appStore.waitUntilIdle()
assertFalse(appStore.state.showFindInPage)
assertTrue(onFindInPageLaunchCalled)
}
}

View File

@ -34,7 +34,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = true,
isToolbarPlacedAtTop = true,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 123)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 123)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -60,7 +60,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = false,
isToolbarPlacedAtTop = true,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 100)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 100)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -86,7 +86,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = true,
isToolbarPlacedAtTop = true,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -112,7 +112,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = false,
isToolbarPlacedAtTop = true,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -138,7 +138,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = true,
isToolbarPlacedAtTop = false,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -164,7 +164,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = false,
isToolbarPlacedAtTop = false,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -190,7 +190,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = true,
isToolbarPlacedAtTop = false,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -216,7 +216,7 @@ class FindInPageIntegrationTest {
isToolbarDynamic = true,
isToolbarPlacedAtTop = false,
)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), mockk(), toolbarInfo, 50)) {
every { getEngineViewsParentLayoutParams() } returns engineViewParentParams
every { getEngineViewParent() } returns engineViewParent
}
@ -236,7 +236,7 @@ class FindInPageIntegrationTest {
val engineView: GeckoEngineView = mockk(relaxed = true)
every { (engineView as EngineView).asView().parent } returns parent
val feature = FindInPageIntegration(mockk(), null, mockk(), engineView, mockk(), 50)
val feature = FindInPageIntegration(mockk(), mockk(), null, mockk(), engineView, mockk(), 50)
assertSame(parent as View, feature.getEngineViewParent())
}
@ -247,7 +247,7 @@ class FindInPageIntegrationTest {
every { layoutParams } returns mockk<ViewGroup.MarginLayoutParams>(relaxed = true)
}
val engineView: GeckoEngineView = mockk(relaxed = true)
val feature = spyk(FindInPageIntegration(mockk(), null, mockk(), engineView, mockk(), 60))
val feature = spyk(FindInPageIntegration(mockk(), mockk(), null, mockk(), engineView, mockk(), 60))
every { feature.getEngineViewParent() } returns parent
assertSame(parent.layoutParams, feature.getEngineViewsParentLayoutParams())

View File

@ -0,0 +1,42 @@
/* 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.components.appstate
import mozilla.components.support.test.ext.joinBlocking
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
class FindInPageStateReducerTest {
@Test
fun `WHEN find in page started action is dispatched THEN state is updated`() {
val appStore = AppStore()
appStore.dispatch(FindInPageAction.FindInPageStarted).joinBlocking()
assertTrue(appStore.state.showFindInPage)
}
@Test
fun `WHEN find in page dismissed action is dispatched THEN state is updated`() {
val appStore = AppStore()
appStore.dispatch(FindInPageAction.FindInPageDismissed).joinBlocking()
assertFalse(appStore.state.showFindInPage)
}
@Test
fun `WHEN find in page shown action is dispatched THEN state is updated`() {
val appStore = AppStore()
appStore.dispatch(FindInPageAction.FindInPageShown).joinBlocking()
assertFalse(appStore.state.showFindInPage)
}
}

View File

@ -38,6 +38,7 @@ import org.mockito.Mockito.verify
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction
import org.mozilla.fenix.components.appstate.AppAction.BookmarkAction
import org.mozilla.fenix.components.appstate.AppAction.FindInPageAction
import org.mozilla.fenix.components.bookmarks.BookmarksUseCase.AddBookmarksUseCase
import org.mozilla.fenix.components.menu.fake.FakeBookmarksStorage
import org.mozilla.fenix.components.menu.middleware.MenuDialogMiddleware
@ -800,6 +801,38 @@ class MenuDialogMiddlewareTest {
assertTrue(dismissedWasCalled)
}
@Test
fun `WHEN find in page action is dispatched THEN find in page app action is dispatched`() = runTestOnMain {
val url = "https://www.mozilla.org"
val title = "Mozilla"
var dismissWasCalled = false
val browserMenuState = BrowserMenuState(
selectedTab = createTab(
url = url,
title = title,
),
)
val appStore = spy(AppStore())
val store = spy(
createStore(
appStore = appStore,
menuState = MenuState(
browserMenuState = browserMenuState,
),
onDismiss = { dismissWasCalled = true },
),
)
store.waitUntilIdle()
store.dispatch(MenuAction.FindInPage)
store.waitUntilIdle()
verify(appStore).dispatch(FindInPageAction.FindInPageStarted)
assertTrue(dismissWasCalled)
}
private fun createStore(
appStore: AppStore = AppStore(),
menuState: MenuState = MenuState(),

View File

@ -221,6 +221,7 @@ class MenuStoreTest {
assertTrue(store.state.browserMenuState!!.isPinned)
}
@Test
fun `WHEN update extension state action is dispatched THEN extension state is updated`() = runTest {
val addon = Addon(id = "ext1")
val store = MenuStore(initialState = MenuState())
@ -232,4 +233,14 @@ class MenuStoreTest {
assertEquals(1, store.state.extensionMenuState.recommendedAddons.size)
assertEquals(addon, store.state.extensionMenuState.recommendedAddons.first())
}
@Test
fun `WHEN find in page action is dispatched THEN state is not updated`() = runTest {
val initialState = MenuState()
val store = MenuStore(initialState = initialState)
store.dispatch(MenuAction.FindInPage).join()
assertEquals(initialState, store.state)
}
}

View File

@ -269,6 +269,16 @@ class MenuTelemetryMiddlewareTest {
assertTelemetryRecorded(Events.browserMenuAction, item = "quit")
}
@Test
fun `WHEN find in page feature is started THEN record the find in page browser menu telemetry`() {
val store = createStore()
assertNull(Events.browserMenuAction.testGetValue())
store.dispatch(MenuAction.DeleteBrowsingDataAndQuit).joinBlocking()
assertTelemetryRecorded(Events.browserMenuAction, item = "find_in_page")
}
private fun assertTelemetryRecorded(
event: EventMetricType<Events.BrowserMenuActionExtra>,
item: String,