1193745 - Implement the tablet tabs tray grid view on mobile

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martyn Haigh (:mhaigh)]

Implement the tablet grid view style tabs tray on mobile, instead of the current list view.  

Involved in this work is swapping the listview for the gridview, changing the tab item layout and cleaning up.

Comment 1

[Martyn Haigh (:mhaigh)]

Attachment: Implement the tablet tabs tray grid view on mobile

Here I remove the mobile tabs tray and replace it with the tablet tabs tray.
I also implement the mobile tabs item view for mobile.

Comment 2

[Martyn Haigh (:mhaigh)]

Attachment: Portrait - SGS5

Here's portrait mode on the SGS5, keep in mind that once bug 1193374 lands the previews will be correct and once bug 1161638 lands we can get rid of the chrome at the bottom of the screen.

Comment 3

[Martyn Haigh (:mhaigh)]

Attachment: Landscape - SGS5

Comment 4

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

Comment on attachment 8647533 [details] [diff] [review]
Implement the tablet tabs tray grid view on mobile

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

In this patch, I think tabs_item_cell is unused - it's used in TabsListLayout [1], which we no longer use. Am I missing something? Also, if possible, it'd be great to merge tablet_tabs_item_cell and tabs_item_cell (or use the resource system so that there are two files with the same name).

Similarly, don't forget to remove all of the old code we're no longer using - TabsListLayout, tabs_item_row.

Otherwise, looks like a sound approach!

[1]: https://mxr.mozilla.org/mozilla-central/search?string=tabs_layout_item_view&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 5

[Martyn Haigh (:mhaigh)]

Here's a build with compact tabs and grid view implemented (not aspect ratios aren't correct, so tab tray thumbnails for anything apart from about:home will look wrong).

https://dl.dropboxusercontent.com/u/7163922/Work/1193745.apk

Here're screenshots from all the devices I can get my hands on currently:
https://www.dropbox.com/sh/6c2twk2o08rscrs/AADeKQQLEOYeweOShTkkYbjQa?dl=0

antlam & bbermes: Feedback and alterations would be appreciated

Comment 6

[Barbara Bermes [:barbara] - NI please!]

- how come some screenshots have an orange border around one tile, what's the purpose of the "orange border", it's the one that was last selected/viewed last, is there a way to deselect it (tapping somewhere outside in the gray area)? I wasn't able to.

Looking at my device, HTC one mobile, some borders have a bigger bottom with no label or anything in it, I guess this won't happen after aspect rations is fixed? What's the dependency for fixing the aspect ratio? It's the https://bugzilla.mozilla.org/show_bug.cgi?id=1193374 bug?

Otherwise, smoking...

Thanks

Comment 7

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

Looks good to me! (I tested with about:home mostly since I know the actual screenshots in the containers themselves still need work :)

great job! *high five!*

Comment 8

[Martyn Haigh (:mhaigh)]

bbermes: The orange border is the currently selected tab - you can't deselect it as there's always got to be a selected tab.

Bug 1193374 landed yesterday, this fixed the aspect ratio issue you were seeing.

Thanks for the feedback :)

Comment 9

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc718298500e

Comment 10

[Martyn Haigh (:mhaigh)]

Attachment: MozReview Request: Bug 1193745 - Implement the tablet tabs tray grid view on mobile r?mcomella

Bug 1193745 - Implement the tablet tabs tray grid view on mobile r?mcomella

Comment 11

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

Comment on attachment 8652955 [details]
MozReview Request: Bug 1193745 - Implement the tablet tabs tray grid view on mobile r?mcomella

https://reviewboard.mozilla.org/r/17323/#review15483

Ship It!

::: mobile/android/base/resources/layout/private_tabs_panel.xml:17
(Diff revision 1)
> -    <view class="org.mozilla.gecko.tabs.TabsPanel$TabsLayout"
> +    <org.mozilla.gecko.tabs.TabsGridLayout

`TabsPanel$TabsLayout` is unused, I think – let's axe it!

::: mobile/android/base/resources/values/dimens.xml:148
(Diff revision 1)
> -    <dimen name="tablet_tab_thumbnail_width">168dp</dimen>
> +    <dimen name="tablet_tab_thumbnail_width">121dp</dimen>

nit: You can probably drop the `tablet_` on these.

::: mobile/android/base/tabs/TabsPanel.java
(Diff revision 1)
> -    public Panel getCurrentPanel() {

nit: I'd prefer this in a separate commit but to save time, it's fine to leave this in for now.

Comment 12

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f48cdd41906

Comment 13

[Martyn Haigh (:mhaigh)]

url:        https://hg.mozilla.org/integration/fx-team/rev/159775bbf798b881edd420887faab3f39abe0db1
changeset:  159775bbf798b881edd420887faab3f39abe0db1
user:       Martyn Haigh <[e-mail]>
date:       Thu Aug 27 13:09:28 2015 +0100
description:
Bug 1193745 - Implement the tablet tabs tray grid view on mobile r=mcomella

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Backed out for robocop crashes.
https://treeherder.mozilla.org/logviewer.html#?job_id=4407893&repo=fx-team

https://hg.mozilla.org/integration/fx-team/rev/49a82a658d1c

Comment 15

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d15808ff4d9

Comment 16

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9785edddce27

Comment 17

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad1f2e56dc2c

Comment 18

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=87787d515ff9

Comment 19

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fff90ed19d2b

Comment 20

[Martyn Haigh (:mhaigh)]

url:        https://hg.mozilla.org/integration/fx-team/rev/6ddd2771e1643911cedf92e8714da1fa26c1e74a
changeset:  6ddd2771e1643911cedf92e8714da1fa26c1e74a
user:       Martyn Haigh <[e-mail]>
date:       Tue Sep 01 13:57:44 2015 +0100
description:
Bug 1193745 - Implement the tablet tabs tray grid view on mobile r=mcomella

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6ddd2771e164