From aa28b6f142c6f00bdbb00523a4862112b37abb5f Mon Sep 17 00:00:00 2001 From: Noah Bond <87384386+MozillaNoah@users.noreply.github.com> Date: Fri, 1 Oct 2021 17:25:12 -0700 Subject: [PATCH] For #21360 - Added toggle for search term tab groups (#21615) * For #21360 - Added toggle for search term tab groups * For #21360 - Lint cleanup * PR: Added missing licenses and possibly fixed UI test * PR: Added a "scrollTo" to potentially fix a UI test * PR: Added potential fix for alwaysStartOnHomeTest * PR: Added temporary ignore to alwaysStartOnHomeTest * PR: added missing ignore comment * For #21360 - Added missing feature flag driven visibility logic Co-authored-by: Sebastian Kaspari Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../java/org/mozilla/fenix/ui/SmokeTest.kt | 1 + .../fenix/ui/robots/SettingsSubMenuTabsRobot.kt | 12 +++++++++++- .../fenix/home/recenttabs/view/RecentTabs.kt | 3 +-- .../mozilla/fenix/settings/TabsSettingsFragment.kt | 6 ++++++ .../fenix/tabstray/browser/NormalBrowserTrayList.kt | 12 +++++++----- .../fenix/tabstray/browser/TitleHeaderBinding.kt | 8 ++++++-- .../org/mozilla/fenix/tabstray/ext/TabSelectors.kt | 10 ++++++---- .../viewholders/NormalBrowserPageViewHolder.kt | 9 ++++++--- .../main/java/org/mozilla/fenix/utils/Settings.kt | 9 +++++++++ app/src/main/res/drawable-night/ic_all_tabs.xml | 13 +++++++++++++ app/src/main/res/drawable/ic_all_tabs.xml | 3 +++ app/src/main/res/values/preference_keys.xml | 1 + app/src/main/res/values/strings.xml | 4 ++++ app/src/main/res/xml/tabs_preferences.xml | 7 +++++++ .../tabstray/browser/TitleHeaderBindingTest.kt | 3 +-- 15 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 app/src/main/res/drawable-night/ic_all_tabs.xml diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt index 701f34aa74..6e7a06b096 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt @@ -1438,6 +1438,7 @@ class SmokeTest { } } + @Ignore // to be fixed here https://github.com/mozilla-mobile/fenix/issues/21644 @Test fun alwaysStartOnHomeTest() { val settings = activityTestRule.activity.applicationContext.settings() diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt index 1ff07f888d..2361171b5a 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt @@ -16,6 +16,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.platform.app.InstrumentationRegistry import androidx.test.uiautomator.UiDevice import org.hamcrest.CoreMatchers.allOf +import org.mozilla.fenix.helpers.TestHelper.scrollToElementByText import org.mozilla.fenix.helpers.click /** @@ -29,7 +30,10 @@ class SettingsSubMenuTabsRobot { fun verifyStartOnHomeOptions() = assertStartOnHomeOptions() - fun clickAlwaysStartOnHomeToggle() = alwaysStartOnHomeToggle().click() + fun clickAlwaysStartOnHomeToggle() { + scrollToElementByText("Always") + alwaysStartOnHomeToggle().click() + } class Transition { val mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) @@ -51,6 +55,8 @@ private fun assertTabViewOptions() { .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) gridToggle() .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) + searchTermTabGroupsToggle() + .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) } private fun assertCloseTabsOptions() { @@ -65,6 +71,8 @@ private fun assertCloseTabsOptions() { } private fun assertStartOnHomeOptions() { + // Scroll to ensure all the items are visible. + scrollToElementByText("Never") startOnHomeHeading() .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) afterFourHoursToggle() @@ -79,6 +87,8 @@ private fun listToggle() = onView(withText("List")) private fun gridToggle() = onView(withText("Grid")) +private fun searchTermTabGroupsToggle() = onView(withText("Search groups")) + private fun closeTabsHeading() = onView(withText("Close tabs")) private fun manuallyToggle() = onView(withText("Manually")) diff --git a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt index cbdfc3d619..c090e1035f 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt @@ -43,7 +43,6 @@ import mozilla.components.browser.icons.compose.Placeholder import mozilla.components.browser.icons.compose.WithIcon import mozilla.components.support.ktx.kotlin.getRepresentativeSnippet import mozilla.components.ui.colors.PhotonColors -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.components.components import org.mozilla.fenix.home.recenttabs.RecentTab @@ -78,7 +77,7 @@ fun RecentTabs( ) } is RecentTab.SearchGroup -> { - if (FeatureFlags.tabGroupFeature) { + if (components.settings.searchTermTabGroupsAreEnabled) { RecentSearchGroupItem( searchTerm = tab.searchTerm, tabId = tab.tabId, diff --git a/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt index fd131a7785..964cfc4096 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt @@ -34,6 +34,7 @@ class TabsSettingsFragment : PreferenceFragmentCompat() { private lateinit var startOnHomeRadioNever: RadioButtonPreference private lateinit var inactiveTabsCategory: PreferenceCategory private lateinit var inactiveTabs: SwitchPreference + private lateinit var searchTermTabGroups: SwitchPreference override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) { setPreferencesFromResource(R.xml.tabs_preferences, rootKey) @@ -58,6 +59,11 @@ class TabsSettingsFragment : PreferenceFragmentCompat() { // pref_key_tab_view_grid and look into using the native RadioGroup in the future. listRadioButton = requirePreference(R.string.pref_key_tab_view_list_do_not_use) gridRadioButton = requirePreference(R.string.pref_key_tab_view_grid) + searchTermTabGroups = requirePreference(R.string.pref_key_search_term_tab_groups).also { + it.isVisible = FeatureFlags.tabGroupFeature + it.isChecked = it.context.settings().searchTermTabGroupsAreEnabled + it.onPreferenceChangeListener = SharedPreferenceUpdater() + } radioManual = requirePreference(R.string.pref_key_close_tabs_manually) radioOneMonth = requirePreference(R.string.pref_key_close_tabs_after_one_month) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt index e21713f718..f7355c8b6a 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt @@ -10,7 +10,6 @@ import androidx.recyclerview.widget.ConcatAdapter import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabViewHolder import mozilla.components.feature.tabs.tabstray.TabsFeature -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.tabstray.ext.browserAdapter @@ -45,14 +44,15 @@ class NormalBrowserTrayList @JvmOverloads constructor( override val tabsFeature by lazy { val tabsAdapter = concatAdapter.browserAdapter val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled + val searchTermTabGroupsAreEnabled = context.settings().searchTermTabGroupsAreEnabled val tabFilter: (TabSessionState) -> Boolean = { when { - FeatureFlags.tabGroupFeature && inactiveTabsEnabled -> + searchTermTabGroupsAreEnabled && inactiveTabsEnabled -> it.isNormalTabActiveWithoutSearchTerm(maxActiveTime) inactiveTabsEnabled -> it.isNormalTabActive(maxActiveTime) - FeatureFlags.tabGroupFeature -> it.isNormalTabWithoutSearchTerm() + searchTermTabGroupsAreEnabled -> it.isNormalTabWithoutSearchTerm() else -> !it.content.private } @@ -71,11 +71,13 @@ class NormalBrowserTrayList @JvmOverloads constructor( private val searchTermFeature by lazy { val store = context.components.core.store val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled + val searchTermTabGroupsAreEnabled = context.settings().searchTermTabGroupsAreEnabled val tabFilter: (TabSessionState) -> Boolean = { when { - FeatureFlags.tabGroupFeature && inactiveTabsEnabled -> it.isNormalTabActiveWithSearchTerm(maxActiveTime) + searchTermTabGroupsAreEnabled && inactiveTabsEnabled -> + it.isNormalTabActiveWithSearchTerm(maxActiveTime) - FeatureFlags.tabGroupFeature -> it.isNormalTabWithSearchTerm() + searchTermTabGroupsAreEnabled -> it.isNormalTabWithSearchTerm() else -> false } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt index 18c51831ac..163ca067e5 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt @@ -25,8 +25,12 @@ class TitleHeaderBinding( private val showHeader: (Boolean) -> Unit ) : AbstractBinding(store) { override suspend fun onState(flow: Flow) { - flow.map { it.getNormalTrayTabs(settings.inactiveTabsAreEnabled) } - .ifChanged { it.size } + flow.map { + it.getNormalTrayTabs( + settings.searchTermTabGroupsAreEnabled, + settings.inactiveTabsAreEnabled + ) + }.ifChanged { it.size } .collect { if (it.isEmpty()) { showHeader(false) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt index 7b6c49388b..0cac457365 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt @@ -8,7 +8,6 @@ import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.TabSessionState -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.tabstray.browser.maxActiveTime /** @@ -41,13 +40,16 @@ val BrowserState.inactiveTabs: List /** * The list of normal tabs in the tabs tray filtered appropriately based on feature flags. */ -fun BrowserState.getNormalTrayTabs(inactiveTabsEnabled: Boolean): List { +fun BrowserState.getNormalTrayTabs( + searchTermTabGroupsAreEnabled: Boolean, + inactiveTabsEnabled: Boolean +): List { return normalTabs.run { - if (FeatureFlags.tabGroupFeature && inactiveTabsEnabled) { + if (searchTermTabGroupsAreEnabled && inactiveTabsEnabled) { filter { it.isNormalTabActiveWithoutSearchTerm(maxActiveTime) } } else if (inactiveTabsEnabled) { filter { it.isNormalTabActive(maxActiveTime) } - } else if (FeatureFlags.tabGroupFeature) { + } else if (searchTermTabGroupsAreEnabled) { filter { it.isNormalTabWithSearchTerm() } } else { this diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt index 961ec656c9..f947013b19 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt @@ -12,7 +12,6 @@ import androidx.recyclerview.widget.RecyclerView import mozilla.components.browser.state.selector.selectedNormalTab import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.tabstray.Tab -import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.ext.settings import org.mozilla.fenix.selection.SelectionHolder @@ -80,6 +79,7 @@ class NormalBrowserPageViewHolder( val inactiveTabAdapter = concatAdapter.inactiveTabsAdapter val tabGroupAdapter = concatAdapter.tabGroupAdapter val inactiveTabsAreEnabled = containerView.context.settings().inactiveTabsAreEnabled + val searchTermTabGroupsAreEnabled = containerView.context.settings().searchTermTabGroupsAreEnabled val selectedTab = browserStore.state.selectedNormalTab ?: return @@ -102,7 +102,7 @@ class NormalBrowserPageViewHolder( } // Updates tabs into the search term group adapter. - if (FeatureFlags.tabGroupFeature && selectedTab.isNormalTabActiveWithSearchTerm(maxActiveTime)) { + if (searchTermTabGroupsAreEnabled && selectedTab.isNormalTabActiveWithSearchTerm(maxActiveTime)) { tabGroupAdapter.observeFirstInsert { // With a grouping, we need to use the list of the adapter that is already grouped // together for the UI, so we know the final index of the grouping to scroll to. @@ -127,7 +127,10 @@ class NormalBrowserPageViewHolder( // Updates tabs into the normal browser tabs adapter. browserAdapter.observeFirstInsert { - val activeTabsList = browserStore.state.getNormalTrayTabs(inactiveTabsAreEnabled) + val activeTabsList = browserStore.state.getNormalTrayTabs( + searchTermTabGroupsAreEnabled, + inactiveTabsAreEnabled + ) activeTabsList.forEachIndexed { tabIndex, trayTab -> if (trayTab.id == selectedTab.id) { 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 d731c92054..de9bb1ea61 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -421,6 +421,15 @@ class Settings(private val appContext: Context) : PreferencesHolder { featureFlag = FeatureFlags.inactiveTabs ) + /** + * Indicates if the user has enabled the search term tab groups feature. + */ + var searchTermTabGroupsAreEnabled by featureFlagPreference( + appContext.getPreferenceKey(R.string.pref_key_search_term_tab_groups), + default = FeatureFlags.tabGroupFeature, + featureFlag = FeatureFlags.tabGroupFeature + ) + @VisibleForTesting internal fun timeNowInMillis(): Long = System.currentTimeMillis() diff --git a/app/src/main/res/drawable-night/ic_all_tabs.xml b/app/src/main/res/drawable-night/ic_all_tabs.xml new file mode 100644 index 0000000000..b01e330937 --- /dev/null +++ b/app/src/main/res/drawable-night/ic_all_tabs.xml @@ -0,0 +1,13 @@ + + + + diff --git a/app/src/main/res/drawable/ic_all_tabs.xml b/app/src/main/res/drawable/ic_all_tabs.xml index 06064ec416..c48775b194 100644 --- a/app/src/main/res/drawable/ic_all_tabs.xml +++ b/app/src/main/res/drawable/ic_all_tabs.xml @@ -1,3 +1,6 @@ + pref_key_camera_permissions_needed pref_key_inactive_tabs_category pref_key_inactive_tabs + pref_key_search_term_tab_groups pref_key_return_to_browser diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 64a437e007..284077f086 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -661,6 +661,10 @@ List Grid + + Search groups + + Group related sites together Close tabs diff --git a/app/src/main/res/xml/tabs_preferences.xml b/app/src/main/res/xml/tabs_preferences.xml index 3a4f01762d..dd377f743e 100644 --- a/app/src/main/res/xml/tabs_preferences.xml +++ b/app/src/main/res/xml/tabs_preferences.xml @@ -18,6 +18,13 @@ android:defaultValue="true" android:key="@string/pref_key_tab_view_grid" android:title="@string/tab_view_grid" /> + +