[TabStrip] Add dividers between inactive tabs

Add dividers between inactive tabs. -Implemented as additional layer at starting side of each tab. -Not yet animated. -Needs followup to address divider before new tab buttons. -Uses temp value for offset as spacing will change with updated containers. Video: https://drive.google.com/file/d/14ljKunJmOC7IhiO7RiPyxMaQsuk6lRBx/view?usp=share_link Bug: 1373634 Change-Id: I6e0a87f6139d13ceb31c1afc12472ae80fe863ec Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4018078 Reviewed-by: Theresa Sullivan <twellington@chromium.org> Reviewed-by: Sirisha Kavuluru <skavuluru@google.com> Commit-Queue: Neil Coronado <nemco@google.com> Cr-Commit-Position: refs/heads/main@{#1070621}
chromium · Nov 12, 2022 · efad24e · efad24e
1 parent c5e8ae6
commit efad24e
Show file tree

Hide file tree

Showing 10 changed files with 214 additions and 20 deletions.
diff --git a/chrome/android/chrome_java_resources.gni b/chrome/android/chrome_java_resources.gni
@@ -358,6 +358,7 @@ chrome_java_resources = [
   "java/res/drawable/adaptive_toolbar_preference_header.xml",
   "java/res/drawable/arrow_down.xml",
   "java/res/drawable/arrow_up.xml",
+  "java/res/drawable/bg_tabstrip_tab_divider.xml",
   "java/res/drawable/bg_white_dialog.xml",
   "java/res/drawable/bookmark_save_flow_ripple.xml",
   "java/res/drawable/bookmark_title_bar_shadow.xml",

diff --git a/chrome/android/java/res/drawable/bg_tabstrip_tab_divider.xml b/chrome/android/java/res/drawable/bg_tabstrip_tab_divider.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+Copyright 2022 The Chromium Authors
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+
+<shape
+    xmlns:android="http://schemas.android.com/apk/res/android"
+    android:shape="rectangle" >
+    <corners
+        android:radius="1dp"/>
+    <size
+        android:width="2dp"
+        android:height="20dp" />
+    <!-- This resource is passed to the CC layer, which has not yet been updated
+         to work with themes (See https://crbug.com/1381196). Instead, we
+         dynamically re-color this asset in code. Leaving this solid color here
+         to both indicate what color this should be and to allow for solid
+         re-coloring. -->
+    <solid
+        android:color="@color/default_icon_color_accent1_baseline" />
+</shape>
diff --git a/...oid/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java b/...oid/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java
@@ -133,6 +133,9 @@ public Float get(StripLayoutHelper object) {
     private static final float NEW_TAB_BUTTON_TOUCH_TARGET_OFFSET = 12.f;
     static final float BACKGROUND_TAB_BRIGHTNESS_DEFAULT = 1.f;
     static final float BACKGROUND_TAB_BRIGHTNESS_DIMMED = 0.65f;
+    static final float DIVIDER_HIDDEN_OPACITY = 0.f;
+    static final float DIVIDER_DEFAULT_OPACITY = 0.2f;
+    static final float DIVIDER_BOLD_OPACITY = 0.6f;
     static final float FADE_FULL_OPACITY_THRESHOLD_DP = 24.f;
 
     private static final int MESSAGE_RESIZE = 1;
@@ -757,6 +760,35 @@ private void updateCloseButtons() {
         }
     }
 
+    /**
+     * Called to hide dividers when adjacent to the selected tab. Also bolds the first divider for
+     * a tab group when in edit mode.
+     */
+    private void updateDividers() {
+        if (!ChromeFeatureList.sTabStripRedesign.isEnabled() || mMultiStepTabCloseAnimRunning) {
+            return;
+        }
+
+        // Divider is never shown for the first tab.
+        mStripTabs[0].setDividerOpacity(DIVIDER_HIDDEN_OPACITY);
+
+        int selectedTabId = mStripTabs[mModel.index()].getId();
+        for (int i = 1; i < mStripTabs.length; i++) {
+            final StripLayoutTab prevTab = mStripTabs[i - 1];
+            final StripLayoutTab currTab = mStripTabs[i];
+            if (prevTab.getTrailingMargin() > 0) {
+                // The first divider after a tab group margin is bolded to help indicate separation.
+                currTab.setDividerOpacity(DIVIDER_BOLD_OPACITY);
+            } else if (prevTab.getId() == selectedTabId || currTab.getId() == selectedTabId) {
+                // Dividers adjacent to the selected tab are hidden.
+                currTab.setDividerOpacity(DIVIDER_HIDDEN_OPACITY);
+            } else {
+                // Otherwise return the divider to the default opacity.
+                currTab.setDividerOpacity(DIVIDER_DEFAULT_OPACITY);
+            }
+        }
+    }
+
     /**
      * Checks whether a tab at the edge of the strip is partially hidden, in which case the
      * close button will be hidden to avoid accidental clicks.
@@ -1481,8 +1513,11 @@ private void updateStrip() {
         // 7. Invalidate the accessibility provider in case the visible virtual views have changed.
         mRenderHost.invalidateAccessibilityProvider();
 
-        // 8. Hide close buttons if tab width gets lower than 156dp
+        // 8. Hide close buttons if tab width gets lower than 156dp.
         updateCloseButtons();
+
+        // 9. Show dividers between inactive tabs.
+        updateDividers();
     }
 
     private void computeTabInitialPositions() {

diff --git a/...ndroid/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutTab.java b/...ndroid/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutTab.java
@@ -11,6 +11,7 @@
 import android.graphics.RectF;
 import android.util.FloatProperty;
 
+import androidx.annotation.ColorInt;
 import androidx.annotation.VisibleForTesting;
 import androidx.core.content.res.ResourcesCompat;
 
@@ -31,6 +32,7 @@
 import org.chromium.chrome.browser.layouts.components.VirtualView;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.components.browser_ui.styles.ChromeColors;
+import org.chromium.components.browser_ui.styles.SemanticColorUtils;
 import org.chromium.ui.base.LocalizationUtils;
 import org.chromium.ui.resources.AndroidResourceType;
 import org.chromium.ui.resources.LayoutResource;
@@ -164,6 +166,10 @@ public Float get(StripLayoutTab object) {
     private static final int CLOSE_BUTTON_WIDTH_DP = 36;
     private static final int CLOSE_BUTTON_WIDTH_SCROLLING_STRIP_DP = 48;
 
+    // Divider Constants
+    // TODO(crbug.com/1373632): Temp value until the 9-patches are updated.
+    private static final int DIVIDER_OFFSET_X = 9;
+
     private int mId = Tab.INVALID_TAB_ID;
 
     private final Context mContext;
@@ -178,6 +184,7 @@ public Float get(StripLayoutTab object) {
     private boolean mCanShowCloseButton = true;
     private final boolean mIncognito;
     private float mContentOffsetX;
+    private float mDividerOpacity;
     private float mVisiblePercentage = 1.f;
     private String mAccessibilityDescription;
 
@@ -322,6 +329,13 @@ public int getOutlineResourceId() {
         return R.drawable.bg_tabstrip_background_tab_outline;
     }
 
+    /**
+     * @return The Android resource that represents the tab divider.
+     */
+    public int getDividerResourceId() {
+        return R.drawable.bg_tabstrip_tab_divider;
+    }
+
     /**
      * @param foreground Whether or not this tab is a foreground tab.
      * @return The tint color resource that represents the tab background.
@@ -364,6 +378,27 @@ public int getOutlineTint(boolean foreground) {
         return ColorUtils.getColorWithOverlay(baseColor, overlayColor, overlayAlpha);
     }
 
+    /**
+     * @return The tint color resource for the tab divider.
+     */
+    public @ColorInt int getDividerTint() {
+        return SemanticColorUtils.getDefaultIconColorAccent1(mContext);
+    }
+
+    /**
+     * @param dividerOpacity The new opacity for the tab divider.
+     */
+    public void setDividerOpacity(float dividerOpacity) {
+        mDividerOpacity = dividerOpacity;
+    }
+
+    /**
+     * @return The opacity of the tab divider.
+     */
+    public float getDividerOpacity() {
+        return mDividerOpacity;
+    }
+
     /**
      * @param visible Whether or not this {@link StripLayoutTab} should be drawn.
      */
@@ -493,6 +528,13 @@ public float getContentOffsetX() {
         return mContentOffsetX;
     }
 
+    /**
+     * @return The trailing offset for the tab divider.
+     */
+    public float getDividerOffsetX() {
+        return DIVIDER_OFFSET_X;
+    }
+
     /**
      * @param visiblePercentage How much of the tab is visible (not overlapped by other tabs).
      */

diff --git a/...droid/java/src/org/chromium/chrome/browser/compositor/scene_layer/TabStripSceneLayer.java b/...droid/java/src/org/chromium/chrome/browser/compositor/scene_layer/TabStripSceneLayer.java
@@ -187,15 +187,16 @@ private void pushStripTabs(StripLayoutHelperManager layoutHelper,
             final StripLayoutTab st = stripTabs[i];
             boolean isSelected = st.getId() == selectedTabId;
             TabStripSceneLayerJni.get().putStripTabLayer(mNativePtr, TabStripSceneLayer.this,
-                    st.getId(), st.getCloseButton().getResourceId(), st.getResourceId(),
-                    st.getOutlineResourceId(), st.getCloseButton().getTint(),
-                    st.getTint(isSelected), st.getOutlineTint(isSelected), isSelected,
-                    st.getClosePressed(), layoutHelper.getWidth() * mDpToPx,
+                    st.getId(), st.getCloseButton().getResourceId(), st.getDividerResourceId(),
+                    st.getResourceId(), st.getOutlineResourceId(), st.getCloseButton().getTint(),
+                    st.getDividerTint(), st.getTint(isSelected), st.getOutlineTint(isSelected),
+                    isSelected, st.getClosePressed(), layoutHelper.getWidth() * mDpToPx,
                     st.getDrawX() * mDpToPx, st.getDrawY() * mDpToPx, st.getWidth() * mDpToPx,
                     st.getHeight() * mDpToPx, st.getContentOffsetX() * mDpToPx,
-                    st.getCloseButton().getOpacity(), st.isLoading(),
-                    st.getLoadingSpinnerRotation(), st.getBrightness(), st.getOpacity(isSelected),
-                    layerTitleCache, resourceManager);
+                    st.getDividerOffsetX() * mDpToPx, st.getCloseButton().getOpacity(),
+                    st.getDividerOpacity(), st.isLoading(), st.getLoadingSpinnerRotation(),
+                    st.getBrightness(), st.getOpacity(isSelected), layerTitleCache,
+                    resourceManager);
         }
     }
 
