1125050 - Big space between thumbnail rows on Kindle Fire

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Cristina Madaras, QA [:CristinaM]]

Attachment: tabs.png

Environment:
Device: Kindle Fire HD 7"
Build: Firefox for Android 38.0a1 (2015-01-22)

Steps to reproduce:
1. Open some new tabs;
2. Make sure your device is in portrait orientation and open the tabs menu.

Expected result:
Tabs thumbnails are correctly displayed in tabs menu.

Actual result:
The space between thumbnails rows is too big.

Please see the attachment.

Comment 1

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

Probably not worth working on right now if it's just Kindle but Martyn, do you know what's going on here? Can this affect devices other than the Kindle?

Comment 2

[Martyn Haigh (:mhaigh)]

The problem you're seeing here is that the GridView is a fairly brittle UI component.  My thoughts are that we are able to specify left and right padding and also minimum padding between items, but if there isn't enough space to insert another item then we end up with a massive gap in the middle, like you see here.

We need some UI input as this may affect other tablets too, depending on screen dimensions.  

We originally planned this on a N7 which has a screen dimension of 1280x800, the same as the Kindle Fire HD 7", but where the N7 has a DPPX ratio of 1.325, the Fire has a DPPX ratio of 1, hence the tabs take up more room on the Fire than the N7.

To my untrained eye, it looks like it'd be good if we could center the items in the available space but without jumping in to code and possible rooting a device to be able to change pixel ratios, I'm unsure exactly how much work we're looking at for this.

It'd be good to get number on how many Fire HD users we have - may be worth looking for other tablets which could be problematic in our top devices (mcomella: know how to get these figures?)

Anthony: What're your thoughts?

Comment 3

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

Attachment: New N7 tabs panel

Comment 4

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

Martyn, I noticed we use stretchMode="columnWidth" but then later specify a column width (which doesn't make much sense to me).

Would stretchMode="spacingWidthUniform" be a better choice? I tried it locally and it seems to work well on my N7 and N9, but I'm not sure of any possible side effects.

(In reply to Martyn Haigh (:mhaigh) from comment #2)
> It'd be good to get number on how many Fire HD users we have - may be worth
> looking for other tablets which could be problematic in our top devices
> (mcomella: know how to get these figures?)

(Potentially no longer relevant but) QA might know about device numbers and which devices would be similar to the Fire HD.

Note: We don't official support Kindle devices so I'm more concerned about similar top devices.

Comment 5

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

Attachment: (WIP) Update stretchMode for TabsPanel's GridLayout

The patch, for reference.

Comment 6

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

Spoke to mhaigh on IRC – he's alright with this change, provided :antlam UX approval.

NI for screenshots, or at least to poke :antlam in person.

Comment 7

[Martyn Haigh (:mhaigh)]

Comment on attachment 8554802 [details] [diff] [review]
(WIP) Update stretchMode for TabsPanel's GridLayout

Review of attachment 8554802 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good - provided it doesn't do anything funny on a real device :)

Comment 8

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

Going to wait to speak to mcomella in person about this. Just leaving notes. 

But it sounds like setting/fixing the center column space (between the tabs) and then just centering the entire row would be a good solution.

If that's the case, we currently use 24 dp spacing so we could try setting that or using a direct multiple of it.

Comment 9

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

Comment on attachment 8554802 [details] [diff] [review]
(WIP) Update stretchMode for TabsPanel's GridLayout

This is actually the same as what we currently have because some styles are overwritten in code [1].

After talking to :antlam, it seems dangerous to change the way we lay out the GridView because the docs don't clearly articulate how each layout setting affects the grid and thus don't know how a layout change will affect a wide range of devices - i.e. a uplift of this sort would be dangerous.

As is, I think this bug will only affect two column layouts which means smaller devices (e.g. 7" devices) with low DPI. I don't think these devices are very common so let's find out just what a problem this could be!

NI self: try this on an old N7, which is the same DPI.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tabs/TabsGridLayout.java?rev=0c7496cca523#83

Comment 10

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

Attachment: Old N7 tabs panel

And this looks fine on my old N7 tablet too. Maybe this is a Kindle specific issue?

Cristina (or others), have you seen this behavior on any other device before? If not, I'm thinking we shouldn't worry about this for new-tablet-v1.

Comment 11

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

The old N7 is TVDPI but I'm not sure about the Kindle - maybe that's the difference?

This article seems to indicate the displays are pretty identical: http://gizmodo.com/5944679/kindle-fire-hd-vs-nexus-7-whats-the-best-7-inch-tablet-display

Comment 12

[Cristina Madaras, QA [:CristinaM]]

I didn't see this behavior on any other device, probably is a Kindle specific issue.

Comment 13

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.