From e9189dc089ac6dd15aad0663698e03363584eb22 Mon Sep 17 00:00:00 2001 From: Sawyer Blatz Date: Thu, 4 Jun 2020 13:21:20 -0700 Subject: [PATCH] For #11045: Add reader mode to urlView --- .../mozilla/fenix/browser/BrowserFragment.kt | 29 ++++++++++++++- .../components/toolbar/BrowserInteractor.kt | 4 +++ .../toolbar/BrowserToolbarController.kt | 11 +++++- .../components/toolbar/BrowserToolbarView.kt | 1 + .../components/toolbar/DefaultToolbarMenu.kt | 26 -------------- .../res/drawable/ic_readermode_selected.xml | 24 +++++++++++++ app/src/main/res/values-night/colors.xml | 2 ++ app/src/main/res/values/attrs.xml | 2 ++ app/src/main/res/values/colors.xml | 2 ++ app/src/main/res/values/strings.xml | 4 ++- app/src/main/res/values/styles.xml | 3 ++ .../toolbar/DefaultToolbarMenuTest.kt | 35 ++----------------- 12 files changed, 81 insertions(+), 62 deletions(-) create mode 100644 app/src/main/res/drawable/ic_readermode_selected.xml diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 85d3d1331..f4d11824f 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -10,6 +10,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.coordinatorlayout.widget.CoordinatorLayout +import androidx.core.content.ContextCompat import androidx.lifecycle.Observer import androidx.navigation.fragment.findNavController import com.google.android.material.snackbar.Snackbar @@ -17,6 +18,8 @@ import kotlinx.android.synthetic.main.fragment_browser.* import kotlinx.android.synthetic.main.fragment_browser.view.* import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.browser.session.Session +import mozilla.components.browser.state.selector.findTab +import mozilla.components.browser.toolbar.BrowserToolbar import mozilla.components.feature.app.links.AppLinksUseCases import mozilla.components.feature.contextmenu.ContextMenuCandidate import mozilla.components.feature.readerview.ReaderViewFeature @@ -52,6 +55,8 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { private val windowFeature = ViewBoundFeatureWrapper() private val searchFeature = ViewBoundFeatureWrapper() + private var readerModeAvailable = false + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -68,17 +73,39 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { val components = context.components return super.initializeUI(view)?.also { + val readerModeAction = + BrowserToolbar.ToggleButton( + image = ContextCompat.getDrawable(requireContext(), R.drawable.ic_readermode)!!, + imageSelected = ContextCompat.getDrawable(requireContext(), R.drawable.ic_readermode_selected)!!, + contentDescription = requireContext().getString(R.string.browser_menu_read), + contentDescriptionSelected = requireContext().getString(R.string.browser_menu_read_close), + visible = { + readerModeAvailable + }, + selected = getSessionById()?.let { + activity?.components?.core?.store?.state?.findTab(it.id)?.readerState?.active + } ?: false, + listener = browserInteractor::onReaderModePressed + ) + + browserToolbarView.view.addPageAction(readerModeAction) + readerViewFeature.set( feature = ReaderViewFeature( context, components.core.engine, components.core.store, view.readerViewControlsBar - ) { available, _ -> + ) { available, active -> if (available) { components.analytics.metrics.track(Event.ReaderModeAvailable) } + + readerModeAvailable = available + readerModeAction.setSelected(active) + runIfFragmentIsAttached { + browserToolbarView.view.invalidateActions() browserToolbarView.toolbarIntegration.invalidateMenu() } }, diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt index 0f049ecc6..ab4e93a98 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt @@ -35,4 +35,8 @@ open class BrowserInteractor( override fun onScrolled(offset: Int) { browserToolbarController.handleScroll(offset) } + + override fun onReaderModePressed(enabled: Boolean) { + browserToolbarController.handleReaderModePressed(enabled) + } } 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 132eef8c0..44c57f6f4 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 @@ -53,9 +53,10 @@ interface BrowserToolbarController { fun handleToolbarClick() fun handleTabCounterClick() fun handleBrowserMenuDismissed(lowPrioHighlightItems: List) + fun handleReaderModePressed(enabled: Boolean) } -@Suppress("LargeClass") +@Suppress("LargeClass", "TooManyFunctions") class DefaultBrowserToolbarController( private val activity: Activity, private val navController: NavController, @@ -124,6 +125,14 @@ class DefaultBrowserToolbarController( onTabCounterClicked.invoke() } + override fun handleReaderModePressed(enabled: Boolean) { + if (enabled) { + readerModeController.showReaderView() + } else { + readerModeController.hideReaderView() + } + } + override fun handleBrowserMenuDismissed(lowPrioHighlightItems: List) { lowPrioHighlightItems.forEach { when (it) { diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index fee3bc30d..77d9f20dc 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt @@ -52,6 +52,7 @@ interface BrowserToolbarViewInteractor { fun onTabCounterClicked() fun onBrowserMenuDismissed(lowPrioHighlightItems: List) fun onScrolled(offset: Int) + fun onReaderModePressed(enabled: Boolean) } @SuppressWarnings("LargeClass") class BrowserToolbarView( diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt index e0f40e1a9..17c04f2c4 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt @@ -15,7 +15,6 @@ import mozilla.components.browser.menu.BrowserMenuHighlight import mozilla.components.browser.menu.WebExtensionBrowserMenuBuilder import mozilla.components.browser.menu.item.BrowserMenuDivider import mozilla.components.browser.menu.item.BrowserMenuHighlightableItem -import mozilla.components.browser.menu.item.BrowserMenuHighlightableSwitch import mozilla.components.browser.menu.item.BrowserMenuImageSwitch import mozilla.components.browser.menu.item.BrowserMenuImageText import mozilla.components.browser.menu.item.BrowserMenuItemToolbar @@ -143,9 +142,6 @@ class DefaultToolbarMenu( if (canInstall() && installToHomescreen.isHighlighted()) { lowPrioHighlightItems.add(ToolbarMenu.Item.InstallToHomeScreen) } - if (shouldShowReaderMode() && readerMode.isHighlighted()) { - lowPrioHighlightItems.add(ToolbarMenu.Item.ReaderMode(false)) - } if (shouldShowOpenInApp() && openInApp.isHighlighted()) { lowPrioHighlightItems.add(ToolbarMenu.Item.OpenInApp) } @@ -161,10 +157,6 @@ class DefaultToolbarMenu( session != null && context.components.useCases.webAppUseCases.isPinningSupported() && context.components.useCases.webAppUseCases.isInstallable() - private fun shouldShowReaderMode(): Boolean = session?.let { - store.state.findTab(it.id)?.readerState?.readerable - } ?: false - private fun shouldShowOpenInApp(): Boolean = session?.let { session -> val appLink = context.components.useCases.appLinksUseCases.appLinkRedirect appLink(session.url).hasExternalApp() @@ -196,7 +188,6 @@ class DefaultToolbarMenu( if (shouldShowSaveToCollection) saveToCollection else null, desktopMode, openInApp.apply { visible = ::shouldShowOpenInApp }, - readerMode.apply { visible = ::shouldShowReaderMode }, readerAppearance.apply { visible = ::shouldShowReaderAppearance }, BrowserMenuDivider(), menuToolbar @@ -301,23 +292,6 @@ class DefaultToolbarMenu( onItemTapped.invoke(ToolbarMenu.Item.Quit) } - private val readerMode = BrowserMenuHighlightableSwitch( - label = context.getString(R.string.browser_menu_read), - startImageResource = R.drawable.ic_readermode, - initialState = { - session?.let { - store.state.findTab(it.id)?.readerState?.active - } ?: false - }, - highlight = BrowserMenuHighlight.LowPriority( - label = context.getString(R.string.browser_menu_read), - notificationTint = getColor(context, R.color.whats_new_notification_color) - ), - isHighlighted = { !context.settings().readerModeOpened } - ) { checked -> - onItemTapped.invoke(ToolbarMenu.Item.ReaderMode(checked)) - } - private val readerAppearance = BrowserMenuImageText( label = context.getString(R.string.browser_menu_read_appearance), imageResource = R.drawable.ic_readermode_appearance, diff --git a/app/src/main/res/drawable/ic_readermode_selected.xml b/app/src/main/res/drawable/ic_readermode_selected.xml new file mode 100644 index 000000000..c72b5267b --- /dev/null +++ b/app/src/main/res/drawable/ic_readermode_selected.xml @@ -0,0 +1,24 @@ + + + + + + + + + diff --git a/app/src/main/res/values-night/colors.xml b/app/src/main/res/values-night/colors.xml index 74229f093..9c9c764b8 100644 --- a/app/src/main/res/values-night/colors.xml +++ b/app/src/main/res/values-night/colors.xml @@ -45,6 +45,8 @@ @color/add_on_private_browsing_exterior_circle_background_dark_theme @color/add_on_private_browsing_interior_icon_background_dark_theme @color/prompt_login_edit_text_cursor_color_dark_theme + #C689FF + #00B3F4 @color/tab_tray_item_text_dark_theme diff --git a/app/src/main/res/values/attrs.xml b/app/src/main/res/values/attrs.xml index 98d45db5f..3c6241eba 100644 --- a/app/src/main/res/values/attrs.xml +++ b/app/src/main/res/values/attrs.xml @@ -49,6 +49,8 @@ + + diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index 5fc9ef814..6709fb322 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -63,6 +63,8 @@ @color/accent_bright_light_theme #FFFFFF #312a64 + #FF9059ff + #FF0250bb @color/ink_80 diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a17ccce1d..8591aae6b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -118,8 +118,10 @@ Powered by %1$s - + Reader view + + Close reader view Open in app diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index 4f39b51d6..9a9b58647 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -73,6 +73,9 @@ @color/add_on_private_browsing_exterior_circle_background_normal_theme @color/add_on_private_browsing_interior_icon_background_normal_theme @color/prompt_login_edit_text_cursor_color_normal_theme + @color/readermode_start_gradient_normal_theme + @color/readermode_end_gradient_normal_theme + @color/tab_tray_item_background_normal_theme @color/tab_tray_item_selected_background_normal_theme diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenuTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenuTest.kt index a5166903f..25006899a 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenuTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenuTest.kt @@ -6,7 +6,6 @@ import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.spyk -import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ReaderState @@ -69,15 +68,10 @@ class DefaultToolbarMenuTest { every { appLinkRedirect.hasExternalApp() } returns true every { getAppLinkRedirect(any()) } returns appLinkRedirect - val session: Session = mockk(relaxed = true) - every { session.id } returns "readerable-tab" - every { sessionManager.selectedSession } returns session - val list = defaultToolbarMenu.getLowPrioHighlightItems() assertEquals(ToolbarMenu.Item.InstallToHomeScreen, list[0]) - assertEquals(ToolbarMenu.Item.ReaderMode(false), list[1]) - assertEquals(ToolbarMenu.Item.OpenInApp, list[2]) + assertEquals(ToolbarMenu.Item.OpenInApp, list[1]) } @Test @@ -94,29 +88,9 @@ class DefaultToolbarMenuTest { every { appLinkRedirect.hasExternalApp() } returns true every { getAppLinkRedirect(any()) } returns appLinkRedirect - val session: Session = mockk(relaxed = true) - every { session.id } returns "readerable-tab" - every { sessionManager.selectedSession } returns session - - val list = defaultToolbarMenu.getLowPrioHighlightItems() - - assertEquals(ToolbarMenu.Item.ReaderMode(false), list[0]) - assertEquals(ToolbarMenu.Item.OpenInApp, list[1]) - } - - @Test - fun `get all low prio highlight items without ReaderMode`() { - every { context.components.useCases.webAppUseCases.isPinningSupported() } returns true - every { context.components.useCases.webAppUseCases.isInstallable() } returns true - - val getAppLinkRedirect: AppLinksUseCases.GetAppLinkRedirect = mockk(relaxed = true) - every { context.components.useCases.appLinksUseCases.appLinkRedirect } returns getAppLinkRedirect - every { getAppLinkRedirect(any()).hasExternalApp() } returns true - val list = defaultToolbarMenu.getLowPrioHighlightItems() - assertEquals(ToolbarMenu.Item.InstallToHomeScreen, list[0]) - assertEquals(ToolbarMenu.Item.OpenInApp, list[1]) + assertEquals(ToolbarMenu.Item.OpenInApp, list[0]) } @Test @@ -124,10 +98,6 @@ class DefaultToolbarMenuTest { every { context.components.useCases.webAppUseCases.isPinningSupported() } returns true every { context.components.useCases.webAppUseCases.isInstallable() } returns true - val session: Session = mockk(relaxed = true) - every { session.id } returns "readerable-tab" - every { sessionManager.selectedSession } returns session - val getAppLinkRedirect: AppLinksUseCases.GetAppLinkRedirect = mockk(relaxed = true) every { context.components.useCases.appLinksUseCases.appLinkRedirect } returns getAppLinkRedirect every { getAppLinkRedirect(any()).hasExternalApp() } returns false @@ -135,6 +105,5 @@ class DefaultToolbarMenuTest { val list = defaultToolbarMenu.getLowPrioHighlightItems() assertEquals(ToolbarMenu.Item.InstallToHomeScreen, list[0]) - assertEquals(ToolbarMenu.Item.ReaderMode(false), list[1]) } }