Issue mozilla-mobile#19809: Change default tab tray view to grid layout

grigoryk · Jun 15, 2021 · c7a2532 · c7a2532
1 parent 565e2b9
commit c7a2532
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 14 deletions.
diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt
@@ -1096,7 +1096,6 @@ class SmokeTest {
             verifyExistingOpenTabs("Test_Page_1")
             verifyCloseTabsButton("Test_Page_1")
             verifyOpenedTabThumbnail()
-            verifyBrowserTabsTrayURL("localhost")
             verifyTabTrayOverflowMenu(true)
             verifyNewTabButton()
         }

diff --git a/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt
@@ -40,7 +40,12 @@ class TabsSettingsFragment : PreferenceFragmentCompat() {
     }
 
     private fun setupPreferences() {
-        listRadioButton = requirePreference(R.string.pref_key_tab_view_list)
+        // This should be the only use case for pref_key_tab_view_list_do_not_use.
+        // In the Fenix impl of RadioGroups, we need to always have an individual pref value
+        // for it to work. This is weird for a radio group that should hold a value from that group.
+        // For the tabs tray, we only need a boolean value, so let's rely on only the
+        // 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)
 
         radioManual = requirePreference(R.string.pref_key_close_tabs_manually)

diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayList.kt
@@ -63,7 +63,7 @@ class BrowserTrayList @JvmOverloads constructor(
         TabsTouchHelper(
             observable = adapter as TabsAdapter,
             onViewHolderTouched = { swipeToDelete.isSwipeable },
-            onViewHolderDraw = { context.components.settings.listTabView }
+            onViewHolderDraw = { context.components.settings.gridTabView.not() }
         )
     }
 

diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt
@@ -321,14 +321,9 @@ class Settings(private val appContext: Context) : PreferencesHolder {
                 numberOfAppLaunches > APP_LAUNCHES_TO_SHOW_DEFAULT_BROWSER_CARD
     }
 
-    var listTabView by booleanPreference(
-        appContext.getPreferenceKey(R.string.pref_key_tab_view_list),
-        default = true
-    )
-
     var gridTabView by booleanPreference(
         appContext.getPreferenceKey(R.string.pref_key_tab_view_grid),
-        default = false
+        default = true
     )
 
     var manuallyCloseTabs by booleanPreference(

diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml
@@ -254,7 +254,8 @@
     <string name="pref_key_show_collections_placeholder_home" translatable="false">pref_key_show_collections_home</string>
 
     <!-- Tabs Settings -->
-    <string name="pref_key_tab_view_list" translatable="false">pref_key_tab_view_list</string>
+    <!-- pref_key_tab_view_list_do_not_use is needed for UI implementation only and should be expected to be useful. -->
+    <string name="pref_key_tab_view_list_do_not_use" translatable="false">pref_key_tab_view_list</string>
     <string name="pref_key_tab_view_grid" translatable="false">pref_key_tab_view_grid</string>
     <string name="pref_key_tabs" translatable="false">pref_key_tabs</string>
     <string name="pref_key_close_tabs_manually" translatable="false">pref_key_close_tabs_manually</string>

diff --git a/app/src/main/res/xml/tabs_preferences.xml b/app/src/main/res/xml/tabs_preferences.xml
@@ -10,12 +10,12 @@
         app:allowDividerAbove="false"
         app:iconSpaceReserved="false">
         <org.mozilla.fenix.settings.RadioButtonPreference
-            android:defaultValue="true"
-            android:key="@string/pref_key_tab_view_list"
+            android:defaultValue="false"
+            android:key="@string/pref_key_tab_view_list_do_not_use"
             android:title="@string/tab_view_list" />
 
         <org.mozilla.fenix.settings.RadioButtonPreference
-            android:defaultValue="false"
+            android:defaultValue="true"
             android:key="@string/pref_key_tab_view_grid"
             android:title="@string/tab_view_grid" />
     </androidx.preference.PreferenceCategory>

diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayInfoBannerBindingTest.kt
@@ -80,7 +80,7 @@ class TabsTrayInfoBannerBindingTest {
     @Test
     fun `WHEN dismiss THEN auto close tabs info banner will not open tab settings`() {
         view.visibility = GONE
-        settings.listTabView = false
+        settings.gridTabView = true
 
         val binding =
             TabsTrayInfoBannerBinding(