817706 - Add sections for normal, private and synced tabs

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ian Barlow (:ibarlow)]

Now that we support different kinds of tabs, they should probably all have their own homes in the tab tray. 

Tablets: http://cl.ly/image/0s0W0u3L0s1h  
Phones: http://cl.ly/image/0V3N1G3r3A0p

The tab tray should only ever show what's actually open http://cl.ly/image/2O042447243K, since we don't want to clutter the UI with unnecessary empty buckets. If no private tabs are open, don't show a private tabs section. If sync is not set up, don't show a synced tabs section*

-------

*Note: I could see us potentially reconsidering this if we wanted to create more more awareness around Sync. For example, an empty synced tabs section could have some kind of prompt to encourage people to set up a Firefox Account. For the time being, though, let's just keep it simple though and not show empty lists.

Comment 1

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

I want to limit this bug to only adding the dropdown/tabs for selecting normal, private and sync tab lists. The mockup shows horizontal scrolling in portrait mode, which is a different bug.

Comment 2

[Ian Barlow (:ibarlow)]

That makes sense. I still want to make sure we implement this so we only ever show sections that have content in them. In other words, we don't show a Private tabs section, unless you actually have private tabs open.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: Part 1: Enhance TabsTray

This enhances the TabsTray to show either normal or private tabs. This is a small little attribute specified in the XML. The adapter uses the same to just updates the contents based on what is being shown.

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: Part 2: Tabs based switcher

This adds a tabs-based switcher for the tabs-panel. The unwanted text and remote tabs buttons are removed. The width expands to fill the available space. (This patch adds the same functionality for tablets to avoid breaking a build -- will be removed in next patch).

Comment 5

[Sriram Ramasubramanian [:sriram]]

Attachment: Part 3: Spinner switcher

This is a menu based switcher for tablets. This uses GeckoPopupMenu to inflate a menu list and show that on button's click. The button's text changes based on what tab-list is being shown.

It's good to use a spinner. However
1. It would be either white or black and we need to stylize it in themes.xml. We cannot do it until we plan to change all themes.
2. Spinner looks good starting honeycomb only. We need an assurance of some sort that we'll never show spinner on gingerbread and lower devices. If we do, it's better to fake it like this patch does.

Comment 6

[Sriram Ramasubramanian [:sriram]]

Note: This has a visual design based on mockups. There are no specs to complete the UI yet. They would be changed once the visual designs are ready.

Comment 7

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

Comment on attachment 693693 [details] [diff] [review]
Part 1: Enhance TabsTray

>diff --git a/mobile/android/base/resources/values/attrs.xml b/mobile/android/base/resources/values/attrs.xml

>+    <declare-styleable name="TabsTray">
>+        <attr name="privateOnly" format="boolean"/>
>+    </declare-styleable>

I'd like this better if we could use something like "tabType" and have two values: "normal" and "private"

Something like we do for "handleType". Is that easy enough to change?

r+ with that

Comment 8

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

Comment on attachment 693697 [details] [diff] [review]
Part 2: Tabs based switcher

I'm fine with landing this and then tweaking the visual design

Comment 9

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

Comment on attachment 693699 [details] [diff] [review]
Part 3: Spinner switcher

I'm fine with landing this and then tweaking the visual design

Comment 10

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

Try server run before landing please

Comment 11

[Sriram Ramasubramanian [:sriram]]

Changing it to an enum of type "tabType" is easy. However, I cannot use the word "private" as it is a reserved keyword in Java. Once the XML is parsed, it would get converted to something like:

private static final int private = 0x01;

which will fail in the build. Could we use some other name?

Comment 12

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/93e12399bae4
https://hg.mozilla.org/integration/mozilla-inbound/rev/296cc3ea6e9e
https://hg.mozilla.org/integration/mozilla-inbound/rev/7998f3adb4c5

Comment 13

[Sriram Ramasubramanian [:sriram]]

And I tried: https://tbpl.mozilla.org/?tree=Try&rev=309d36f10999

Comment 14

[Sriram Ramasubramanian [:sriram]]

Attachment: Part 4: Optimize

TabsTray can just be a ListView instead of enclosing a ListView inside a LinearLayout. Yaaay! One less view. I can change the name to something else too. TabsList may be.

Comment 15

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

Comment on attachment 694572 [details] [diff] [review]
Part 4: Optimize

This is a positive change, but I'm glad it's a separate patch... in case we get regressions :)

Comment 16

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c33bff7ed7ae

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/93e12399bae4
https://hg.mozilla.org/mozilla-central/rev/296cc3ea6e9e
https://hg.mozilla.org/mozilla-central/rev/7998f3adb4c5
https://hg.mozilla.org/mozilla-central/rev/c33bff7ed7ae