1016822 - [Vertical Homescreen] Implement the RocketBar, homescreen input & status bar visuals properly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P1)

Description

[Cristian Rodriguez (:crdlc)]

This bug will implement:

* bug 1010651
* bug 1010647

Comment 2

[Cristian Rodriguez (:crdlc)]

Note bug 1010651 comment 26

Comment 3

[Cristian Rodriguez (:crdlc)]

Attachment: Homescreen

Homescreen

Comment 4

[Cristian Rodriguez (:crdlc)]

Attachment: Rocketbar

Comment 5

[Cristian Rodriguez (:crdlc)]

Attachment: Utility try displayed from home

Should the status bar be black when is displayed from homescreen? or 15% transparent?

Comment 6

[Cristian Rodriguez (:crdlc)]

Attachment: Github pull request

Hi mates, I've touched Homescreen, System and Rocketbar so I need some reviews. Thanks in advance

Comment 7

[Kevin Grandon :kgrandon]

Comment on attachment 8429994 [details]
Github pull request

Looks fine to me. I did not test with full rocketbar, but at this point we are so close to code complete - 2.0 should be the priority as long as tests pass.

Nice job!

Comment 8

[Cristian Rodriguez (:crdlc)]

I don't know what full rocketbar is so I don't know if I've introduced some regression in that part :(

Comment 9

[Dale Harvey (:daleharvey)]

You can see full rocketbar with HAIDA=1, this does regress it, so did my previous patch, as kevin said that is ok, we can fix them seperately and at this point we should most definitely not be blocking 2.0 nearing code complete on 2.1 functionality, the fixes for 2.1 would be needed anyway

Comment 10

[Dale Harvey (:daleharvey)]

Noticed a few bugs while reviewing, but they dont look related to this patch, checking them against master too, will be done with review shortly

Comment 11

[Cristian Rodriguez (:crdlc)]

I will test with HAIDA=1, I don't want to include regression for free :)

Comment 12

[Dale Harvey (:daleharvey)]

I think its out of scope to fix, we need to remove the e.me bar and fix homescreen sizing with rocketbar enabled

Comment 13

[Cristian Rodriguez (:crdlc)]

OK so is it for 2.1, right?

(In reply to Dale Harvey (:daleharvey) from comment #12)
> I think its out of scope to fix, we need to remove the e.me bar and fix
> homescreen sizing with rocketbar enabled

Comment 14

[Dale Harvey (:daleharvey)]

Yeh full rocketbar is 2.1

Comment 15

[Dale Harvey (:daleharvey)]

Comment on attachment 8429994 [details]
Github pull request

I think it was more my previous patch that regressed 2.1, but 2.0 takes priority and I can fix up 2.1, this is looking good, cheers

Comment 16

[Peter La]

Comment on attachment 8429986 [details]
Homescreen

Hi Cristian,

Thanks for this.  According to the spec, the 'Search or enter address' text should be white (#ffffff) and 50% opacity.  Your implementation looks too faint/grayed-out.

As part of your fix, can I ask that you deviate from the spec in terms of font weight, and set the 'Search or enter adress' text to Light Italic?  This is a refinement our team met about with Patryk and would like to see in the build.  This would also mean that for this change, https://bugzilla.mozilla.org/attachment.cgi?id=8429987&action=edit, we would like to see the same thing done with the font weight - Light Italic please.

Everything else looks great!

Thanks!

Comment 17

[Peter La]

Comment on attachment 8429987 [details]
Rocketbar

Hi Cristian,

Thanks for the work/screenshot.  It's really close but I'd like to see a couple of tweaks based on the spec (https://mozilla.box.com/s/wu5z0dd8glmyxlx2qncu).

1) Cursor color should be #00aacc, not white.
2) Cursor height should be 20px.  Width of 1px is fine (the spec above is actually wrong).
3) Divider line between input text and 'close' text should be 24px high.  It's currently a little too high at ~32px.
4) Can you set 'Search or enter address' text to Light Italic weight please?  This is to match up with our request to do the same for the homescreen.
5) Please set the color of 'Search or enter adress' to #e7e7e7 at 100% opacity.

