1060345 - Create a way of dynamically inflating the correct tab layout

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martyn Haigh (:mhaigh)]

Encapsulate the code surrounding which layout to use for displaying tabs

Comment 1

[Lucas Rocha (:lucasr)]

Maybe we want to use the same approach we used for BrowserToolbar? i.e. dynamically resolve class name on layout inflation level. See: http://hg.mozilla.org/projects/larch/rev/30a930761cb1

Comment 2

[Martyn Haigh (:mhaigh)]

Attachment: Create a way of dynamically inflating the correct tab layout

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 8484292 [details] [diff] [review]
Create a way of dynamically inflating the correct tab layout

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

Looks good with nit fixed.

::: mobile/android/base/BrowserApp.java
@@ +236,5 @@
>          final View view;
>          if (BrowserToolbar.class.getName().equals(name)) {
>              view = BrowserToolbar.create(context, attrs);
> +        } else if (TabsPanel.TabsLayout.class.getName().equals(name)) {
> +            view = TabsPanel.create(context, attrs);

nit: Rename to createTabsLayout() for clarity.

Comment 4

[Martyn Haigh (:mhaigh)]

Attachment: Create a way of dynamically inflating the correct tab layout

Had to change the way we were defining the initial file in the XML as Fennec was crashing with runtime errors with the original method.  This way seems fine though.

https://tbpl.mozilla.org/?tree=Try&rev=15feaa66cd69

Comment 5

[Lucas Rocha (:lucasr)]

Comment on attachment 8485004 [details] [diff] [review]
Create a way of dynamically inflating the correct tab layout

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

Nice catch with the nits fixed. Please file a follow-up to audit all style names and ids to follow the new terminology. For instance, I saw that some TabsLayout instances in the layout files are using ids like 'private_tabs_tray' and styles like 'TabsList'. They should all converge to ...tabs_layout... or ...TabsLayout...

::: mobile/android/base/resources/layout/private_tabs_panel.xml
@@ +47,4 @@
>  
>      <!-- Note: for an unknown reason, scrolling in the TabsListLayout
>           does not work unless it is laid out after the empty view. -->
> +    <view class="org.mozilla.gecko.tabs.TabsPanel$TabsLayout" android:id="@+id/private_tabs_tray"

nit: applies to all changed tags in this patch: align the rest of the attributes with the class attribute here. Something like:

<view class="org.mozilla.gecko.tabs.TabsPanel$TabsLayout"
      android:id="+id/private_tabs_tray"
      ...

Comment 6

[Martyn Haigh (:mhaigh)]

https://hg.mozilla.org/projects/larch/rev/a17eec723de5

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a17eec723de5