Bug 1350737 - Make the selected tab's row the first visible row when the tabs tray opens. r?sebastian draft
authorTom Klein <twointofive@gmail.com>
Sun, 26 Mar 2017 21:44:23 -0500
changeset 563233 c8ebf051e30019c30b7f2e5498df55060814c334
parent 561664 ff7729561efd16e7c3140d40337a84318730ab9c
child 624422 3688f437e98905b89a17d6c198088eca98c92903
push id54242
push userbmo:twointofive@gmail.com
push dateSat, 15 Apr 2017 16:54:45 +0000
reviewerssebastian
bugs1350737, 1299905
milestone55.0a1
Bug 1350737 - Make the selected tab's row the first visible row when the tabs tray opens. r?sebastian That helps usability in the following scenario: 1) Open more tabs than will fit on screen in the tabs tray (and fill the last row with tabs if the tabs tray is in a grid mode); 2) Scroll to the bottom; 3) Open one of the last visible tabs, and from that open tab open a new background tab (e.g. long click on a link in a page and choose "Open in new tab"); 4) Reopen the tabs tray. With the fix, the new tab will be visible at the bottom of the list, whereas previously in list mode it was not visible at all, and in grid modes only the top of the title was visible. (Bug 1299905 would make this sort of situation more widely applicable.) This patch also has the side effect of scrolling the selected tab to the top of the tabs tray on each rotation (previously it was just scrolled into view). MozReview-Commit-ID: 4MKY7P1Mihk
mobile/android/base/java/org/mozilla/gecko/tabs/AutoFitTabsGridLayout.java
mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/AutoFitTabsGridLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/AutoFitTabsGridLayout.java
@@ -38,17 +38,17 @@ class AutoFitTabsGridLayout extends Tabs
     }
 
     private void updateSpacingDecoration(int horizontalItemSpacing) {
         if (spacingDecoration != null) {
             removeItemDecoration(spacingDecoration);
         }
         spacingDecoration = new GridSpacingDecoration(horizontalItemSpacing, verticalItemPadding);
         addItemDecoration(spacingDecoration);
-        updateSelectedPosition();
+        scrollSelectedTabToTopOfTray();
     }
 
     @Override
     public void onSizeChanged(int w, int h, int oldw, int oldh) {
         super.onSizeChanged(w, h, oldw, oldh);
 
         if (w == oldw) {
             return;
--- a/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayout.java
@@ -7,16 +7,17 @@ package org.mozilla.gecko.tabs;
 
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Tab;
 import org.mozilla.gecko.Tabs;
 import org.mozilla.gecko.widget.RecyclerViewClickSupport;
 
 import android.content.Context;
 import android.content.res.TypedArray;
+import android.support.v7.widget.LinearLayoutManager;
 import android.support.v7.widget.RecyclerView;
 import android.util.AttributeSet;
 import android.view.View;
 import android.widget.Button;
 
 import java.util.ArrayList;
 
 import static org.mozilla.gecko.Tab.TabType;
@@ -105,17 +106,17 @@ public abstract class TabsLayout extends
 
         switch (msg) {
             case ADDED:
                 int tabIndex = Integer.parseInt(data);
                 tabIndex = tabIndex == Tabs.NEW_LAST_INDEX ? tabsAdapter.getItemCount() : tabIndex;
                 tabsAdapter.notifyTabInserted(tab, tabIndex);
                 if (addAtIndexRequiresScroll(tabIndex)) {
                     // (The SELECTED tab is updated *after* this call to ADDED, so don't just call
-                    // updateSelectedPosition().)
+                    // scrollSelectedTabToTopOfTray().)
                     scrollToPosition(tabIndex);
                 }
                 break;
 
             case CLOSED:
                 if (tab.isPrivate() == isPrivate && tabsAdapter.getItemCount() > 0) {
                     tabsAdapter.removeTab(tab);
                 }
@@ -163,38 +164,43 @@ public abstract class TabsLayout extends
         closeTab(view);
     }
 
     @Override
     public boolean onItemMove(int fromPosition, int toPosition) {
         return tabsAdapter.moveTab(fromPosition, toPosition);
     }
 
-    /** Updates the selected position in the list so that it will be scrolled to the right place. */
-    protected void updateSelectedPosition() {
+    /**
+     * Scroll the selected tab to the top of the tray.
+     * One of the motivations for scrolling to the top is so that, as often as possible, if we open
+     * a background tab from the selected tab, when we return to the tabs tray the new tab will be
+     * visible for selecting without requiring additional scrolling.
+     */
+    protected void scrollSelectedTabToTopOfTray() {
         final int selected = getSelectedAdapterPosition();
         if (selected != NO_POSITION) {
-            scrollToPosition(selected);
+            ((LinearLayoutManager)getLayoutManager()).scrollToPositionWithOffset(selected, 0);
         }
     }
 
     private void refreshTabsData() {
         // Store a different copy of the tabs, so that we don't have to worry about
         // accidentally updating it on the wrong thread.
         final ArrayList<Tab> tabData = new ArrayList<>();
         final Iterable<Tab> allTabs = Tabs.getInstance().getTabsInOrder();
 
         for (final Tab tab : allTabs) {
             if (tab.isPrivate() == isPrivate && tab.getType() == type) {
                 tabData.add(tab);
             }
         }
 
         tabsAdapter.setTabs(tabData);
-        updateSelectedPosition();
+        scrollSelectedTabToTopOfTray();
     }
 
     private void closeTab(View view) {
         final TabsLayoutItemView itemView = (TabsLayoutItemView) view;
         final Tab tab = getTabForView(itemView);
         if (tab == null) {
             // We can be null here if this is the second closeTab call resulting from a sufficiently
             // fast double tap on the close tab button.