Closed Bug 685839 Opened 8 years ago Closed 8 years ago

Implement tab list for portrait orientation in tablet mode

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 9

People

(Reporter: lucasr, Assigned: wesj)

References

Details

Attachments

(3 files, 1 obsolete file)

Blocks: 655762
Attached patch WIP (obsolete) — Splinter Review
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
Assignee: nobody → wjohnston
Blocks: 686938
Blocks: 686983
Blocks: 686417
Attached image 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
Attached patch Patch v1Splinter Review
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.
Attachment #559627 - Attachment is obsolete: true
Attachment #561076 - Flags: review?(mark.finkle)
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
Attachment #561076 - Flags: review?(mark.finkle) → review+
Think abut testing too
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
Whiteboard: [inbound]
Blocks: 687761
https://hg.mozilla.org/mozilla-central/rev/7f35e447e8a9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 9
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
Status: RESOLVED → VERIFIED
Depends on: 689682
You need to log in before you can comment on or make changes to this bug.