Closed
Bug 1125050
Opened 10 years ago
Closed 4 years ago
Big space between thumbnail rows on Kindle Fire
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, defect)
Tracking
(firefox37 affected, firefox38 affected)
RESOLVED
INCOMPLETE
People
(Reporter: CristinaM, Unassigned, Mentored)
References
Details
(Whiteboard: [lang=java])
Attachments
(3 files, 1 obsolete file)
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•10 years ago
|
||
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?
Flags: needinfo?(mhaigh)
Comment 2•10 years ago
|
||
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?
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(mhaigh)
Flags: needinfo?(alam)
Comment 4•10 years ago
|
||
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.
Flags: needinfo?(michael.l.comella)
Updated•10 years ago
|
Flags: needinfo?(mhaigh)
Comment 5•10 years ago
|
||
The patch, for reference.
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Comment 6•10 years ago
|
||
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.
Flags: needinfo?(mhaigh) → needinfo?(michael.l.comella)
Comment 7•10 years ago
|
||
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 :)
Attachment #8554802 -
Flags: review+
Comment 8•10 years ago
|
||
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.
Flags: needinfo?(alam)
Comment 9•10 years ago
|
||
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
Attachment #8554802 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8554704 -
Attachment description: N7 tabs panel → New N7 tabs panel
Comment 10•10 years ago
|
||
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.
Flags: needinfo?(michael.l.comella) → needinfo?(cristina.madaras)
Comment 11•10 years ago
|
||
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
Reporter | ||
Comment 12•10 years ago
|
||
I didn't see this behavior on any other device, probably is a Kindle specific issue.
Flags: needinfo?(cristina.madaras)
Updated•10 years ago
|
Assignee: michael.l.comella → nobody
Status: ASSIGNED → NEW
Updated•10 years ago
|
Mentor: michael.l.comella, mhaigh
Whiteboard: [lang=java]
Comment 13•4 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•