Skip to content

Commit

Permalink
Issue mozilla-mobile#19809: Remove Grid layout info banner in tabs tray
Browse files Browse the repository at this point in the history
  • Loading branch information
jonalmeida authored and Grisha Kruglov committed Jun 15, 2021
1 parent b0b7389 commit 565e2b9
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 274 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,42 +49,14 @@ class TabsTrayInfoBannerBinding(
}

private fun displayInfoBannerIfNeeded(settings: Settings) {
banner = displayGridViewBannerIfNeeded(settings)
?: displayAutoCloseTabsBannerIfNeeded(settings)
banner = displayAutoCloseTabsBannerIfNeeded(settings)

banner?.apply {
infoBannerView.visibility = VISIBLE
showBanner()
}
}

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 &&
Expand Down
5 changes: 0 additions & 5 deletions app/src/main/java/org/mozilla/fenix/utils/Settings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
3 changes: 0 additions & 3 deletions app/src/main/res/values/preference_keys.xml
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,6 @@
<!-- A value of `true` means the Auto Close Tabs Banner has not been shown yet -->
<string name="pref_key_should_show_auto_close_tabs_banner" translatable="false">pref_key_should_show_auto_close_tabs_banner</string>

<!-- A value of `true` means the Grid View Banner has not been shown yet -->
<string name="pref_key_should_show_grid_view_banner" translatable="false">pref_key_should_show_grid_view_banner</string>

<string name="pref_key_migrating_from_fenix_nightly_tip" translatable="false">pref_key_migrating_from_fenix_nightly_tip</string>
<string name="pref_key_migrating_from_firefox_nightly_tip" translatable="false">pref_key_migrating_from_firefox_nightly_tip</string>
<string name="pref_key_migrating_from_fenix_tip" translatable="false">pref_key_migrating_from_fenix_tip</string>
Expand Down
7 changes: 0 additions & 7 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,6 @@
<!-- Text for the negative action button to dismiss the Close Tabs Banner. -->
<string name="tab_tray_close_tabs_banner_negative_button_text">Dismiss</string>

<!-- Text for the banner message to tell users about our grid view feature. -->
<string name="tab_tray_grid_view_banner_message">Change the layout of open tabs. Go to settings and select grid under tab view.</string>
<!-- Text for the positive action button to go to Settings for auto close tabs. -->
<string name="tab_tray_grid_view_banner_positive_button_text">Go to settings</string>
<!-- Text for the negative action button to dismiss the Close Tabs Banner. -->
<string name="tab_tray_grid_view_banner_negative_button_text">Dismiss</string>

<!-- Home screen icons - Long press shortcuts -->
<!-- Shortcut action to open new tab -->
<string name="home_screen_shortcut_open_new_tab_2">New tab</string>
Expand Down
Original file line number Diff line number Diff line change
@@ -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) }
}
}
Loading

0 comments on commit 565e2b9

Please sign in to comment.