1010651 - (vertical-homescreen) RocketBar missing search icon

Status: RESOLVED DUPLICATE of bug 1016822

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

Description

[Peter La]

There should be a search icon (magnifying glass) on the right-hand side of the search bar on homescreen.

Please refer to this spec:

https://mozilla.box.com/s/5qkt3dgs7ria4bhw6hcd

Comment 1

[Kevin Grandon :kgrandon]

Peter - having trouble accessing files on box for some reason. Can you please attach the magnifying glass asset to this bug for the usual required sizes? (@1x, @1.5x, @2x, @2.25x). Thanks!

Comment 2

[Peter La]

Attachment: RocketBar Search Icons @1x, 1.5x, 2x, 2.25x, and 3.375x (full HD)

Hi Kevin,

Here are the icons.

You can refer to this spec for placement of the icon:
https://mozilla.box.com/s/5qkt3dgs7ria4bhw6hcd

Comment 3

[Kevin Grandon :kgrandon]

Attachment: Relevant spec page

Comment 4

[Kevin Grandon :kgrandon]

Hey Cristian - This is some fairly simple visual polish that needs to be done. The search bar is a bit weird because it is just a proxy for the system app right now, but I think we can implement this only in the homescreen.

Let me know if you want to take this simple one, if not no worries. Thanks!

Comment 5

[Cristian Rodriguez (:crdlc)]

ok will do, thanks a lot Kevin!

Comment 6

[Cristian Rodriguez (:crdlc)]

Attachment: Github pull request

Thanks for the review

Comment 7

[Cristian Rodriguez (:crdlc)]

Attachment: Screenshot of the patch

Thanks a lot

Comment 8

[Peter La]

Comment on attachment 8426942 [details]
Screenshot of the patch

Hi Cristian, thanks for the screenshot.  The positioning is slightly off - if you can just move the icon 5 pixels to the right, it'll be perfect, thanks. :)

Comment 9

[Cristian Rodriguez (:crdlc)]

Yes I can, in a couple of minutes, thanks for the feedback!

(In reply to Peter La from comment #8)
> Comment on attachment 8426942 [details]
> Screenshot of the patch
> 
> Hi Cristian, thanks for the screenshot.  The positioning is slightly off -
> if you can just move the icon 5 pixels to the right, it'll be perfect,
> thanks. :)

Comment 10

[Cristian Rodriguez (:crdlc)]

Attachment: 5px to right

Comment 11

[Peter La]

Comment on attachment 8427014 [details]
5px to right

Perfect!!!  Thanks Cristian.

Comment 12

[Cristian Rodriguez (:crdlc)]

Thanks to you Peter!

Comment 13

[Cristian Rodriguez (:crdlc)]

Kevin's comment addressed

Comment 14

[Kevin Grandon :kgrandon]

Attachment: Alignment problem - nexus4.png

Hey Cristian - I'm seeing an alignment issue on my nexus4 device. Do you have a high resolution device to test with?

It's a bit hard to see, but in this example, you can see the homescreen search bar when you are focused on the system one. Ideally there should be no visible movement when you enter the system search app.

I know this code is a bit crazy right now for 2.0 due to the duplicated styles. It makes simple stuff like this difficult :(

Comment 15

[Kevin Grandon :kgrandon]

Comment on attachment 8426937 [details]
Github pull request

Going to clear the review until we fix the tiny alignment issue. Let me know if you don't have a high resolution device - I could also do some playing with the app-manager to see what's causing it.

Comment 16

[Cristian Rodriguez (:crdlc)]

Yes, I know :) I gonna fix it in search app. I didn't it because of comment 4. I thought that we have to work only in homescreen and we will work later in search app. I gonna implement it today, thanks

Comment 17

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8426937 [details]
Github pull request

Fixed Kevin, thanks

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 8426937 [details]
Github pull request

This looks good to me, but I am a bit concerned about the transparent statusbar work in here. I think Dale also might be working on something. (I know he's got the transparent search app background working which will make this much better).

Dale - before we land this could you take a look and let us know what you think?

Comment 19

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8426937 [details]
Github pull request

Well, after talking with Dale we have decided what the bug 1009401 will be landed before this one. Once done that, I will do the changes only for the homescreen instead of touching system and homescreen. For this reason, I've removed the Alive's review.

Comment 20

[Dale Harvey (:daleharvey)]

Comment on attachment 8426937 [details]
Github pull request

Yup, talked with Christian on IRC, this patch looks great but its a bit difficult to split out the changes to the statusbar from the app_windows as they both make similiar changes in a different way, I think it will be best to land the visual refresh changes then rebase this on top

One other thing I should mention, there is an obvious issue with making the statusbar transparent, people may have white backgrounds rendering statusbar information invisible, android solves this by using a black gradient instead of a fully transparent bar

Comment 21

[Cristian Rodriguez (:crdlc)]

Bug 1009401 already landed so working on it right now, thanks Dale and Kevin

Comment 22

[Cristian Rodriguez (:crdlc)]

Hi Alive, I touched statusbar and system style sheets so I would like that you review the path too. Thanks a lot

Comment 23

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

Comment on attachment 8426937 [details]
Github pull request

r=me

Comment 24

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/mozilla-b2g/gaia/commit/d984a2e9774791deb3c487463034e8c650a46f2f

Comment 26

[Peter La]

@Cristian @Dale

Just to clarify, the original intent of this particular bug was just to put the search icon in.  The transparency work should have been done in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1010647

However, since you guys have already coordinated on just how to separate things, I'm fine with however you want to divide up the work.

And a note about the transparent status bar - it's not 100% transparent.  According to the spec, there is a 15% black rectangle occupying the status bar area, so even if the user sets a fully white wallpaper, you should still be able to make out the status bar elements:
https://mozilla.box.com/s/5qkt3dgs7ria4bhw6hcd

Comment 27

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Wrong statusbar upon the lockscren window

This bug make the status bar incorrectly overlap on the LockScreen window.
I'll fire another bug and NI assigner.

Comment 28

[Greg Weng [:snowmantw][:gweng][:λ]]

I've backed out the patch due to the symptom:

https://github.com/mozilla-b2g/gaia/commit/6be112bd2ed92daa1eea6c3c18ddaf15230cc5a5