Closed Bug 1202861 (compact-tabs) Opened 9 years ago Closed 8 years ago

Add "Compact tabs" settings item under "Customize"

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox53 verified)

VERIFIED FIXED
Firefox 53
Tracking Status
firefox53 --- verified

People

(Reporter: antlam, Assigned: twointofive)

References

Details

Attachments

(4 files)

This is what we currently call it in the iOS Fennec settings. I think it might be worth while putting this in as a pref for now, enabled by default. Pros: - Allows us to measure telemetry on this pref - We can use this to gauge feedback - Keeps consistent with iOS - Enabled by default means all users will still get to see the new tray
Flags: needinfo?(mhaigh)
Title: Compact tabs Subtitle: tbd I think this pref should not revert between old and new UI, but rather offer two column versus one column layout. That is to say, new proportions, full screen tabs tray, and back button (bug 1200619), will all still be the same when a user flips between this pref. That way, this will be a way to address feedback from users that require a page title and a 1 column layout. Thoughts? I'll attach a mock.
Flags: needinfo?(alam)
Attached image prev_mob_tray4b.png
This is what I meant. Let me know if you have questions. I've changed nothing except placing the text next to the tab preview. Paddings are all the same, dimensions, etc
Flags: needinfo?(alam)
(In reply to Anthony Lam (:antlam) from comment #1) > Title: Compact tabs > Subtitle: tbd > > I think this pref should not revert between old and new UI, but rather offer > two column versus one column layout. That is to say, new proportions, full > screen tabs tray, and back button (bug 1200619), will all still be the same > when a user flips between this pref. Sounds good to me. Most feedback is about the size of the thumbnails, having too many hit targets jammed together, small 'x', and strange swipe-to-close behavior with two columns. Your mock/plan should address all of those issues. One thing I would suggest: Should this be in Settings > Customize? Or should we just have a simple toggle button in the actionbar of the tabs tray? I like having the button in the tabs tray. It's easier to find and it's better for context. Tap, and the layout changes.
(In reply to Mark Finkle (:mfinkle) from comment #3) > One thing I would suggest: Should this be in Settings > Customize? Or should > we just have a simple toggle button in the actionbar of the tabs tray? > > I like having the button in the tabs tray. It's easier to find and it's > better for context. Tap, and the layout changes. Hm, interesting. How about under the overflow menu "..." ? While I want to give it top visibility, after the user chooses a preference, it's not necessarily great UX to leave it around all the time.
Flags: needinfo?(mark.finkle)
(In reply to Anthony Lam (:antlam) from comment #4) > (In reply to Mark Finkle (:mfinkle) from comment #3) > > One thing I would suggest: Should this be in Settings > Customize? Or should > > we just have a simple toggle button in the actionbar of the tabs tray? > > > > I like having the button in the tabs tray. It's easier to find and it's > > better for context. Tap, and the layout changes. > > Hm, interesting. How about under the overflow menu "..." ? > > While I want to give it top visibility, after the user chooses a preference, > it's not necessarily great UX to leave it around all the time. SOLD!
Flags: needinfo?(mark.finkle)
Can we please make sure that we add some probes to measure this, i.e. how many people use compact vs. old style? Thanks :)
Hey Martyn, please let me know if you have any questions regarding the probes I'd like to put in there and or if there is anything else you need info from me.
+1 on this bug. My problem with the new two column view is: * I can't see enough of the title to understand what tab contains * Often I see a globe icon rather than a screen contents as I open in background tab from FB/Twitter * the status message fade in/out covers the bottom row as this is my most recent tabs Mcomella asked me to NI him on this in passing.
Flags: needinfo?(michael.l.comella)
(In reply to Edwin Wong [:edwong] from comment #9) > * Often I see a globe icon rather than a screen contents as I open in > background tab from FB/Twitter This should be filed in another bug, though I'd expect it to already be filed. > * the status message fade in/out covers the bottom row as this is my most > recent tabs We plan to fix this with bug 1157526, where a SnackBar will cover significantly less of the tabs. > Mcomella asked me to NI him on this in passing. I don't recall asking that – are you sure it was me?
Flags: needinfo?(michael.l.comella)
I raised some of these issues a while back with bugs like bug 651822. I would ideally like to see us have three options, the grid option we have presently, a compact option with just the text and favicon akin to what we present on desktop and also implement a tab carousel as per what Chrome* and the Android Recent Task switcher users. We could store thumbnails locally to ensure that users are never presented with a grey globe. Users could switch between modes via pinch-to-zoom I filed bug 1119628 to put lay the foundation for that.
Depends on: 1215162
Depends on: 1215158
Alias: compact-tabs
Depends on: 1215232
See Also: 1215232
Flags: needinfo?(martyn.haigh+bugzilla)
Assignee: nobody → twointofive
Comment on attachment 8815432 [details] Bug 1202861 - 1. Create compact (two column) tabs tray option for portrait mode phones. https://reviewboard.mozilla.org/r/96334/#review96808 I used it on my phone and it's great \o/ I think we can land this, but we should change the default for now. We are currently thinking about moving this behind switchboard flags to enable/disable this for a random sample of users and see how they react. But for now we can just land it disabled by default. ::: mobile/android/base/java/org/mozilla/gecko/tabs/AutoFitTabsGridLayout.java:17 (Diff revision 1) > +import android.content.res.Resources; > +import android.support.v7.widget.GridLayoutManager; > +import android.util.AttributeSet; > +import android.util.Log; > + > +class AutoFitTabsGridLayout extends TabsGridLayout { Random thought: Ignoring all the different spacing values and if we get a design that accommodates this: Could we at some point have a single layout that supports 1, 2 or dynamic columns? Apart from a different tab layout in the 1 column case this "feels" like it's the same grid only with a different number of columns. ::: mobile/android/base/java/org/mozilla/gecko/tabs/TabsPanel.java:79 (Diff revision 1) > > if (HardwareUtils.isTablet() || isLandscape) { > - return new TabsGridLayout(context, attrs); > + return new AutoFitTabsGridLayout(context, attrs); > + } else { > + // Phone in portrait mode. > + if (GeckoSharedPrefs.forApp(context).getBoolean("android.not_a_preference.compact_tabs", true)) { Use the pref constant here! (And see below for default) ::: mobile/android/base/resources/xml/preferences_general.xml:36 (Diff revision 1) > + <SwitchPreference android:key="android.not_a_preference.compact_tabs" > + android:title="@string/pref_compact_tabs" > + android:summary="@string/pref_compact_tabs_summary" > + android:defaultValue="true" > + android:persistent="true"/> Product (barbara) and UX (antlam) are currently discussing whether we are going to make this the default or not. But I don't want to block landing this, so maybe we should just make it off by default for now. And then later change the default again in a follow-up bug.
Attachment #8815432 - Flags: review?(s.kaspari) → review+
Comment on attachment 8815433 [details] Bug 1202861 - 2. Refresh tabs panel when compact tabs configuration changes. https://reviewboard.mozilla.org/r/96336/#review96792 ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1430 (Diff revision 1) > + if (mTabsPanel != null) { > + mTabsPanel.unregisterListeners(); > + } Instead of doing this here, maybe it makes more sense to do it in the view in onAttachedToWindow() / onDetachedFromWindow()? ::: mobile/android/base/java/org/mozilla/gecko/tabs/TabsPanel.java:51 (Diff revision 1) > implements GeckoPopupMenu.OnMenuItemClickListener, > LightweightTheme.OnChangeListener, > - IconTabWidget.OnTabChangedListener { > + IconTabWidget.OnTabChangedListener, > + SharedPreferences.OnSharedPreferenceChangeListener { > private static final String LOGTAG = "Gecko" + TabsPanel.class.getSimpleName(); > + private static final String PREF_COMPACT_TABS_KEY = "android.not_a_preference.compact_tabs"; We keep most of them in GeckoPreferences. There's also a constant for "android.not_a_preference.".
Attachment #8815433 - Flags: review?(s.kaspari) → review+
Comment on attachment 8815433 [details] Bug 1202861 - 2. Refresh tabs panel when compact tabs configuration changes. https://reviewboard.mozilla.org/r/96336/#review96792 > Instead of doing this here, maybe it makes more sense to do it in the view in onAttachedToWindow() / onDetachedFromWindow()? To be honest it's not clear to me when/under what circumstances onDetachedFromWindow for TabsPanel can get called, but testing looks good, so I've made the switch.
Comment on attachment 8815432 [details] Bug 1202861 - 1. Create compact (two column) tabs tray option for portrait mode phones. https://reviewboard.mozilla.org/r/96334/#review96808 > Random thought: Ignoring all the different spacing values and if we get a design that accommodates this: Could we at some point have a single layout that supports 1, 2 or dynamic columns? Apart from a different tab layout in the 1 column case this "feels" like it's the same grid only with a different number of columns. I'm not sure exactly what you mean, but basically I think the answer is yes. We could (probably) have a single TabsLayout class that determines, based on phone vs. tablet, portrait vs. landscape, dimensions available, and a range of permissible inter-tab spacings, how many "ideal"-width tabs we can fit across, and then sets things accordingly. It might not give desired span counts in all cases though without some extra guidance (is my guess). Implementation-wise it would help if we just did the current LinearLayoutManager case using a one-column GridLayoutManager, and it would also help (i.e. simplify things) if we always allowed a range of spacing options between tabs (unlike the current compact tabs, where the inner spacing is fixed equal to the outer padding).
Pushed by twointofive@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4de97a01678b 1. Create compact (two column) tabs tray option for portrait mode phones. r=sebastian https://hg.mozilla.org/integration/autoland/rev/88e0fb654a10 2. Refresh tabs panel when compact tabs configuration changes. r=sebastian
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Verified as fixed in Nightly build 53.0a1 2017-01-09; Devices: -LG G4 Android 5.1; -Lenovo A536 Android 4.4.2; Verified there's no change made on tablets, on: Asus ZenPad (Android 6.0.1).
Status: RESOLVED → VERIFIED
Hi, This two columns layout is by any chance conditioned by the device's screen width?
Flags: needinfo?(s.kaspari)
QA Contact: oana.horvath
Hi Mihai, If the "Compact tabs" setting is on and you're viewing the tabs tray on a phone in portrait mode, then two-column (compact) tabs should be displayed no matter what the screen width is. bug 1323952 made compact tabs an A/B experiment, so (if I understand correctly) 50% of users have the "Compact tabs" setting turned on by default, and the other 50% have it turned off by default. The compact tabs setting is under "Settings --> General --> Compact tabs" - you can change the setting by hand no matter which A/B bin you wound up in.
Flags: needinfo?(s.kaspari)
Verified on Aurora 53.0a2 the Compact tabs setting, the A/B test is working, and the tabs are looking good. No issues in tabs tray view & spacing. Devices: -LG G4 (Android 5.1) -OnePlus Two (Android 6.0) -Lenovo A536 (Android 4.4.2) -Motorola Nexus 6 (Android 7.1.1) -Asus Transformer Pad TF300T (Android 4.2.1) -Huawei MediaPad M2 - Android 5.1.1 More details about the testing done can be found in the feature's test plan: https://wiki.mozilla.org/QA/Fennec/Custom_tabs_in_settings
Tom, can you suggest a release note for this feature for 53 beta? Thanks!
Flags: needinfo?(twointofive)
Hello Oana, Thank you for your support Per your comment26, may I know if the compact tab has anything to do with custom-tab ? Because I found the wiki link of feature test plan(feature's test plan: https://wiki.mozilla.org/QA/Fennec/Custom_tabs_in_settings) is ended with Custom_tabs_in_settings ? Thank you for your support !!
Nope this has nothing to do with custom tabs. It's more or less a matter of feature parity with our iOS Fennec counter part. The same setting is in that product too. Hope this helps! :)
Hi Rachelle, it was my mistake writing the wiki title as "Custom tabs", instead of "Compact". Apologies for the confusion.
I'll take a shot, but welcome feedback from others listening on this bug. "Added a "Compact tabs" option to arrange tabs in two columns in portrait mode on phones. Currently enabled by default for 50% of users, the option can be turned on or off in Settings:General."
Flags: needinfo?(twointofive)
Verified in Beta 53.0b2 (2017-15-03), all tests have passed. Devices: -Prestigio Grace X5 (Android 4.4.2) -Huawei Honor 5X (Android 5.1.1) -LG G4 (Android 6.0) -Asus ZenPad 8.0 Z380KL (Android 6.0.1)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: