[TABLETUI] Make a curvy tab menu in portrait mode

VERIFIED FIXED in Firefox 9

Status

defect
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: ibarlow, Assigned: lucasr)

Tracking

Firefox 9
Firefox 9
All
Android
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 4 obsolete attachments)

Posted image mockup
Let's make our portrait UI reference our nice tab shape we have in landscape mode, like this attached mock
Blocks: 655762
OS: Mac OS X → Android
Hardware: x86 → All
Blocks: 686417
Assignee: nobody → lucasr.at.mozilla
Sent while flying to NY! :-)
Attachment #560738 - Flags: review?(wjohnston)
This only implements the curvy design for the toolbar. I'll file a separate bug for the pressed state for the tabs button (it looks bad now). Bug 681805 can be used to track the new back and forward buttons design.
Attachment #560740 - Flags: review?(wjohnston)
Posted image Screenshot
Here's how it looks.
Comment on attachment 560740 [details] [diff] [review]
(2/2) Implement curvy design for tablet portrait mode

Review of attachment 560740 [details] [diff] [review]:
-----------------------------------------------------------------

As is, this doesn't work, but the fix is pretty easy. I'd like to hear from madhava/ibarlow and mfinkle about the color issue too.

::: mobile/themes/core/honeycomb/browser.css
@@ +364,1 @@
>  }

Welcome to the wonderful world of wesj hacks!

::: mobile/themes/core/honeycomb/defines.inc
@@ +29,1 @@
>  

I think we should avoid adding colors for very specific things like this. Our palette seems to big to me already. Can we use one of the colors we already have? color_text_header maybe? I know its not exactly the same, so lets ping ian/mfinkle about it.

::: mobile/themes/core/jar.mn
@@ +367,1 @@
>    skin/honeycomb/images/remotetabs-48.png             (honeycomb/images/remotetabs-48.png)

This name doesn't match what you're using in CSS.
Attachment #560740 - Flags: review?(wjohnston) → review+
Comment on attachment 560738 [details] [diff] [review]
Reduce main toolbar height to match planned design

Review of attachment 560738 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks!
Attachment #560738 - Flags: review?(wjohnston) → review+
Whoops. Foiled by splinter. Hollar if you have trouble figuring out what I was commenting on.
Comment on attachment 560740 [details] [diff] [review]
(2/2) Implement curvy design for tablet portrait mode

Review of attachment 560740 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/themes/core/honeycomb/defines.inc
@@ +29,1 @@
>  

I'm fine with adding specific color constants when no other constant matches, or this is a unique part of the UI. The panel-row-header (too general) class is an example too, from a different bug. Don't name it explicitly for the one UI button though.

However, if this situation is text on a black background, I would have thought we already have a similar constant available.

Maybe color-text-toolbutton ?
Just updated to apply to latest trunk. Keeping the review+.
Attachment #560738 - Attachment is obsolete: true
Attachment #561542 - Flags: review+
Attachment #561543 - Flags: review?(wjohnston)
Move the background switch to the tabs button itself. Use negative margin to move the back button a bit on top of the curvy button.
Attachment #560740 - Attachment is obsolete: true
Attachment #561545 - Flags: review?(wjohnston)
I really think using text placeholder color is a bad idea. So, I'm dropping the patch that changes color_text_placeholder and I'm just defining a more generic color color_text_toolbutton_inverse to use on the tabs button. Sorry for the spam.
Attachment #561543 - Attachment is obsolete: true
Attachment #561545 - Attachment is obsolete: true
Attachment #561543 - Flags: review?(wjohnston)
Attachment #561545 - Flags: review?(wjohnston)
Attachment #561546 - Flags: review?(wjohnston)
Comment on attachment 561546 [details] [diff] [review]
(3/3) Implement curvy design for tablet portrait mode

Review of attachment 561546 [details] [diff] [review]:
-----------------------------------------------------------------

Splendid!
Attachment #561546 - Flags: review?(wjohnston) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0348871e3c45
https://hg.mozilla.org/mozilla-central/rev/6355165a737b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110923
Firefox/9.0a1 Fennec/9.0a1
Device: Acer ICONIA A500
OS: Android 3.1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.