685285 - [TABLETUI] Make a curvy tab menu in portrait mode

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ian Barlow (:ibarlow)]

Attachment: mockup

Let's make our portrait UI reference our nice tab shape we have in landscape mode, like this attached mock

Comment 1

[Ian Barlow (:ibarlow)]

Attachment: graphic assets for curvy tab background

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: Reduce main toolbar height to match planned design

Sent while flying to NY! :-)

Comment 3

[Lucas Rocha (:lucasr)]

Attachment: (2/2) Implement curvy design for tablet portrait mode

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.

Comment 4

[Lucas Rocha (:lucasr)]

Attachment: Screenshot

Here's how it looks.

Comment 5

[Wesley Johnston (:wesj)]

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.

Comment 6

[Wesley Johnston (:wesj)]

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!

Comment 7

[Wesley Johnston (:wesj)]

Whoops. Foiled by splinter. Hollar if you have trouble figuring out what I was commenting on.

Comment 8

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

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 ?

Comment 9

[Ian Barlow (:ibarlow)]

Attachment: Mockup of tab selection state (turns blue)

Comment 10

[Ian Barlow (:ibarlow)]

Attachment: graphic asset for selected tab

Comment 11

[Lucas Rocha (:lucasr)]

Attachment: (1/3) Reduce main toolbar height to match planned design

Just updated to apply to latest trunk. Keeping the review+.

Comment 12

[Lucas Rocha (:lucasr)]

Attachment: (2/3) Change color_text_placeholder as per design

Comment 13

[Lucas Rocha (:lucasr)]

Attachment: (3/3) Implement curvy design for tablet portrait mode

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.

Comment 14

[Lucas Rocha (:lucasr)]

Attachment: (3/3) Implement curvy design for tablet portrait mode

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.

Comment 15

[Wesley Johnston (:wesj)]

Comment on attachment 561546 [details] [diff] [review]
(3/3) Implement curvy design for tablet portrait mode

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

Splendid!

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0348871e3c45
https://hg.mozilla.org/integration/mozilla-inbound/rev/6355165a737b

Comment 17

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/0348871e3c45
https://hg.mozilla.org/mozilla-central/rev/6355165a737b

Comment 18

[Cristian Nicolae (:xti)]

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