Audit tab related XML files to use new terminology

RESOLVED FIXED in Firefox 36

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mhaigh, Assigned: mhaigh)

Tracking

unspecified
Firefox 36
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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...
Uploaded wrong patch file
Attachment #8488558 - Attachment is obsolete: true
Attachment #8488558 - Flags: review?(lucasr.at.mozilla)
Attachment #8488560 - Flags: review?(lucasr.at.mozilla)
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
Attachment #8488560 - Flags: review?(lucasr.at.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/7be62fa11c20
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
You need to log in before you can comment on or make changes to this bug.