685839 - Implement tab list for portrait orientation in tablet mode

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Lucas Rocha (:lucasr)]

Mockup here: http://www.flickr.com/photos/61892693@N03/6052859709/in/photostream

Comment 1

[Wesley Johnston (:wesj)]

Attachment: WIP

This is what I like to call rough. But wanted to give a status update.

I've basically added an arrowbox here, and when we show the tab popup, we fill it in with the current tabs. I added some hacks to handle zombie tabs. I force this to close when you tap it right now because there is no spec on what to do there, and making sure that the favicons/titles update dynamically while this is shown is going to be more work. Beyond that, there's a few css hacks to get things to look like the mockups for now.

I'm highlighting the selected tab blueish, because it seems handy to know which one is selected.

build is at: http://people.mozilla.com/~wjohnston/fennec-tabs.apk

Comment 2

[Ian Barlow (:ibarlow)]

Attachment: mockup of menu

A few tweaks to the menu layout, most notably changing the "new tab" row to white, and adding a checkmark to indicate the active tab

Comment 3

[Ian Barlow (:ibarlow)]

Attachment: graphic assets: checkmark and new tab icon

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch v1

Ready for review (I hope). Screenshots on all three themes are at:

http://people.mozilla.com/~wjohnston/screenshots/honeycomb-tabsmenu/

I am basically copying the current list of tabs into this arrowbox each time we show it. If we have hundreds, that might be a problem. Right now that seems really unlikely though (maybe if we're ever running on Win8 Metro?).

I'm stealing data out of the session store right now to fill in info on zombie tabs. Eventually we should just store that somewhere more "public" on the tab I think. I also have to use the favicon service there because, when I printed out browser.__SS_data I didn't see any favicon info to use. The box is hidden if we resize into portrait mode (i.e. the sidebar shows).

The items have heights of -moz-calc(0.75 * @touch_row@). We could probably make that a define, but for now I just -moz-calced it all through here.

Had to do some strange things for ian's crazy borders. The borders are shorter than the width of the row, so I have to set them on an inner box, but they should stick outside of the row when it is selected. Right now, I am adding a margin on the outer box (so that I have some space where the tap highlight will not show) and using a negative margin on the inner box to shift the border outside (maybe a transform would be better?). That leaves a small 1px of extra space on the last item.

A couple follow ups to file. We should show this on mousedown, and allow dragging your finger to the item you want, lifting, and selecting it. Subsequent mousedown+drags should pan the list. Second, the titles and favicons shown here aren't dynamic. If the list is showing and they change, we don't update the list.

Comment 5

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

Comment on attachment 561076 [details] [diff] [review]
Patch v1

>   show: function show() {
>+    while(this.list.firstChild) {
>+      this.list.removeChild(this.list.firstChild);
>+    }
>+    let tabs = Browser.tabs;

nit: no {} and add a blank line after while loop

>+      if (browser.__SS_restore) {
>+        let entry = browser.__SS_data.entries[0];
>+        caption = entry.title;

Add a comment inside the if block about how we need to get some data from the session if the tab is a zombie


>+  closeTab: function(aTab) {
>+
>+  closeTabReturn: function(aMessage) {

Someday it might be nice to find a way to let BrowserUI handle this and TabsPopup just listens for an event?

>diff --git a/mobile/themes/core/images/checkmark.png b/mobile/themes/core/images/checkmark.png

nit: should we keep the -hdpi naming for this too?
also: how different is the images/check-30.png image?

>diff --git a/mobile/themes/core/tablet.css b/mobile/themes/core/tablet.css

>+arrowbox {
>+  -moz-stack-sizing: ignore;
>+}

nit: if this is not honeycomb only, maybe we should keep it near the other arrowbox stuff in platform.css

>+.documenttab-popup-checkmark {
>+  min-width: 26px; /* size of the checkmark image  plus the padding */
>+}

>+.documenttab-popup-checkmark {
>+  min-width: 26px; /* size of the checkmark image  plus the padding */
>+}

Remove the extra rule

r+, but fix the nits

Comment 6

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

Think abut testing too

Comment 7

[Wesley Johnston (:wesj)]

pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/7f35e447e8a9

> Someday it might be nice to find a way to let BrowserUI handle this and
> TabsPopup just listens for an event?
filing

> nit: should we keep the -hdpi naming for this too?
> also: how different is the images/check-30.png image?
renamed. the image is different. we could try to combine these...

> nit: if this is not honeycomb only, maybe we should keep it near the other
> arrowbox stuff in platform.css
this is important for the other themes in tablet mode as well (or any mode for that matter). I left it in. tell me if i need to kill it

Comment 8

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/7f35e447e8a9

Comment 9

[Cristian Nicolae (:xti)]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110922
Firefox/9.0a1 Fennec/9.0a1
Device: Acer ICONIA A500
OS: Android 3.1