1202861 - (compact-tabs) Add "Compact tabs" settings item under "Customize"

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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.

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: 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

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(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.

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(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.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(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!

Comment 6

[Barbara Bermes [:barbara] - NI please!]

Can we please make sure that we add some probes to measure this, i.e. how many people use compact vs. old style?

Thanks :)

Comment 8

[Barbara Bermes [:barbara] - NI please!]

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.

Comment 9

[Edwin Wong [:edwong]]

+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.

Comment 10

[Edwin Wong [:edwong]]

Attachment: Screenshot_2015-10-08-13-38-26.png

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(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?

Comment 12

[Paul [pwd]]

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.

Comment 13

[Tom Klein]

Attachment: Bug 1202861 - 1. Create compact (two column) tabs tray option for portrait mode phones.

Changing the setting currently won't take effect until you rotate the tabs
panel (to cause it to be recreated); that will be fixed in the next commit.

Review commit: https://reviewboard.mozilla.org/r/96334/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96334/

Comment 14

[Tom Klein]

Attachment: Bug 1202861 - 2. Refresh tabs panel when compact tabs configuration changes.

Review commit: https://reviewboard.mozilla.org/r/96336/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96336/

Comment 15

[Sebastian Kaspari (:sebastian; :pocmo)]

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.

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

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.".

Comment 17

[Tom Klein]

Comment on attachment 8815432 [details]
Bug 1202861 - 1. Create compact (two column) tabs tray option for portrait mode phones.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96334/diff/1-2/

Comment 18

[Tom Klein]

Comment on attachment 8815433 [details]
Bug 1202861 - 2. Refresh tabs panel when compact tabs configuration changes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96336/diff/1-2/

Comment 19

[Tom Klein]

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 20

[Tom Klein]

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).

Comment 21

[Pulsebot]

Pushed by [e-mail]:
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

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4de97a01678b
https://hg.mozilla.org/mozilla-central/rev/88e0fb654a10

Comment 23

[Oana Horvath [:ohorvath]]

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).

Comment 24

[u549602]

Hi,

This two columns layout is by any chance conditioned by the device's screen width?

Comment 25

[Tom Klein]

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.

Comment 26

[Oana Horvath [:ohorvath]]

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

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Tom, can you suggest a release note for this feature for 53 beta? Thanks!

Comment 28

[Rachelle Yang [:ryang][[e-mail]]]

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 !!

Comment 29

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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! :)

Comment 30

[Oana Horvath [:ohorvath]]

Hi Rachelle, it was my mistake writing the wiki title as "Custom tabs", instead of "Compact". Apologies for the confusion.

Comment 31

[Tom Klein]

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."

Comment 32

[Oana Horvath [:ohorvath]]

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)