diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index a9572322a..26645c8e7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -129,9 +129,6 @@ private val Event.wrapper: EventWrapper<*>? { SearchShortcuts.selected.record(it) }, { SearchShortcuts.selectedKeys.valueOf(it) } ) - is Event.ReaderModeAvailable -> EventWrapper( - { ReaderMode.available.record(it) } - ) is Event.FindInPageOpened -> EventWrapper( { FindInPage.opened.record(it) } ) @@ -311,9 +308,15 @@ private val Event.wrapper: EventWrapper<*>? is Event.CollectionTabSelectOpened -> EventWrapper( { Collections.tabSelectOpened.record(it) } ) + is Event.ReaderModeAvailable -> EventWrapper( + { ReaderMode.available.record(it) } + ) is Event.ReaderModeOpened -> EventWrapper( { ReaderMode.opened.record(it) } ) + is Event.ReaderModeClosed -> EventWrapper( + { ReaderMode.closed.record(it) } + ) is Event.ReaderModeAppearanceOpened -> EventWrapper( { ReaderMode.appearance.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt index 64f0308a4..93f9f4ffc 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt @@ -99,6 +99,7 @@ sealed class Event { object HistoryAllItemsRemoved : Event() object ReaderModeAvailable : Event() object ReaderModeOpened : Event() + object ReaderModeClosed : Event() object ReaderModeAppearanceOpened : Event() object CollectionRenamed : Event() object CollectionTabRestored : Event() diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt index 6a79b1d9c..46000a273 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt @@ -18,7 +18,6 @@ import kotlinx.coroutines.launch import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager -import mozilla.components.browser.state.selector.findTab import mozilla.components.concept.engine.EngineView import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.support.ktx.kotlin.isUrl @@ -133,8 +132,10 @@ class DefaultBrowserToolbarController( override fun handleReaderModePressed(enabled: Boolean) { if (enabled) { readerModeController.showReaderView() + activity.components.analytics.metrics.track(Event.ReaderModeOpened) } else { readerModeController.hideReaderView() + activity.components.analytics.metrics.track(Event.ReaderModeClosed) } } @@ -165,7 +166,6 @@ class DefaultBrowserToolbarController( lowPrioHighlightItems.forEach { when (it) { ToolbarMenu.Item.AddToHomeScreen -> activity.settings().installPwaOpened = true - is ToolbarMenu.Item.ReaderMode -> activity.settings().readerModeOpened = true ToolbarMenu.Item.OpenInApp -> activity.settings().openInAppOpened = true else -> { } @@ -305,21 +305,9 @@ class DefaultBrowserToolbarController( deleteAndQuit(activity, scope, snackbar) } - is ToolbarMenu.Item.ReaderMode -> { - activity.settings().readerModeOpened = true - - val enabled = currentSession?.let { - activity.components.core.store.state.findTab(it.id)?.readerState?.active - } ?: false - - if (enabled) { - readerModeController.hideReaderView() - } else { - readerModeController.showReaderView() - } - } ToolbarMenu.Item.ReaderModeAppearance -> { readerModeController.showControls() + activity.components.analytics.metrics.track(Event.ReaderModeAppearanceOpened) } ToolbarMenu.Item.OpenInApp -> { activity.settings().openInAppOpened = true @@ -385,12 +373,6 @@ class DefaultBrowserToolbarController( ToolbarMenu.Item.SyncedTabs -> Event.BrowserMenuItemTapped.Item.SYNC_TABS ToolbarMenu.Item.InstallToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN ToolbarMenu.Item.Quit -> Event.BrowserMenuItemTapped.Item.QUIT - is ToolbarMenu.Item.ReaderMode -> - if (item.isChecked) { - Event.BrowserMenuItemTapped.Item.READER_MODE_ON - } else { - Event.BrowserMenuItemTapped.Item.READER_MODE_OFF - } ToolbarMenu.Item.ReaderModeAppearance -> Event.BrowserMenuItemTapped.Item.READER_MODE_APPEARANCE ToolbarMenu.Item.OpenInApp -> Event.BrowserMenuItemTapped.Item.OPEN_IN_APP diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt index 6a7e8ed5a..db8dbbe14 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt @@ -25,7 +25,6 @@ interface ToolbarMenu { object SyncedTabs : Item() object AddonsManager : Item() object Quit : Item() - data class ReaderMode(val isChecked: Boolean) : Item() object OpenInApp : Item() object Bookmark : Item() object ReaderModeAppearance : Item() diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt index 636d96552..2a265b0b4 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt @@ -192,7 +192,6 @@ class DefaultBrowserToolbarControllerTest { fun `handle BrowserMenu dismissed with all options available`() = runBlockingTest { val itemList: List = listOf( ToolbarMenu.Item.AddToHomeScreen, - ToolbarMenu.Item.ReaderMode(true), ToolbarMenu.Item.OpenInApp ) @@ -221,7 +220,6 @@ class DefaultBrowserToolbarControllerTest { controller.handleBrowserMenuDismissed(itemList) assertEquals(true, activity.settings().installPwaOpened) - assertEquals(true, activity.settings().readerModeOpened) assertEquals(true, activity.settings().openInAppOpened) } @@ -553,19 +551,6 @@ class DefaultBrowserToolbarControllerTest { verify { deleteAndQuit(activity, testScope, null) } } - @Test - fun handleToolbarReaderModePress() { - val item = ToolbarMenu.Item.ReaderMode(false) - - every { currentSession.id } returns "reader-inactive-tab" - controller.handleToolbarItemInteraction(item) - verify { readerModeController.showReaderView() } - - every { currentSession.id } returns "reader-active-tab" - controller.handleToolbarItemInteraction(item) - verify { readerModeController.hideReaderView() } - } - @Test fun handleToolbarCloseTabPressWithLastPrivateSession() { every { currentSession.id } returns "1"