This bug will implement: * bug 1010651 * bug 1010647

Note bug 1010651 comment 26

Homescreen

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

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

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!

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

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

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

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

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

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

Yeh full rocketbar is 2.1

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 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 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.

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 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.

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.

(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

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!

(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.

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

(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 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.

Peter, fixed, we have to set the opacity for the placeholder. See bug 556145 comment 14 Result: http://i.imgur.com/kAJsRl9.png

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 on attachment 8429994 [details] Github pull request Be careful not to introduce https://bugzilla.mozilla.org/show_bug.cgi?id=1010651#c27 again.

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.

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

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

and is it wrong?

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

ok, I have seen the problem, fixing it immediately

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.

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

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