881777 - Search suggestion rows in new about:home are too tall

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

Just wondering if that's intended or just something that needs fixing. Sriram?

Comment 1

[Sriram Ramasubramanian [:sriram]]

They are having a height of 64dp, as per the spec.
http://hg.mozilla.org/projects/fig/file/72e003d38d00/mobile/android/base/resources/values/dimens.xml#l40

Comment 2

[Lucas Rocha (:lucasr)]

Ok. Maybe the search suggestions should have different height then? Ian, what's the expected height for search suggestion rows?

Comment 3

[Ian Barlow (:ibarlow)]

Can I see a screenshot of what it looks like in its current state?

Comment 4

[Lucas Rocha (:lucasr)]

Attachment: Screenshot 1

Comment 5

[Lucas Rocha (:lucasr)]

Attachment: Screenshot 2

Comment 6

[Lucas Rocha (:lucasr)]

Ian, maybe it's a matter of vertically centering the suggestions?

Comment 7

[Ian Barlow (:ibarlow)]

(In reply to Lucas Rocha (:lucasr) from comment #6)
> Ian, maybe it's a matter of vertically centering the suggestions?

Seems like a good first step ;)

You could also start the single line suggestions height to be the same as the URL/Action bar: http://cl.ly/image/0r1W362t3c0I

Comment 8

[Lucas Rocha (:lucasr)]

Attachment: (1/2) Use different height for search engine rows

Comment 9

[Lucas Rocha (:lucasr)]

Attachment: (2/2) Adjust favicon margins in SearchEngineRow

Comment 10

[Brian Nicholson (:bnicholson)]

(In reply to Lucas Rocha (:lucasr) from comment #9)
> Created attachment 761506 [details] [diff] [review]
> (2/2) Adjust favicon margins in SearchEngineRow

Looks like bug 880525. Think this patch can land on m-c?

Comment 11

[Lucas Rocha (:lucasr)]

(In reply to Brian Nicholson (:bnicholson) from comment #10)
> (In reply to Lucas Rocha (:lucasr) from comment #9)
> > Created attachment 761506 [details] [diff] [review]
> > (2/2) Adjust favicon margins in SearchEngineRow
> 
> Looks like bug 880525. Think this patch can land on m-c?

The code has changed quite a bit in fig. So, I'd have to write a slightly different patch for bug 880525.

Comment 12

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/projects/fig/rev/86cf65db4547
http://hg.mozilla.org/projects/fig/rev/d5ab1e6fced4

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/86cf65db4547
https://hg.mozilla.org/mozilla-central/rev/d5ab1e6fced4