Closed Bug 817706 Opened 7 years ago Closed 7 years ago

Add sections for normal, private and synced tabs

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 20
Tracking Status
firefox20 --- verified

People

(Reporter: ibarlow, Assigned: sriram)

References

Details

Attachments

(4 files)

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.
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.
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.
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.
Attachment #693693 - Flags: review?(mark.finkle)
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).
Attachment #693697 - Flags: review?(mark.finkle)
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.
Attachment #693699 - Flags: review?(mark.finkle)
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 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
Attachment #693693 - Flags: review?(mark.finkle) → review+
Comment on attachment 693697 [details] [diff] [review]
Part 2: Tabs based switcher

I'm fine with landing this and then tweaking the visual design
Attachment #693697 - Flags: review?(mark.finkle) → review+
Comment on attachment 693699 [details] [diff] [review]
Part 3: Spinner switcher

I'm fine with landing this and then tweaking the visual design
Attachment #693699 - Flags: review?(mark.finkle) → review+
Try server run before landing please
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?
Attached patch Part 4: OptimizeSplinter Review
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.
Attachment #694572 - Flags: review?(mark.finkle)
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 :)
Attachment #694572 - Flags: review?(mark.finkle) → review+
Depends on: 824469
Status: RESOLVED → VERIFIED
OS: Mac OS X → Android
Hardware: x86 → ARM
Version: unspecified → Trunk
Depends on: 824501
Depends on: 826269
Depends on: 828329
Depends on: 825612
You need to log in before you can comment on or make changes to this bug.