Closed Bug 1010651 Opened 6 years ago Closed 6 years ago

(vertical-homescreen) RocketBar missing search icon

Categories

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

All
Gonk (Firefox OS)
defect

Tracking

(feature-b2g:2.0)

RESOLVED DUPLICATE of bug 1016822
2.0 S2 (23may)
feature-b2g 2.0

People

(Reporter: pla, Assigned: crdlc)

References

Details

(Whiteboard: ux-tracking, visual design, visual-tracking, [ft:systemsfe][systemsfe])

Attachments

(6 files, 1 obsolete file)

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
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!
Flags: needinfo?(pla)
feature-b2g: --- → 2.0
Hi Kevin,

Here are the icons.

You can refer to this spec for placement of the icon:
https://mozilla.box.com/s/5qkt3dgs7ria4bhw6hcd
Flags: needinfo?(pla)
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!
Flags: needinfo?(crdlc)
ok will do, thanks a lot Kevin!
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Flags: needinfo?(crdlc)
Priority: -- → P1
Attached file Github pull request
Thanks for the review
Attachment #8426937 - Flags: review?(kgrandon)
Attachment #8426937 - Flags: review?(alive)
Attached image Screenshot of the patch (obsolete) —
Thanks a lot
Attachment #8426942 - Flags: ui-review?(pla)
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. :)
Attachment #8426942 - Flags: ui-review?(pla) → ui-review-
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. :)
Attached image 5px to right
Attachment #8426942 - Attachment is obsolete: true
Attachment #8427014 - Flags: ui-review?(pla)
Comment on attachment 8427014 [details]
5px to right

Perfect!!!  Thanks Cristian.
Attachment #8427014 - Flags: ui-review?(pla) → ui-review+
Thanks to you Peter!
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → All
Kevin's comment addressed
Whiteboard: ux-tracking, visual design, visual-tracking, [ft:systemsfe] → ux-tracking, visual design, visual-tracking, [ft:systemsfe][systemsfe]
Target Milestone: --- → 2.0 S2 (23may)
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 :(
Flags: needinfo?(crdlc)
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.
Attachment #8426937 - Flags: review?(kgrandon)
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
Flags: needinfo?(crdlc)
Comment on attachment 8426937 [details]
Github pull request

Fixed Kevin, thanks
Attachment #8426937 - Flags: review?(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?
Attachment #8426937 - Flags: review?(kgrandon)
Attachment #8426937 - Flags: review+
Attachment #8426937 - Flags: feedback?(dale)
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.
Attachment #8426937 - Flags: review?(alive)
Depends on: 1009401
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
Attachment #8426937 - Flags: feedback?(dale) → feedback+
Bug 1009401 already landed so working on it right now, thanks Dale and Kevin
Attachment #8426937 - Flags: review?(alive)
Hi Alive, I touched statusbar and system style sheets so I would like that you review the path too. Thanks a lot
Comment on attachment 8426937 [details]
Github pull request

r=me
Attachment #8426937 - Flags: review?(alive) → review+
Merged in master:

https://github.com/mozilla-b2g/gaia/commit/d984a2e9774791deb3c487463034e8c650a46f2f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1010647
@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
This bug make the status bar incorrectly overlap on the LockScreen window.
I'll fire another bug and NI assigner.
I've backed out the patch due to the symptom:

https://github.com/mozilla-b2g/gaia/commit/6be112bd2ed92daa1eea6c3c18ddaf15230cc5a5
Status: RESOLVED → REOPENED
Flags: needinfo?(crdlc)
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(crdlc)
Resolution: --- → DUPLICATE
Duplicate of bug: 1016822
You need to log in before you can comment on or make changes to this bug.