1207739 - Quick search icons have too much padding

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect)

Description

[Robin Andersen [:tecgirl]]

Attachment: search-icon-padding.zip

The spacing between the quick search icons is too great. Currently 67dp, decrease to 44dp.

See attached .zip for current layout and a mock for the improvement.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

This was actually intentionally done in bug 1158291 to hint at the ability to scroll sideways in this part of the UI. And from that work, the spacing is dynamically determined depending on what phone the user has. That way, there's always one icon just off screen.

Comment 2

[Robin Andersen [:tecgirl]]

We could still lessen the padding and still use this logic though, yes?

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Currently the clickable area (icon + padding) has a minimum width of 72dp and we increase the padding dynamically to cut off the last item.  We could reduce the minimum width in order to show more icons (denser) and still cut-off the last one.

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Robin Andersen [:tecgirl] from comment #2)
> We could still lessen the padding and still use this logic though, yes?

Maybe. But I'm uncomfortable going below 72dp minimum as it starts to get really cluttered. Since 48dp is the standard "measurement" for general tap targets, etc in our UI and I got to 72 by multiplying by 1.5 so it's nice and proportional.

(In reply to Sebastian Kaspari (:sebastian) from comment #3)
> Currently the clickable area (icon + padding) has a minimum width of 72dp
> and we increase the padding dynamically to cut off the last item.  We could
> reduce the minimum width in order to show more icons (denser) and still
> cut-off the last one.

Since the Nightly build in the screenshot has a bug, looking at the Beta screenshot, showing 1 more icon would get too crowded.

I'd prefer to keep the min. width here for now.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Anthony Lam (:antlam) from comment #4)
> I'd prefer to keep the min. width here for now.

WONTFIX?

Comment 6

[cousteau]

Attachment: Screenshot on a Samsung Galaxy Trend Plus (800x480, 4")

The spacing still looks excessive to me.

If the standard measure for tap targets is 48dp, and the spacing between search icons is 72dp, plus 24dp(?) of the icon size itself, that makes 96dp in total per button (0.6in if I made the math right); this is, twice as much as the "standard measure".  So theoretically you could fit twice as many search engines if you followed this 48dp rule.

If you wanted to apply this 1.5x rule then that'd be 48dp+24dp, which I think would be a sensible choice.

Attached is a screenshot of how this currently looks on my cellphone (which has a 4" screen): after the stretching, only 2½ icons are shown, and it feels like a lot of space has been wasted unnecessarily.  Notice how the spacing between 2 icons is about 3.5 keyboard keys.  If the 72dp spacing were reduced to 48 that'd be 3½ icons after stretching, which makes more sense.

Also, keep in mind that this 48dp or 72dp spacing is the *worst case* one; the average one would be larger.

(Maybe instead of rounding down the number of icons they should be rounded to the nearest .5, allowing icons to become narrower or wider to get as close as possible to the desired optimal width, rather than always becoming wider.)

Comment 7

[cousteau]

Attachment: 24x24 icons, with padding twice as wide as the icons

Never mind my previous comment; I thought the 72dp were the padding only, not including the icon.

This leads to the question of why are my icons so separated.  If I understood correctly, the orange and black icons in attachment 8727860 [details] would have a size of 24x24dp (which check out assuming that 1in=160dp; I did the math).  However that'd mean that with a 72dp width, the minimum padding between icons should be 48dp, i.e. twice as large as an icon.

In this new attachment, you can see how if this were true there would be enough space for 1 more icon (plus some extra margin on the left).  Therefore, either my cellphone's dp are not 1/160 of an inch and the icons are not 24x24dp (because they don't use the full 24x24 box), or the 72dp+stretching rule isn't being applied correctly.

Regarding the search engine bar being "too crowded", if what you mean is that there are too many icons then that sounds like the solution would be to restrict the number of icons directly, not the icon size, am I right?

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

What device are you using? this could be a bug. But I can't tell for sure just from your screenshots. 

Perhaps if you enable the layout bounds we would be able to get a better sense of whats going on here.

NI-ing sebastian after comment 7^

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: searchenginebar-bug.png

I have drawn the padding on top of the screenshot. This is definitely buggy. The container around the icons should have grown to use the whole available space. Instead they seem to be still the minimum size and there's weird gap on the left.

Comment 10

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Sebastian Kaspari (:sebastian) from comment #9)
> Created attachment 8732281 [details]
> searchenginebar-bug.png
> 
> I have drawn the padding on top of the screenshot. This is definitely buggy.
> The container around the icons should have grown to use the whole available
> space. Instead they seem to be still the minimum size and there's weird gap
> on the left.

Yes, that gap shouldnt be there...

Comment 11

[cousteau]

(In reply to Sebastian Kaspari (:sebastian) from comment #9)

Sorry if I wasn't clear.  That last "screenshot" was a mockup demonstrating that there was enough space for one more icon, not an actual screenshot (hence the duplicate icons). So the extra gap is there only to show that there is still small extra space even after adding the extra icon.

I'll try to enable the overlay if I figure out how to do it.

Comment 12

[cousteau]

Attachment: Screenshot_2016-03-25-11-44-47.png

Comment 13

[cousteau]

Attachment: Layout boundaries of the actual screenshot

As you can see (I believe; I can't check pixel wise right now) there should be room for one more icon.  (If I understood correctly, 72dp = 3 icons.)

Comment 14

[cousteau]

So after checking the screenshot pixel-wise:
(1) An icon has a size of 36x36 px, and a theoretical size of 24x24 dp, therefore 1 dp = 36/24 = 1.5 px (on my device).
(2) A search engine "slot" should be at least 72 dp (= 108 px) wide.
(3) There are 407 px on my device to display search engines.
(4) Theoretically, it should be able to display 407/108 = 3.77 search engines, which would round to 3.5 search engines of 116 px each (116 > 108).
(5) In practice, it displays 2.5 search engines of 163 px each.

In short, my phone displays 2.5 search engines but it should be able to display 3.5 (unless I missed something).  Is this normal?

Comment 15

[cousteau]

...never mind.  Apparently the algorithm (bug 1158291) is

> * If we can only display (n) search engine icons then display (n - 0.5) search engine icons and again dynamically determine the width to use all available space.

This means that for my particular case the 3.77 is rounded down to 3 and then to 2.5.  I would argue that the way to go would be to round down to the nearest (smaller) n.5, not to the nearest n.0 and then get (n-1).5, otherwise it diverges unnecessarily from the "minimum of 72dp".

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

Thanks for bringing this up again on IRC. I'll prepare a patch for that.

Comment 17

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: searchenginebar_spacing.png

This is a screenshot of the fixed version.

Comment 18

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1207739 - SearchEngineBar: Fix calculation to determine number of search engines to display.

If we cannot display all search engines then we want to show a smaller number so that the last one is
cut-off (we only display half of it). The previous version did not calculate the next smallest half
correctly: Instead of displaying 3.5 engines if we have space for 3.7 engines, we only displayed 2.5.

Review commit: https://reviewboard.mozilla.org/r/57920/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57920/

Comment 19

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8760279 [details]
Bug 1207739 - SearchEngineBar: Fix calculation to determine number of search engines to display.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57920/diff/1-2/

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8760279 [details]
Bug 1207739 - SearchEngineBar: Fix calculation to determine number of search engines to display.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57920/diff/2-3/

Comment 21

[Andrzej Hunt :ahunt]

Comment on attachment 8760279 [details]
Bug 1207739 - SearchEngineBar: Fix calculation to determine number of search engines to display.

https://reviewboard.mozilla.org/r/57920/#review54768

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/914265a547bf
SearchEngineBar: Fix calculation to determine number of search engines to display. r=ahunt

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/914265a547bf

Comment 24

[Sebastian Kaspari (:sebastian; :pocmo)]

cousteau: Can you verify that this is fixed for you with the latest Nightly build[1]? :)

[1] https://nightly.mozilla.org/

Comment 25

[cousteau]

Now it looks perfect, thanks!

(I was thinking that maybe if instead of 3.77 it were 4.33, as is the case for many 4.5" screens, it may be worth considering to round them to 4.5 instead of 3.5 so that they are a bit compressed, but on a second thought that might look too ugly.)

Comment 26

[Sorina Florean [:sflorean]]

Setting the flag verified in build 50.0a1, based on Comment 25.