From 565e2b905b1cbfa3ac8a06f61670e837990da9ff Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Thu, 3 Jun 2021 21:32:18 -0400 Subject: [PATCH] Issue #19809: Remove Grid layout info banner in tabs tray --- .../tabstray/TabsTrayInfoBannerBinding.kt | 30 +-- .../java/org/mozilla/fenix/utils/Settings.kt | 5 - app/src/main/res/values/preference_keys.xml | 3 - app/src/main/res/values/strings.xml | 7 - .../tabstray/TabsTrayInfoBannerBindingTest.kt | 109 +++++++++ .../fenix/tabstray/TabsTrayInfoBannerTest.kt | 230 ------------------ 6 files changed, 110 insertions(+), 274 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBindingTest.kt delete mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBinding.kt index be92f210573f..f25e519d484b 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBinding.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBinding.kt @@ -49,8 +49,7 @@ class TabsTrayInfoBannerBinding( } private fun displayInfoBannerIfNeeded(settings: Settings) { - banner = displayGridViewBannerIfNeeded(settings) - ?: displayAutoCloseTabsBannerIfNeeded(settings) + banner = displayAutoCloseTabsBannerIfNeeded(settings) banner?.apply { infoBannerView.visibility = VISIBLE @@ -58,33 +57,6 @@ class TabsTrayInfoBannerBinding( } } - private fun displayGridViewBannerIfNeeded(settings: Settings): InfoBanner? { - return if ( - settings.shouldShowGridViewBanner && - settings.canShowCfr && - settings.listTabView - ) { - InfoBanner( - context = context, - message = context.getString(R.string.tab_tray_grid_view_banner_message), - dismissText = context.getString(R.string.tab_tray_grid_view_banner_negative_button_text), - actionText = context.getString(R.string.tab_tray_grid_view_banner_positive_button_text), - container = infoBannerView, - dismissByHiding = true, - dismissAction = { - metrics?.track(Event.TabsTrayCfrDismissed) - settings.shouldShowGridViewBanner = false - } - ) { - navigationInteractor.onTabSettingsClicked() - metrics?.track(Event.TabsTrayCfrTapped) - settings.shouldShowGridViewBanner = false - } - } else { - null - } - } - private fun displayAutoCloseTabsBannerIfNeeded(settings: Settings): InfoBanner? { return if ( settings.shouldShowAutoCloseTabsBanner && diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index eb7e07e795b6..dd304404a34e 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -727,11 +727,6 @@ class Settings(private val appContext: Context) : PreferencesHolder { default = true ) - var shouldShowGridViewBanner by booleanPreference( - appContext.getPreferenceKey(R.string.pref_key_should_show_grid_view_banner), - default = true - ) - @VisibleForTesting(otherwise = PRIVATE) internal val trackingProtectionOnboardingCount = counterPreference( appContext.getPreferenceKey(R.string.pref_key_tracking_protection_onboarding), diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index bea14a3a31dd..1dd74ffd4698 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -225,9 +225,6 @@ pref_key_should_show_auto_close_tabs_banner - - pref_key_should_show_grid_view_banner - pref_key_migrating_from_fenix_nightly_tip pref_key_migrating_from_firefox_nightly_tip pref_key_migrating_from_fenix_tip diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3fecd505e089..8f78da3dcc83 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -92,13 +92,6 @@ Dismiss - - Change the layout of open tabs. Go to settings and select grid under tab view. - - Go to settings - - Dismiss - New tab diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBindingTest.kt new file mode 100644 index 000000000000..b0b7664c5b7f --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBindingTest.kt @@ -0,0 +1,109 @@ +/* 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.tabstray + +import android.view.View.GONE +import android.view.View.VISIBLE +import android.view.ViewGroup +import androidx.coordinatorlayout.widget.CoordinatorLayout +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher +import mozilla.components.browser.state.action.TabListAction +import mozilla.components.browser.state.state.createTab +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainCoroutineRule +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.tabstray.TabsTrayInfoBannerBinding.Companion.TAB_COUNT_SHOW_CFR +import org.mozilla.fenix.utils.Settings + +@RunWith(FenixRobolectricTestRunner::class) +@OptIn(ExperimentalCoroutinesApi::class) +class TabsTrayInfoBannerBindingTest { + + private lateinit var store: BrowserStore + private lateinit var view: ViewGroup + private lateinit var interactor: NavigationInteractor + private lateinit var metrics: MetricController + private lateinit var settings: Settings + + @get:Rule + val coroutinesTestRule = MainCoroutineRule(TestCoroutineDispatcher()) + + @Before + fun setUp() { + store = BrowserStore() + view = CoordinatorLayout(testContext) + interactor = mockk(relaxed = true) + metrics = mockk(relaxed = true) + settings = Settings(testContext) + } + + @Test + fun `WHEN tab number reaches CFR count THEN banner is shown`() { + view.visibility = GONE + + val binding = + TabsTrayInfoBannerBinding( + context = testContext, + store = store, + infoBannerView = view, + settings = settings, + navigationInteractor = interactor, + metrics = metrics + ) + + binding.start() + for (i in 1 until TAB_COUNT_SHOW_CFR) { + store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) + store.waitUntilIdle() + + assert(view.visibility == GONE) + } + + store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) + store.waitUntilIdle() + assert(view.visibility == VISIBLE) + } + + @Test + fun `WHEN dismiss THEN auto close tabs info banner will not open tab settings`() { + view.visibility = GONE + settings.listTabView = false + + val binding = + TabsTrayInfoBannerBinding( + context = testContext, + store = store, + infoBannerView = view, + settings = settings, + navigationInteractor = interactor, + metrics = metrics + ) + + binding.start() + for (i in 1..TAB_COUNT_SHOW_CFR) { + store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) + store.waitUntilIdle() + } + + assert(view.visibility == VISIBLE) + binding.banner?.dismissAction?.invoke() + + verify(exactly = 0) { interactor.onTabSettingsClicked() } + assert(!settings.shouldShowAutoCloseTabsBanner) + verify(exactly = 0) { metrics.track(Event.TabsTrayCfrTapped) } + verify(exactly = 1) { metrics.track(Event.TabsTrayCfrDismissed) } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerTest.kt deleted file mode 100644 index 93504c306b18..000000000000 --- a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerTest.kt +++ /dev/null @@ -1,230 +0,0 @@ -/* 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.tabstray - -import android.view.View.GONE -import android.view.View.VISIBLE -import android.view.ViewGroup -import androidx.coordinatorlayout.widget.CoordinatorLayout -import io.mockk.mockk -import io.mockk.verify -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.TestCoroutineDispatcher -import mozilla.components.browser.state.action.TabListAction -import mozilla.components.browser.state.state.createTab -import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.support.test.libstate.ext.waitUntilIdle -import mozilla.components.support.test.robolectric.testContext -import mozilla.components.support.test.rule.MainCoroutineRule -import org.junit.Before -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner -import org.mozilla.fenix.tabstray.TabsTrayInfoBannerBinding.Companion.TAB_COUNT_SHOW_CFR -import org.mozilla.fenix.utils.Settings - -@RunWith(FenixRobolectricTestRunner::class) -@OptIn(ExperimentalCoroutinesApi::class) -class TabsTrayInfoBannerTest { - - private lateinit var store: BrowserStore - private lateinit var view: ViewGroup - private lateinit var interactor: NavigationInteractor - private lateinit var metrics: MetricController - private lateinit var settings: Settings - - @get:Rule - val coroutinesTestRule = MainCoroutineRule(TestCoroutineDispatcher()) - - @Before - fun setUp() { - store = BrowserStore() - view = CoordinatorLayout(testContext) - interactor = mockk(relaxed = true) - metrics = mockk(relaxed = true) - settings = Settings(testContext) - } - - @Test - fun `WHEN tab number reaches CFR count THEN banner is shown`() { - view.visibility = GONE - - val binding = - TabsTrayInfoBannerBinding( - context = testContext, - store = store, - infoBannerView = view, - settings = settings, - navigationInteractor = interactor, - metrics = metrics - ) - - binding.start() - for (i in 1 until TAB_COUNT_SHOW_CFR) { - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - - assert(view.visibility == GONE) - } - - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - assert(view.visibility == VISIBLE) - } - - @Test - fun `WHEN in list view THEN grid view banner info banner should be shown`() { - view.visibility = GONE - settings.listTabView = true - - val binding = - TabsTrayInfoBannerBinding( - context = testContext, - store = store, - infoBannerView = view, - settings = settings, - navigationInteractor = interactor, - metrics = metrics - ) - - binding.start() - for (i in 1..TAB_COUNT_SHOW_CFR) { - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - } - - assert(view.visibility == VISIBLE) - binding.banner?.actionToPerform?.invoke() - - verify(exactly = 1) { interactor.onTabSettingsClicked() } - assert(!settings.shouldShowGridViewBanner) - assert(settings.shouldShowAutoCloseTabsBanner) - verify(exactly = 1) { metrics.track(Event.TabsTrayCfrTapped) } - verify(exactly = 0) { metrics.track(Event.TabsTrayCfrDismissed) } - } - - @Test - fun `WHEN grid view banner already shown THEN auto close tabs info banner should be shown`() { - view.visibility = GONE - settings.listTabView = true - settings.shouldShowGridViewBanner = false - - val binding = - TabsTrayInfoBannerBinding( - context = testContext, - store = store, - infoBannerView = view, - settings = settings, - navigationInteractor = interactor, - metrics = metrics - ) - - binding.start() - for (i in 1..TAB_COUNT_SHOW_CFR) { - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - } - - assert(view.visibility == VISIBLE) - binding.banner?.actionToPerform?.invoke() - - verify(exactly = 1) { interactor.onTabSettingsClicked() } - assert(!settings.shouldShowGridViewBanner) - assert(!settings.shouldShowAutoCloseTabsBanner) - verify(exactly = 1) { metrics.track(Event.TabsTrayCfrTapped) } - verify(exactly = 0) { metrics.track(Event.TabsTrayCfrDismissed) } - } - - @Test - fun `WHEN dismiss THEN grid view info banner will not open tab settings`() { - view.visibility = GONE - settings.listTabView = true - - val binding = - TabsTrayInfoBannerBinding( - context = testContext, - store = store, - infoBannerView = view, - settings = settings, - navigationInteractor = interactor, - metrics = metrics - ) - - binding.start() - for (i in 1..TAB_COUNT_SHOW_CFR) { - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - } - - assert(view.visibility == VISIBLE) - binding.banner?.dismissAction?.invoke() - - verify(exactly = 0) { interactor.onTabSettingsClicked() } - assert(!settings.shouldShowGridViewBanner) - assert(settings.shouldShowAutoCloseTabsBanner) - verify(exactly = 0) { metrics.track(Event.TabsTrayCfrTapped) } - verify(exactly = 1) { metrics.track(Event.TabsTrayCfrDismissed) } - } - - @Test - fun `WHEN dismiss THEN auto close tabs info banner will not open tab settings`() { - view.visibility = GONE - settings.listTabView = false - - val binding = - TabsTrayInfoBannerBinding( - context = testContext, - store = store, - infoBannerView = view, - settings = settings, - navigationInteractor = interactor, - metrics = metrics - ) - - binding.start() - for (i in 1..TAB_COUNT_SHOW_CFR) { - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - } - - assert(view.visibility == VISIBLE) - binding.banner?.dismissAction?.invoke() - - verify(exactly = 0) { interactor.onTabSettingsClicked() } - assert(settings.shouldShowGridViewBanner) - assert(!settings.shouldShowAutoCloseTabsBanner) - verify(exactly = 0) { metrics.track(Event.TabsTrayCfrTapped) } - verify(exactly = 1) { metrics.track(Event.TabsTrayCfrDismissed) } - } - - @Test - fun `WHEN both banners were already shown THEN no info banner should be shown`() { - view.visibility = GONE - settings.listTabView = true - settings.shouldShowGridViewBanner = false - settings.shouldShowAutoCloseTabsBanner = false - - val binding = - TabsTrayInfoBannerBinding( - context = testContext, - store = store, - infoBannerView = view, - settings = settings, - navigationInteractor = interactor, - metrics = metrics - ) - - binding.start() - for (i in 1..TAB_COUNT_SHOW_CFR) { - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))) - store.waitUntilIdle() - } - - assert(view.visibility == GONE) - } -}