1066546 - Move new tab and overflow menu icons in the tabs layout to the top whilst in portrait mode

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martyn Haigh (:mhaigh)]

Currently the new tabs icon and the overflow menu icon are based at the bottom of the tabs layout when in portrait mode, a leftover from the side bar currently implemented.  This is implemented via android resource folder switching with the portrait and landscape layouts being drawn from different layout xml files (the layout-large-land-v11 folder having alternate resources for landscape).

We need to move these UI elements back to the top whilst the browser is in portrait mode, whilst also keeping the existing functionality for when isNewTablet != true

Comment 1

[Martyn Haigh (:mhaigh)]

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

Comment 2

[Martyn Haigh (:mhaigh)]

Attachment: Move icons in the tabs layout to the top whilst in portrait mode

The long awaited patch!

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 8498190 [details] [diff] [review]
Move icons in the tabs layout to the top whilst in portrait mode

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

Could you please describe the changes in this patch? I think I need more context.

::: mobile/android/base/tabs/TabsPanel.java
@@ +144,5 @@
>  
> +    private void inflateLayout(Context context) {
> +        if(NewTabletUI.isEnabled(context)) {
> +            LayoutInflater.from(context).inflate(R.layout.tabs_panel_default, this);
> +            ViewStub header = (ViewStub)findViewById(R.id.tabs_panel_header_stub);

Why do you need a viewstub for the header? To allow setting different attributes when in new tablet UI?

Comment 4

[Martyn Haigh (:mhaigh)]

The reason for the complexity here is that the tabs panel header and footer are both included files; without using the viewstub for the header the new tablet ui landscape will pick up the wrong layout file.  We can solve this by inlining the header and footer in the relative files - I don't know why we didn't do this in the first place to be honest as there's a direct one to one correlation between the parent and header views.

Comment 5

[Martyn Haigh (:mhaigh)]

Attachment: Part 1 - Inline header and footer layouts

Here's what this patch would look like if we inline the header and footer layouts in to the main tabs panel layout file

Comment 6

[Martyn Haigh (:mhaigh)]

Attachment: Part 2 - configure tabs panel to use correct layout for new tablet ui

Comment 7

[Martyn Haigh (:mhaigh)]

Attachment: Part 2 - configure tabs panel to use correct layout for new tablet ui

Last one didn't include file name changes

Comment 8

[Lucas Rocha (:lucasr)]

Comment on attachment 8499466 [details] [diff] [review]
Part 1 - Inline header and footer layouts

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

Ah, now I see, makes sense ;-)

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 8499470 [details] [diff] [review]
Part 2 - configure tabs panel to use correct layout for new tablet ui

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

Looks good. Just make sure the change to values-large-land-v11/layout.xml is working fine on phones and old/new tablet UIs.

::: mobile/android/base/resources/values-large-land-v11/layout.xml
@@ -4,4 @@
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <resources>
> -    <item type="layout" name="tabs_layout_item_view">@layout/tabs_item_row</item>

Hmmm, isn't that supposed to point to tabs_item_cell in large-land?

Comment 10

[Martyn Haigh (:mhaigh)]

https://hg.mozilla.org/integration/fx-team/rev/4a1cf6241e5f
https://hg.mozilla.org/integration/fx-team/rev/9f9ab3840334

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4a1cf6241e5f
https://hg.mozilla.org/mozilla-central/rev/9f9ab3840334