Comment 18

[Peter La]

Hi Cristian,

Sorry, but in Comment #16 regarding attachment 8429986 [details], I made a mistake with the 'Search or enter address' color and opacity.  Please set it to #e7e7e7 at 100% opacity (and Light Italic weight).

Thanks!

Comment 19

[Peter La]

Comment on attachment 8429989 [details]
Utility try displayed from home

Hi Cristian,

I'm not sure what I am reviewing here.  It looks fine (just like the previous version).  The only thing I would want to change is the gradient on the bottom grey bar (with the icon toggle switches).  I think it should go flat grey to line up better with the visual refresh - but I will file a new bug for that.

Comment 20

[Cristian Rodriguez (:crdlc)]

Basically it was a ni :) Sorry. We have two scenarios:

1) Users are playing with an app (status bar is black) so if they pull down the utility tray this should be black for sure

but....

2) Users are playing with the homescreen (status bar is semi-transparent) so if they pull down the utility tray, should it be semi-transparent or black? IMHO the semi-transparent status bar when the utility tray is displayed is very weird :) So I decided that my patch will show the status bar as black in any case.

Thanks

(In reply to Peter La from comment #19)
> Comment on attachment 8429989 [details]
> Utility try displayed from home
> 
> Hi Cristian,
> 
> I'm not sure what I am reviewing here.  It looks fine (just like the
> previous version).  The only thing I would want to change is the gradient on
> the bottom grey bar (with the icon toggle switches).  I think it should go
> flat grey to line up better with the visual refresh - but I will file a new
> bug for that.

Comment 21

[Cristian Rodriguez (:crdlc)]

