1010740 - Toolbar refinement - Change entry asset and tweak padding

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Screen Shot 2014-05-15 at 11.15.33 AM.png

Current toolbar (bottom) compared to the refined toolbar (top). 

Mockup is to depict overall positioning and layout of icons, tab curve, and input field changes (border radius, padding, border  etc...)

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: urlbar_dot9.zip

Attaching .9 files for the URL bar in both active and default states

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: URL bar 4.png

Attaching specs for spacing in dp units

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: urlbar_dot9.zip

Updated to fix .9 file bug

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: urlbar_dot9.zip

Border-radius refinements

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: urlbar_dot9.zip

moar radius!

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_toolbar_states2.png

Just updating the preview of the changes as per our discussions, Lucas :) (x was too contrasty, etc)

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_toolbar_states3_specs.png

Played with static images on a Nexus 5 and the uneven spacing didn't come across as well as it did on a laptop. Attaching updated measurements :) maybe we can give these a try.

Comment 9

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: spec_toolbar_x.png

Figured this might help too.

Comment 10

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

^ icon can be found in bug 1014848

Comment 11

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: Screen Shot 2014-07-17 at 10.20.00 AM.png

this is REALLLLLY close! (attaching a visual comparison)

On the curve: I seem to have lost the values we're using for the cubic-bezier but if you let me know I can tune it again for you. I think the bottom half of the curve needs to stick out (to the right) just a bit more...

On the input bar: the size is just a bit too big, I think this contributes to making it feel too cramped.

Comment 12

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: v3_compare.mov

Also, not sure if this helps Lucas, but I've overlay-ed the current build on top of the design file and just switched back and forth to compare the differences. (here's a MOV file!)

Comment 13

[Lucas Rocha (:lucasr)]

Attachment: Update icon for the edit cancel button (r=mcomella)

Comment 14

[Lucas Rocha (:lucasr)]

Attachment: Change appearance of input entry in toolbar (r=mcomella)

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8471534 [details] [diff] [review]
Update icon for the edit cancel button (r=mcomella)

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

lgtm w/ nits considered.

::: mobile/android/base/resources/drawable/close_edit_mode_selector.xml
@@ +9,5 @@
>      <item gecko:state_dark="true"
>            android:drawable="@drawable/close_edit_mode_dark"/>
>  
> +    <item gecko:state_private="true"
> +          android:drawable="@drawable/close_edit_mode_light"/>

"_light" refers to the background color, i.e. this drawable is darker than close_edit_mode_dark. Since private mode has a dark background, are you sure that this should be light?

Semi-relatedly, I'm not sure if this convention makes sense (it took me a lot longer to grow the intuition for it just now than if it was reversed, and I wrote it!) and bring up the option of changing it.

::: mobile/android/base/toolbar/BrowserToolbar.java
@@ +1381,3 @@
>          urlEditLayout.setPrivateMode(isPrivate);
>  
> +

nit: excess ws.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8471539 [details] [diff] [review]
Change appearance of input entry in toolbar (r=mcomella)

Perhaps I'm missing a patch from an unmarked dependent bug, but it appears that the curve hasn't been changed and, with the reduction of the tabs counter's padding, it looks pretty strange.

However, I trust that you know what's going on here and I don't want to waste more cycles when I'm confident you know what's going on here, so r+.

Comment 17

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #15)
> Comment on attachment 8471534 [details] [diff] [review]
> Update icon for the edit cancel button (r=mcomella)
> 
> Review of attachment 8471534 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm w/ nits considered.
> 
> ::: mobile/android/base/resources/drawable/close_edit_mode_selector.xml
> @@ +9,5 @@
> >      <item gecko:state_dark="true"
> >            android:drawable="@drawable/close_edit_mode_dark"/>
> >  
> > +    <item gecko:state_private="true"
> > +          android:drawable="@drawable/close_edit_mode_light"/>
> 
> "_light" refers to the background color, i.e. this drawable is darker than
> close_edit_mode_dark. Since private mode has a dark background, are you sure
> that this should be light?

Yep, close_edit_mode_dark looks a bit too constrasty. We'll only use close_edit_mode_dark when the theme is dark to ensure it's visible with arbitrary background images.

> Semi-relatedly, I'm not sure if this convention makes sense (it took me a
> lot longer to grow the intuition for it just now than if it was reversed,
> and I wrote it!) and bring up the option of changing it.

I agree it's confusing. Our styles desperately need some major cleanups... 

> ::: mobile/android/base/toolbar/BrowserToolbar.java
> @@ +1381,3 @@
> >          urlEditLayout.setPrivateMode(isPrivate);
> >  
> > +
> 
> nit: excess ws.

Fixed.

Comment 18

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #16)
> Comment on attachment 8471539 [details] [diff] [review]
> Change appearance of input entry in toolbar (r=mcomella)
> 
> Perhaps I'm missing a patch from an unmarked dependent bug, but it appears
> that the curve hasn't been changed and, with the reduction of the tabs
> counter's padding, it looks pretty strange.

Yeah, lots of interdependent changes here, apologies. It will be alright in the end, promise :-)

Comment 19

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/aaa50f34df6d
https://hg.mozilla.org/integration/fx-team/rev/6908f8780c29

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Backed out along with the rest of the patch stack for robocop failures.
https://hg.mozilla.org/integration/fx-team/rev/d18d3e14ac3e

Comment 21

[Lucas Rocha (:lucasr)]

The string changes in bug 1010741 caused the failures. Forgot to update the tests accordingly. Try build looks nice and green now, pushed:

https://hg.mozilla.org/integration/fx-team/rev/0a0c695b4576
https://hg.mozilla.org/integration/fx-team/rev/53ad0481f772

Comment 22

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0a0c695b4576
https://hg.mozilla.org/mozilla-central/rev/53ad0481f772