Closed Bug 1025517 Opened 5 years ago Closed 5 years ago

[Homescreen] Search bar text box not centered vertically in container


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

Gonk (Firefox OS)
Not set


(b2g-v2.0 fixed, b2g-v2.1 verified)

2.0 S5 (4july)
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- verified


(Reporter: dmarcos, Assigned: crdlc)



(Whiteboard: [systemsfe])


(10 files, 1 obsolete file)

The magnifying glass is not vertically centered either inside the text box. See attached image.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-]
Attached image It seems fixed
Currently the bug is invalid, in the past obviously it made sense, closing
Closed: 5 years ago
Resolution: --- → INVALID
The magnifying is still not vertically centered.
Flags: needinfo?(crdlc)
Resolution: INVALID → ---
Comment on attachment 8441332 [details]
It seems fixed

Peter, please what is not centered here?
Flags: needinfo?(crdlc) → needinfo?(pla)
Attached image firefoxSearchBox.jpg
The magnifying glass is not centered inside the text box. This is the firefox search box.
In the firefox search box the magnifying class is clearly centered vertically
I want to know the Peter's opinion because this implementation was approved by him see bug 1016822 comment 27
No longer blocks: vertical-home-next
Can you please put a reason when unblocking? I was using the bug as a bucket for everything that would make it nicer, this is possibly one of those.
Hey Diego,

It was a design choice to reduce the length of the handle and center the icon on the circle (so that the circular part of the magnifying glass is concentric with the circle shape from the right end of outer pill shape).
Flags: needinfo?(pla)
Closed: 5 years ago5 years ago
Resolution: --- → INVALID
The search box is still not centered in container. Attaching new image
Resolution: INVALID → ---
Hey Diego, that is also by design so that it looks more balanced in the 'at rest' state in terms of spacing above and below the search pill.  In particular, this balances the gap between the top of the pill and the status bar icons, and the gap below the pill.

I am, however, considering removing the search box rectangle altogether, since it's interacting in a bad way with the scroll bar currently (and there isn't an easy workaround there).
We can play with CSS and when the box is in resting position at the top you reduce the the top margin/padding but when it's in "elastic over scrolling" mode you add that top margin so the box looks centered. I don't mind helping to polish this if nobody wants to jump on it.
Unfortunately we don't have good overscroll events that we can hook into yet :-/

I think the best option here is to consider removing the background backdrop of the search bar for 2.0. In 2.1 this is being moved to the system and should be less of a problem.
Diego, I'm going to investigate some options for this.  And look into centering the icon as well, as I've gathered some opinions from my team and some think it feels uncentered.  At first I thought it'd be cool to center on the circular elements, but I can see the other point of view as well.

Will try to post something soon.
I really appreciate that you tried to solve the design problem by thinking differently. I just misunderstood it. Vertical align can be tricky. Even Apple has issues. Look at the iOS search bar (attached picture). I still don't understand the logic for the vertical alignment. The cursor is not centered, the magnifying glass either, the lower case glyphs either, the upper cases either (same color squares have the same size).
Attached image iosSearchBar.jpg
Hi Diego.  Thanks for the example.  One thing to keep in mind when considering centering is that actual centering (like what you have called out in your iOS screenshot) vs centering based on the visual 'mass' or 'weight' of a particular shape are two different things.  Centering should be based on the latter, or visual weight, and sometimes that is open to interpretation.

After getting some feedback from my team mates, we've decided to center the search icon differently.  See next comments for spec + new icon asset.

Note, I did try to get rid of the rectangle beneath the search input field pill shape, but this would force me to lower the pill so that it has more breathing room next to the status bar.  This is a non-starter for 2.0 though as it would affect the full screen search layout.  We will revisit cleaning this up in 2.1 and 2.2.
Please use the new assets I'm attaching in the next comment.

Note that as usual, the icon labelled 2x is actually 1.6875x (qHD).  We'll need to address this inconsistency later in one pass.
Attached file
All 5 sizes included.
ni'ing Diego as a reminder.  Hope you still have time to look at this, and thanks for caring about the details!
Flags: needinfo?(dmarcos)
Assignee: nobody → crdlc
Attached image New implementation (obsolete) —
Is it ok? Thanks
Attachment #8447025 - Flags: ui-review?(pla)
Attached file Github pull request
Thanks Vivien for taking a look here
Attachment #8447031 - Flags: review?(21)
Comment on attachment 8447025 [details]
New implementation

Hi Cristian, great work as usual!

It looks perfect in terms of vertical position.  Horizontal position needs to move to the LEFT by about 1px before I can uireview+.

Attachment #8447025 - Flags: ui-review?(pla) → ui-review-
It looks great to me. I would love the screen of the flame to be brighter. Your design would shine much more.
Flags: needinfo?(dmarcos)
Attached image 1px to left
Attachment #8447025 - Attachment is obsolete: true
Attachment #8447846 - Flags: ui-review?(pla)
Whiteboard: [systemsfe]
Comment on attachment 8447846 [details]
1px to left

Perfect, thank you!
Attachment #8447846 - Flags: ui-review?(pla) → ui-review+
Merged in master:
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Comment on attachment 8447031 [details]
Github pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no regression
[User impact] if declined: visual improvement
[Testing completed]: no needed, ux+, changed the search icon in the rocketbar
[Risk to taking this patch] (and alternatives if risky): null
[String changes made]: no
Attachment #8447031 - Flags: approval-gaia-v2.0?(bbajaj)
Target Milestone: --- → 2.0 S5 (4july)
Comment on attachment 8447031 [details]
Github pull request

Approving the low rick css change keeping the ux-review that's granted on the change.
Attachment #8447031 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
Hey guys,
just want to ask if we use 3.375x image like this one:

the size need to be integer, right? but the size here is 2rem if I calculate right 2*3.375=6.75rem or 67.5px and we don't want this, right?

Sorry if I'm wrong
(In reply to Pavel Ivanov [:ivanovpavel] from comment #32)
> Hey guys,
> just want to ask if we use 3.375x image like this one:
> images/search_icon.png
> the size need to be integer, right? but the size here is 2rem if I calculate
> right 2*3.375=6.75rem or 67.5px and we don't want this, right?

I don't think the size needs to be integer? We have several 2.25x images for example. We were provided quite a few 3.375x images, I think because maybe due to the anticipation of full HD speced things. Although it currently looks like only the homescreen/grid code contains them.
Hey Kevin ... may not be expressed properly sorry for that ... I mean that if we have for example 24x24 image size ... and we will have this one:
@1x will be 24x1 = 24px
@2x will be 24*2 = 48px
@2.25x will be 24*2.25 = 54px
@3.375x will be 24*3.375 = 81px

Everything looks good

but if we have 30x30 image size the result will be:
@1x will be 30x1 = 30px
@2x will be 30*2 = 60px
@2.25x will be 30*2.25 = 67.5px
@3.375x will be 30*3.375 = 101.25px

and I'm not sure that `67.5px` and `101.25px` are good numbers for pixels or maybe I'm wrong?
I think we have loads of fractional sized images and styles all around the OS. I wouldn't worry about it unless someone brings something up, then let's do an audit. A quick grep found hundreds of pixel usage with decimals across gaia.
Attached image 009.png
This issue has been successfully verified on Flame 2.1:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Build-ID        20141203001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2
You need to log in before you can comment on or make changes to this bug.