(In reply to Peter La from comment #16)
> Comment on attachment 8429986 [details]
> Homescreen
> 
> Hi Cristian,
> 
> Thanks for this.  According to the spec, the 'Search or enter address' text
> should be white (#ffffff) and 50% opacity.  Your implementation looks too
> faint/grayed-out.

The color is correct in the CSS -> color: rgba(255, 255, 255, .5);

> 
> As part of your fix, can I ask that you deviate from the spec in terms of
> font weight, and set the 'Search or enter adress' text to Light Italic? 
> This is a refinement our team met about with Patryk and would like to see in
> the build.  This would also mean that for this change,
> https://bugzilla.mozilla.org/attachment.cgi?id=8429987&action=edit, we would
> like to see the same thing done with the font weight - Light Italic please.
> 
> Everything else looks great!
> 
> Thanks!

will do for sure

Comment 22

[Cristian Rodriguez (:crdlc)]

ok, got, 10x

(In reply to Peter La from comment #18)
> Hi Cristian,
> 
> Sorry, but in Comment #16 regarding attachment 8429986 [details], I made a
> mistake with the 'Search or enter address' color and opacity.  Please set it
> to #e7e7e7 at 100% opacity (and Light Italic weight).
> 
> Thanks!

Comment 23

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

(In reply to Peter La from comment #17)
> Comment on attachment 8429987 [details]
> Rocketbar
> 
> Hi Cristian,
> 
> Thanks for the work/screenshot.  It's really close but I'd like to see a
> couple of tweaks based on the spec
> (https://mozilla.box.com/s/wu5z0dd8glmyxlx2qncu).
> 
> 1) Cursor color should be #00aacc, not white.

Hey Peter, unfortunately cursor icon is not customizable. We cannot change the color as Gecko is setting it to the same color of the font.

If you are interested in changing this color you should file a bug for the platform guys.

Comment 24

[Cristian Rodriguez (:crdlc)]

Attachment: Homescreen round 2

Comment 25

[Cristian Rodriguez (:crdlc)]

Attachment: Rocket bar round 2

The color and width of the caret cannot be defined via CSS. I could change the height playing with the input's height

Comment 26

[Peter La]

(In reply to Cristian Rodriguez (:crdlc) from comment #20)
> Basically it was a ni :) Sorry. We have two scenarios:
> 
> 1) Users are playing with an app (status bar is black) so if they pull down
> the utility tray this should be black for sure
> 
> but....
> 
> 2) Users are playing with the homescreen (status bar is semi-transparent) so
> if they pull down the utility tray, should it be semi-transparent or black?
> IMHO the semi-transparent status bar when the utility tray is displayed is
> very weird :) So I decided that my patch will show the status bar as black
> in any case.
> 
> Thanks
> 
> (In reply to Peter La from comment #19)
> > Comment on attachment 8429989 [details]
> > Utility try displayed from home
> > 
> > Hi Cristian,
> > 
> > I'm not sure what I am reviewing here.  It looks fine (just like the
> > previous version).  The only thing I would want to change is the gradient on
> > the bottom grey bar (with the icon toggle switches).  I think it should go
> > flat grey to line up better with the visual refresh - but I will file a new
> > bug for that.

Oooh, I see.  Yes, it makes sense to change it to black in this case.  I agree that transparet would look strange here.

Comment 27

[Peter La]

Comment on attachment 8430696 [details]
Homescreen round 2

Looks great, thanks Cristian!

Approving for now - although I've spoken to Cristian about an opacity problem with the placeholder text which isn't easily solvable immediately.

We'll raise a new bug for it if needed.

Comment 28

[Cristian Rodriguez (:crdlc)]

Peter, fixed, we have to set the opacity for the placeholder. See bug 556145 comment 14

Result: http://i.imgur.com/kAJsRl9.png

Comment 29

[Cristian Rodriguez (:crdlc)]

Finished. The placeholder text have to be dimmer in the rocketbar since you already invoked it and presumably you’ve already read the string

http://i.imgur.com/r3GvyGy.png

Comment 30

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8429994 [details]
Github pull request

Be careful not to introduce https://bugzilla.mozilla.org/show_bug.cgi?id=1010651#c27 again.

Comment 31

[Cristian Rodriguez (:crdlc)]

I tested it and I could not see it with this new patch. You?

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #30)
> Comment on attachment 8429994 [details]
> Github pull request
> 
> Be careful not to introduce
> https://bugzilla.mozilla.org/show_bug.cgi?id=1010651#c27 again.

Comment 32

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/mozilla-b2g/gaia/commit/93a74dfb2ad9a7fc8f5e5a7d79ae86dd6fe769c4

Comment 33

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: Screen Shot 2014-05-06 at 18.46.13.png

Hi Cristian,

Your commit made statusbar completely invisible in lockscreen (see my screenshot). Could check? thanks.

Comment 34

[Cristian Rodriguez (:crdlc)]

and is it wrong?

Comment 35

[Cristian Rodriguez (:crdlc)]

if it is wrong, please let mo know how the statusbar should be displayed and I fix that

Comment 36

[Cristian Rodriguez (:crdlc)]

ok, I have seen the problem, fixing it immediately

Comment 37

[Cristian Rodriguez (:crdlc)]

Fixed here bug 1018147

(In reply to John Lu [:mnjul] @MoCoTaipei from comment #33)
> Created attachment 8431496 [details]
> Screen Shot 2014-05-06 at 18.46.13.png
> 
> Hi Cristian,
> 
> Your commit made statusbar completely invisible in lockscreen (see my
> screenshot). Could check? thanks.

Comment 38

[Kevin Grandon :kgrandon]

Just making a note that for the regression in bug 1018147 I backed this out, then combined it with the fixed patch to ensure the commit is atomic. (Important in case we need to back it out again).

Reverted: https://github.com/mozilla-b2g/gaia/commit/eef3b756d8956e48d50b76752bb1d9548506e75e
Re-landed with two patches combined: https://github.com/mozilla-b2g/gaia/commit/c729c2fa9159c2211744b531a9ee8403e7174848

Comment 39

[Kevin Grandon :kgrandon]

Mass modify - set status-b2g-v2.0 fixed for fixed bugs under vertical homescreen dependency tree.