1063586 - Audit tab related XML files to use new terminology

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martyn Haigh (:mhaigh)]

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...

Comment 1

[Martyn Haigh (:mhaigh)]

Attachment: Audit tab related XML files to use new terminology

https://tbpl.mozilla.org/?tree=Try&rev=9ff00d36297a

Comment 2

[Martyn Haigh (:mhaigh)]

Attachment: Audit tab related XML files to use new terminology

Uploaded wrong patch file

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 8488560 [details] [diff] [review]
Audit tab related XML files to use new terminology

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

I pointed out a few cases but please audit this patch for the following terminology:
- Tabs panel -> the name of the entire UI for managing tabs (the header + list/grid of tabs + footer). Some times this is confusingly 'tabs tray' which is not necessarily the same as the old TabsTray widget.
- Tab panel -> the content of each 'section' of the tabs panel. Namely, the panels you access via the header e.g. 'Tabs', 'Private tabs', etc. A tab panel might contain more than a TabsLayout in them. For example, the private tabs panel contain the empty view as well as the TabsLayout widget.
- TabsLayout -> the widget used to display the tabs. Right now it's a list but we'll also have the grid layout for new tablet UI.

No need to request another review.

::: mobile/android/base/BrowserApp.java
@@ +2207,4 @@
>      }
>  
>      /**
> +     * Hides certain UI elements (e.g. button toast, tabs layout) when the

I think this comment is more about 'tabs panel'.

@@ +2238,4 @@
>                      return false;
>              }
>  
> +            // If the tab layout is showing, hide it and don't send the event to content.

Same here: tabs panel.

::: mobile/android/base/RemoteTabsExpandableListAdapter.java
@@ +109,4 @@
>          lastModifiedView.setText(TabsAccessor.getLastSyncedString(context, now, client.lastModified));
>  
>          // This view exists only in some of our group views: it's present
> +        // for the home panel groups and not for the tabs layout groups.

tabs panel

@@ +153,4 @@
>          final RemoteTab tab = client.tabs.get(childPosition);
>  
>          // The view is a TwoLinePageRow only for some of our child views: it's
> +        // present for the home panel children and not for the tabs layout

tabs panel

::: mobile/android/base/tabs/TabsPanel.java
@@ +269,4 @@
>          return mActivity.onOptionsItemSelected(item);
>      }
>  
> +    private static int getTabContainerHeight(TabsLayoutContainer listContainer) {

getTabsContainerHeight
listContainer -> tabsContainer

@@ +567,4 @@
>          mHeader.setLayerType(View.LAYER_TYPE_NONE, null);
>          mTabsContainer.setLayerType(View.LAYER_TYPE_NONE, null);
>  
> +        // If the tabs layout is now hidden, call hide() on current panel and unset it as the current panel

tabs layout -> tabs panel

::: mobile/android/base/widget/IconTabWidget.java
@@ +83,4 @@
>          if (!mIsIcon) {
>              return null;
>          }
> +        // We can have multiple views in the tabs layout for each child. This finds the

tabs layout -> tabs panel

Comment 4

[Martyn Haigh (:mhaigh)]

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

Comment 5

[Martyn Haigh (:mhaigh)]

https://hg.mozilla.org/integration/fx-team/rev/7be62fa11c20

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7be62fa11c20