@@ -230,10 +231,11 @@ void updateTabStripRightFade(long nativeTabStripSceneLayer, TabStripSceneLayer c
                 int resourceId, float opacity, ResourceManager resourceManager,
                 @ColorInt int rightFadeColor);
         void putStripTabLayer(long nativeTabStripSceneLayer, TabStripSceneLayer caller, int id,
-                int closeResourceId, int handleResourceId, int handleOutlineResourceId,
-                int closeTint, int handleTint, int handleOutlineTint, boolean foreground,
-                boolean closePressed, float toolbarWidth, float x, float y, float width,
-                float height, float contentOffsetX, float closeButtonAlpha, boolean isLoading,
+                int closeResourceId, int dividerResourceId, int handleResourceId,
+                int handleOutlineResourceId, int closeTint, int dividerTint, int handleTint,
+                int handleOutlineTint, boolean foreground, boolean closePressed, float toolbarWidth,
+                float x, float y, float width, float height, float contentOffsetX,
+                float dividerOffsetX, float closeButtonAlpha, float dividerAlpha, boolean isLoading,
                 float spinnerRotation, float brightness, float opacity,
                 LayerTitleCache layerTitleCache, ResourceManager resourceManager);
         void setContentTree(

diff --git a/...unit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java b/...unit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java
@@ -64,10 +64,13 @@
 
 /** Tests for {@link StripLayoutHelper}. */
 @RunWith(BaseRobolectricTestRunner.class)
+// clang-format off
 @Features.EnableFeatures({ChromeFeatureList.TAB_STRIP_IMPROVEMENTS,
-        ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS, ChromeFeatureList.TAB_GROUPS_FOR_TABLETS})
+        ChromeFeatureList.TAB_STRIP_REDESIGN, ChromeFeatureList.GRID_TAB_SWITCHER_FOR_TABLETS,
+        ChromeFeatureList.TAB_GROUPS_FOR_TABLETS})
 @Config(manifest = Config.NONE, qualifiers = "sw600dp")
 public class StripLayoutHelperTest {
+    // clang-format on
     @Rule
     public TestRule mFeaturesProcessorRule = new Features.JUnitProcessor();
     @Mock
@@ -471,6 +474,62 @@ public void testTabSelected_EdgeTab_End_Rtl_HideCloseBtn() {
         Mockito.verify(tabs[4]).setCanShowCloseButton(false, false);
     }
 
+    @Test
+    @Feature("Tab Strip Redesign")
+    public void testUpdateDividers_WithTabSelected() {
+        // Setup with 5 tabs. Select tab 2.
+        initializeTest(false, false, 2);
+        StripLayoutTab[] tabs = mStripLayoutHelper.getStripLayoutTabs();
+
+        // Trigger update to set divider values.
+        mStripLayoutHelper.updateLayout(TIMESTAMP);
+
+        // Verify tabs 2 and 3's dividers are hidden due to selection.
+        float hiddenOpacity = StripLayoutHelper.DIVIDER_HIDDEN_OPACITY;
+        float visibleOpacity = StripLayoutHelper.DIVIDER_DEFAULT_OPACITY;
+        // clang-format off
+        assertEquals("First divider should always be hidden.",
+                hiddenOpacity, tabs[0].getDividerOpacity(), EPSILON);
+        assertEquals("Divider should be at default opacity.",
+                visibleOpacity, tabs[1].getDividerOpacity(), EPSILON);
+        assertEquals("Divider is adjacent to selected tab and should be hidden.",
+                hiddenOpacity, tabs[2].getDividerOpacity(), EPSILON);
+        assertEquals("Divider is adjacent to selected tab and should be hidden.",
+                hiddenOpacity, tabs[3].getDividerOpacity(), EPSILON);
+        assertEquals("Divider should be at default opacity.",
+                visibleOpacity, tabs[4].getDividerOpacity(), EPSILON);
+        // clang-format on
+    }
+
+    @Test
+    @Feature("Tab Strip Redesign")
+    public void testUpdateDividers_WithTabGroups() {
+        // Setup with 5 tabs. Select tab 4.
+        initializeTest(false, false, 4);
+        StripLayoutTab[] tabs = mStripLayoutHelper.getStripLayoutTabs();
+
+        // Trigger update to set divider values. Mock a tab group margin after tab 1.
+        tabs[1].setTrailingMargin(100.f);
+        mStripLayoutHelper.updateLayout(TIMESTAMP);
+
+        // Verify tab 2's divider is bolded.
+        float hiddenOpacity = StripLayoutHelper.DIVIDER_HIDDEN_OPACITY;
+        float visibleOpacity = StripLayoutHelper.DIVIDER_DEFAULT_OPACITY;
+        float boldedOpacity = StripLayoutHelper.DIVIDER_BOLD_OPACITY;
+        // clang-format off
+        assertEquals("First divider should always be hidden.",
+                hiddenOpacity, tabs[0].getDividerOpacity(), EPSILON);
+        assertEquals("Divider should be at default opacity.",
+                visibleOpacity, tabs[1].getDividerOpacity(), EPSILON);
+        assertEquals("Divider is after tab group margin and should be bolded.",
+                boldedOpacity, tabs[2].getDividerOpacity(), EPSILON);
+        assertEquals("Divider should be at default opacity.",
+                visibleOpacity, tabs[3].getDividerOpacity(), EPSILON);
+        assertEquals("Divider is adjacent to selected tab and should be hidden.",
+                hiddenOpacity, tabs[4].getDividerOpacity(), EPSILON);
+        // clang-format on
+    }
+
     @Test
     @Feature("Tab Strip Improvements")
     public void testScrollOffset_OnResume_StartOnLeft_SelectedRightmostTab() {
@@ -1461,7 +1520,7 @@ public void testTabClosing_NonLastTab_TabResize_NewTabButtonMoved() {
         // Arrange
         int tabCount = 4;
         TabUiFeatureUtilities.setTabMinWidthForTesting(TAB_WIDTH_MEDIUM);
-        initializeTest(false, false, false, 4, tabCount);
+        initializeTest(false, false, false, 3, tabCount);
         StripLayoutTab[] tabs = getRealStripLayoutTabs(TAB_WIDTH_MEDIUM, tabCount);
         mStripLayoutHelper.setStripLayoutTabsForTest(tabs);
         mStripLayoutHelper.onSizeChanged(SCREEN_WIDTH, SCREEN_HEIGHT, false, TIMESTAMP);

diff --git a/chrome/browser/android/compositor/layer/tab_handle_layer.cc b/chrome/browser/android/compositor/layer/tab_handle_layer.cc
@@ -28,6 +28,7 @@ scoped_refptr<TabHandleLayer> TabHandleLayer::Create(
 void TabHandleLayer::SetProperties(
     int id,
     ui::Resource* close_button_resource,
+    ui::Resource* divider_resource,
     ui::NinePatchResource* tab_handle_resource,
     ui::NinePatchResource* tab_handle_outline_resource,
     bool foreground,
@@ -38,7 +39,9 @@ void TabHandleLayer::SetProperties(
     float width,
     float height,
     float content_offset_x,
+    float divider_offset_x,
     float close_button_alpha,
+    float divider_alpha,
     bool is_loading,
     float spinner_rotation,
     float brightness,
@@ -91,7 +94,7 @@ void TabHandleLayer::SetProperties(
 
   if (title_layer) {
     title_layer->setOpacity(1.0f);
-    unsigned expected_children = 4;
+    unsigned expected_children = 5;
     title_layer_ = title_layer->layer();
     if (layer_->children().size() < expected_children) {
       layer_->AddChild(title_layer_);
@@ -145,6 +148,19 @@ void TabHandleLayer::SetProperties(
     }
   }
 
+  if (divider_alpha == 0.f) {
+    divider_->SetIsDrawable(false);
+  } else {
+    divider_->SetIsDrawable(true);
+    divider_->SetUIResourceId(divider_resource->ui_resource()->id());
+    divider_->SetBounds(divider_resource->size());
+    int divider_y = (tab_handle_resource->padding().y() + height) / 2 -
+                    divider_->bounds().height() / 2;
+    int divider_x = is_rtl ? width - divider_offset_x : divider_offset_x;
+    divider_->SetPosition(gfx::PointF(divider_x, divider_y));
+    divider_->SetOpacity(divider_alpha);
+  }
+
   if (title_layer) {
     int title_y = tab_handle_resource->padding().y() / 2 + height / 2 -
                   title_layer->size().height() / 2;
@@ -193,6 +209,7 @@ TabHandleLayer::TabHandleLayer(LayerTitleCache* layer_title_cache)
     : layer_title_cache_(layer_title_cache),
       layer_(cc::Layer::Create()),
       close_button_(cc::UIResourceLayer::Create()),
+      divider_(cc::UIResourceLayer::Create()),
       decoration_tab_(cc::NinePatchLayer::Create()),
       tab_outline_(cc::NinePatchLayer::Create()),
       brightness_(1.0f),
@@ -205,6 +222,7 @@ TabHandleLayer::TabHandleLayer(LayerTitleCache* layer_title_cache)
   layer_->AddChild(decoration_tab_);
   layer_->AddChild(tab_outline_);
   layer_->AddChild(close_button_);
+  layer_->AddChild(divider_);
 }
 
 TabHandleLayer::~TabHandleLayer() {

diff --git a/chrome/browser/android/compositor/layer/tab_handle_layer.h b/chrome/browser/android/compositor/layer/tab_handle_layer.h
@@ -35,6 +35,7 @@ class TabHandleLayer : public Layer {
 
   void SetProperties(int id,
                      ui::Resource* close_button_resource,
+                     ui::Resource* divider_resource,
                      ui::NinePatchResource* tab_handle_resource,
                      ui::NinePatchResource* tab_handle_outline_resource,
                      bool foreground,
@@ -45,7 +46,9 @@ class TabHandleLayer : public Layer {
                      float width,
                      float height,
                      float content_offset_x,
+                     float divider_offset_x,
                      float close_button_alpha,
+                     float divider_alpha,
                      bool is_loading,
                      float spinner_rotation,
                      float brightness,
@@ -61,6 +64,7 @@ class TabHandleLayer : public Layer {
 
   scoped_refptr<cc::Layer> layer_;
   scoped_refptr<cc::UIResourceLayer> close_button_;
+  scoped_refptr<cc::UIResourceLayer> divider_;
   scoped_refptr<cc::NinePatchLayer> decoration_tab_;
   scoped_refptr<cc::NinePatchLayer> tab_outline_;
   scoped_refptr<cc::Layer> title_layer_;