Skip to content

Commit 337a771

Browse files
Sirisha KavuluruChromium LUCI CQ
Sirisha Kavuluru
authored and
Chromium LUCI CQ
committed
[Merge M111][TabStripRedesign] Add missing start / end dividers for tab strip
Demo: https://drive.google.com/file/d/1euYlZYdZifZr40npIckDmf53cV1rl5Gr/view?usp=share_link Spec: https://docs.google.com/presentation/d/1VxlgkrY0ef5bVJiE1EC3qK8qkLCh5AmZHWRhaV0xB4Y/edit?resourcekey=0-7c1vUSKo4bZhr4gn178ALw#slide=id.g180ed310390_0_4 (cherry picked from commit d4c5b7d) Bug: 1412231 Change-Id: I8f32508af79615cf6828ce1675854d62761fe1b1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4214878 Reviewed-by: Neil Coronado <nemco@google.com> Commit-Queue: Sirisha Kavuluru <skavuluru@google.com> Cr-Original-Commit-Position: refs/heads/main@{#1100203} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4220176 Cr-Commit-Position: refs/branch-heads/5563@{#129} Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}
1 parent 2ea9fdd commit 337a771

File tree

8 files changed

+152
-63
lines changed

8 files changed

+152
-63
lines changed

chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java

+31-14
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,6 @@ public Float get(StripLayoutHelper object) {
155155
static final float TAB_OPACITY_VISIBLE_FOREGROUND = 1.f;
156156
static final float BACKGROUND_TAB_BRIGHTNESS_DEFAULT = 1.f;
157157
static final float BACKGROUND_TAB_BRIGHTNESS_DIMMED = 0.65f;
158-
static final float DIVIDER_HIDDEN_OPACITY = 0.f;
159-
static final float DIVIDER_DEFAULT_OPACITY = 1.f;
160158
static final float FADE_FULL_OPACITY_THRESHOLD_DP = 24.f;
161159

162160
private static final int MESSAGE_RESIZE = 1;
@@ -884,8 +882,14 @@ private void updateForegroundTabContainersAndDividers() {
884882
int selectedTabId = mStripTabs[index].getId();
885883

886884
// Divider is never shown for the first tab.
887-
mStripTabs[0].setDividerOpacity(DIVIDER_HIDDEN_OPACITY);
885+
mStripTabs[0].setStartDividerVisible(false);
888886
setForegroundTabContainerVisible(mStripTabs[0], selectedTabId);
887+
// End divider for first tab is only shown in reorder mode when tab has trailing margin and
888+
// container is not visible.
889+
boolean endDividerVisible = mInReorderMode
890+
&& mStripTabs[0].getContainerOpacity() == TAB_OPACITY_HIDDEN
891+
&& mStripTabs[0].getTrailingMargin() > 0;
892+
mStripTabs[0].setEndDividerVisible(endDividerVisible);
889893

890894
for (int i = 1; i < mStripTabs.length; i++) {
891895
final StripLayoutTab prevTab = mStripTabs[i - 1];
@@ -894,17 +898,30 @@ private void updateForegroundTabContainersAndDividers() {
894898
// Set container opacity.
895899
setForegroundTabContainerVisible(currTab, selectedTabId);
896900

897-
// Set divider opacity.
898-
if (prevTab.getId() == selectedTabId || currTab.getId() == selectedTabId
899-
|| currTab.getContainerOpacity() > TAB_OPACITY_HIDDEN) {
900-
// Dividers adjacent to selected tab are hidden. Additionally, when tab containers
901-
// are visible for grouped tabs in edit mode, tab dividers are unneeded and
902-
// therefore hidden.
903-
currTab.setDividerOpacity(DIVIDER_HIDDEN_OPACITY);
904-
} else {
905-
// All other dividers are visible.
906-
currTab.setDividerOpacity(DIVIDER_DEFAULT_OPACITY);
907-
}
901+
/**
902+
* Start divider should be visible when:
903+
* 1. In reorder mode and currTab container is hidden
904+
* 2. Not in reorder mode and prevTab is not selected and currTab is not selected
905+
*/
906+
boolean startDividerVisible =
907+
(mInReorderMode && currTab.getContainerOpacity() == TAB_OPACITY_HIDDEN)
908+
|| (!mInReorderMode && prevTab.getId() != selectedTabId
909+
&& currTab.getId() != selectedTabId);
910+
currTab.setStartDividerVisible(startDividerVisible);
911+
912+
/**
913+
* End divider should be applied when:
914+
* 1. In reorder mode and currTab container is hidden and
915+
* (a) currTab's trailing margin > 0 (ie last tab in group) OR
916+
* (b) currTab is last tab in strip (last tab does not have trailing margin)
917+
* 2. Not in reorder mode and currTab is last tab on strip and is not selected
918+
*/
919+
endDividerVisible =
920+
(mInReorderMode && currTab.getContainerOpacity() == TAB_OPACITY_HIDDEN
921+
&& (currTab.getTrailingMargin() > 0 || i == (mStripTabs.length - 1)))
922+
|| (!mInReorderMode && i == (mStripTabs.length - 1)
923+
&& currTab.getId() != selectedTabId);
924+
currTab.setEndDividerVisible(endDividerVisible);
908925
}
909926
}
910927

chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutTab.java

+22-8
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,6 @@ public Float get(StripLayoutTab object) {
189189
private static final float DETACHED_CONTENT_OFFSET_Y = 10.f;
190190

191191
// Divider Constants
192-
// TODO(crbug.com/1373632): Temp value until the 9-patches are updated.
193192
private static final int DIVIDER_OFFSET_X = 13;
194193
@VisibleForTesting
195194
static final float DIVIDER_FOLIO_LIGHT_OPACITY = 0.2f;
@@ -207,11 +206,12 @@ public Float get(StripLayoutTab object) {
207206
private boolean mIsDying;
208207
private boolean mCanShowCloseButton = true;
209208
private boolean mFolioAttached = true;
209+
private boolean mStartDividerVisible;
210+
private boolean mEndDividerVisible;
210211
private final boolean mIncognito;
211212
private float mBottomMargin;
212213
private float mContainerOpacity;
213214
private float mContentOffsetX;
214-
private float mDividerOpacity;
215215
private float mVisiblePercentage = 1.f;
216216
private String mAccessibilityDescription;
217217

@@ -459,17 +459,31 @@ public int getOutlineTint(boolean foreground) {
459459
}
460460

461461
/**
462-
* @param dividerOpacity The new opacity for the tab divider.
462+
* @param visible Visibility of tab's start divider.
463463
*/
464-
public void setDividerOpacity(float dividerOpacity) {
465-
mDividerOpacity = dividerOpacity;
464+
public void setStartDividerVisible(boolean visible) {
465+
mStartDividerVisible = visible;
466466
}
467467

468468
/**
469-
* @return The opacity of the tab divider.
469+
* @return Visibility of tab's start divider.
470470
*/
471-
public float getDividerOpacity() {
472-
return mDividerOpacity;
471+
public boolean isStartDividerVisible() {
472+
return mStartDividerVisible;
473+
}
474+
475+
/**
476+
* @param visible Visibility of end divider.
477+
*/
478+
public void setEndDividerVisible(boolean visible) {
479+
mEndDividerVisible = visible;
480+
}
481+
482+
/**
483+
* @return Visibility of tab's end divider.
484+
*/
485+
public boolean isEndDividerVisible() {
486+
return mEndDividerVisible;
473487
}
474488

475489
/**

chrome/android/java/src/org/chromium/chrome/browser/compositor/scene_layer/TabStripSceneLayer.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,9 @@ private void pushStripTabs(StripLayoutHelperManager layoutHelper,
209209
st.getHeight() * mDpToPx, st.getContentOffsetX() * mDpToPx,
210210
st.getContentOffsetY() * mDpToPx, st.getDividerOffsetX() * mDpToPx,
211211
st.getBottomMargin() * mDpToPx, st.getCloseButtonPadding() * mDpToPx,
212-
st.getCloseButton().getOpacity(), st.getDividerOpacity(), st.isLoading(),
213-
st.getLoadingSpinnerRotation(), st.getBrightness(), st.getContainerOpacity(),
214-
layerTitleCache, resourceManager);
212+
st.getCloseButton().getOpacity(), st.isStartDividerVisible(),
213+
st.isEndDividerVisible(), st.isLoading(), st.getLoadingSpinnerRotation(),
214+
st.getBrightness(), st.getContainerOpacity(), layerTitleCache, resourceManager);
215215
}
216216
}
217217

@@ -255,9 +255,10 @@ void putStripTabLayer(long nativeTabStripSceneLayer, TabStripSceneLayer caller,
255255
int handleOutlineTint, boolean foreground, boolean closePressed, float toolbarWidth,
256256
float x, float y, float width, float height, float contentOffsetX,
257257
float contentOffsetY, float dividerOffsetX, float bottomOffsetY,
258-
float closeButtonPadding, float closeButtonAlpha, float dividerAlpha,
259-
boolean isLoading, float spinnerRotation, float brightness, float opacity,
260-
LayerTitleCache layerTitleCache, ResourceManager resourceManager);
258+
float closeButtonPadding, float closeButtonAlpha, boolean isStartDividerVisible,
259+
boolean isEndDividerVisible, boolean isLoading, float spinnerRotation,
260+
float brightness, float opacity, LayerTitleCache layerTitleCache,
261+
ResourceManager resourceManager);
261262
void setContentTree(
262263
long nativeTabStripSceneLayer, TabStripSceneLayer caller, SceneLayer contentTree);
263264
}

chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java

+47-15
Original file line numberDiff line numberDiff line change
@@ -487,21 +487,53 @@ public void testUpdateDividers_WithTabSelected() {
487487
// Trigger update to set divider values.
488488
mStripLayoutHelper.updateLayout(TIMESTAMP);
489489

490-
// Verify tabs 2 and 3's dividers are hidden due to selection.
491-
float hiddenOpacity = StripLayoutHelper.DIVIDER_HIDDEN_OPACITY;
492-
float visibleOpacity = StripLayoutHelper.DIVIDER_DEFAULT_OPACITY;
493-
// clang-format off
494-
assertEquals("First divider should always be hidden.",
495-
hiddenOpacity, tabs[0].getDividerOpacity(), EPSILON);
496-
assertEquals("Divider should be at default opacity.",
497-
visibleOpacity, tabs[1].getDividerOpacity(), EPSILON);
498-
assertEquals("Divider is adjacent to selected tab and should be hidden.",
499-
hiddenOpacity, tabs[2].getDividerOpacity(), EPSILON);
500-
assertEquals("Divider is adjacent to selected tab and should be hidden.",
501-
hiddenOpacity, tabs[3].getDividerOpacity(), EPSILON);
502-
assertEquals("Divider should be at default opacity.",
503-
visibleOpacity, tabs[4].getDividerOpacity(), EPSILON);
504-
// clang-format on
490+
// Verify tabs 2 and 3's start dividers are hidden due to selection.
491+
assertFalse(
492+
"First start divider should always be hidden.", tabs[0].isStartDividerVisible());
493+
assertTrue("Start divider should be visible.", tabs[1].isStartDividerVisible());
494+
assertFalse("Start divider is for selected tab and should be hidden.",
495+
tabs[2].isStartDividerVisible());
496+
assertFalse("Start divider is adjacent to selected tab and should be hidden.",
497+
tabs[3].isStartDividerVisible());
498+
assertTrue("Start divider should be visible.", tabs[4].isStartDividerVisible());
499+
500+
// Verify only last tab's end divider is visible.
501+
assertFalse("End divider should be hidden.", tabs[0].isEndDividerVisible());
502+
assertFalse("End divider should be hidden.", tabs[1].isEndDividerVisible());
503+
assertFalse("End divider should be hidden.", tabs[2].isEndDividerVisible());
504+
assertFalse("End divider should be hidden.", tabs[3].isEndDividerVisible());
505+
assertTrue("End divider should be visible.", tabs[4].isEndDividerVisible());
506+
}
507+
508+
@Test
509+
@Feature("Tab Strip Redesign")
510+
public void testUpdateDividers_InReorderMode() {
511+
// Setup with 5 tabs. Select 2nd tab.
512+
initializeTest(false, false, true, 1, 5);
513+
mStripLayoutHelper.onSizeChanged(SCREEN_WIDTH, SCREEN_HEIGHT, false, TIMESTAMP);
514+
// group 2nd and 3rd tab.
515+
groupTabs(1, 3);
516+
517+
// Start reorder mode at 2nd tab
518+
mStripLayoutHelper.startReorderModeAtIndexForTesting(1);
519+
// Trigger update to set divider values.
520+
mStripLayoutHelper.updateLayout(TIMESTAMP);
521+
522+
StripLayoutTab[] tabs = mStripLayoutHelper.getStripLayoutTabs();
523+
// Verify only 4th and 5th tab's start divider is visible.
524+
assertFalse(
525+
"First start divider should always be hidden.", tabs[0].isStartDividerVisible());
526+
assertFalse("Start divider should be hidden.", tabs[1].isStartDividerVisible());
527+
assertFalse("Start divider should be hidden.", tabs[2].isStartDividerVisible());
528+
assertTrue("Start divider should be hidden.", tabs[3].isStartDividerVisible());
529+
assertTrue("Start divider should be visible.", tabs[4].isStartDividerVisible());
530+
531+
// Verify end divider visible for 1st and 5th tab.
532+
assertTrue("End divider should be visible.", tabs[0].isEndDividerVisible());
533+
assertFalse("End divider should be hidden.", tabs[1].isEndDividerVisible());
534+
assertFalse("End divider should be hidden.", tabs[2].isEndDividerVisible());
535+
assertFalse("End divider should be hidden.", tabs[3].isEndDividerVisible());
536+
assertTrue("End divider should be visible.", tabs[4].isEndDividerVisible());
505537
}
506538

507539
@Test

chrome/browser/android/compositor/layer/tab_handle_layer.cc

+34-13
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ void TabHandleLayer::SetProperties(
4444
float bottom_offset_y,
4545
float close_button_padding,
4646
float close_button_alpha,
47-
float divider_alpha,
47+
bool is_start_divider_visible,
48+
bool is_end_divider_visible,
4849
bool is_loading,
4950
float spinner_rotation,
5051
float brightness,
@@ -165,17 +166,36 @@ void TabHandleLayer::SetProperties(
165166
}
166167
}
167168

168-
if (divider_alpha == 0.f) {
169-
divider_->SetIsDrawable(false);
169+
int divider_y;
170+
float divider_y_offset_mid =
171+
(tab_handle_resource->padding().y() + height) / 2 -
172+
start_divider_->bounds().height() / 2;
173+
if (is_tab_strip_redesign_enabled) {
174+
divider_y = content_offset_y;
170175
} else {
171-
divider_->SetIsDrawable(true);
172-
divider_->SetUIResourceId(divider_resource->ui_resource()->id());
173-
divider_->SetBounds(divider_resource->size());
174-
int divider_y = (tab_handle_resource->padding().y() + height) / 2 -
175-
divider_->bounds().height() / 2;
176+
divider_y = divider_y_offset_mid;
177+
}
178+
179+
if (!is_start_divider_visible) {
180+
start_divider_->SetIsDrawable(false);
181+
} else {
182+
start_divider_->SetIsDrawable(true);
183+
start_divider_->SetUIResourceId(divider_resource->ui_resource()->id());
184+
start_divider_->SetBounds(divider_resource->size());
176185
int divider_x = is_rtl ? width - divider_offset_x : divider_offset_x;
177-
divider_->SetPosition(gfx::PointF(divider_x, divider_y));
178-
divider_->SetOpacity(divider_alpha);
186+
start_divider_->SetPosition(gfx::PointF(divider_x, divider_y));
187+
start_divider_->SetOpacity(1.0f);
188+
}
189+
190+
if (!is_end_divider_visible) {
191+
end_divider_->SetIsDrawable(false);
192+
} else {
193+
end_divider_->SetIsDrawable(true);
194+
end_divider_->SetUIResourceId(divider_resource->ui_resource()->id());
195+
end_divider_->SetBounds(divider_resource->size());
196+
int divider_x = is_rtl ? divider_offset_x : width - divider_offset_x;
197+
end_divider_->SetPosition(gfx::PointF(divider_x, divider_y));
198+
end_divider_->SetOpacity(1.0f);
179199
}
180200

181201
if (title_layer) {
@@ -205,7 +225,6 @@ void TabHandleLayer::SetProperties(
205225
title_layer->SetIsLoading(false);
206226
}
207227
}
208-
209228
if (close_button_alpha == 0.f) {
210229
close_button_->SetIsDrawable(false);
211230
} else {
@@ -241,7 +260,8 @@ TabHandleLayer::TabHandleLayer(LayerTitleCache* layer_title_cache)
241260
layer_(cc::Layer::Create()),
242261
tab_(cc::Layer::Create()),
243262
close_button_(cc::UIResourceLayer::Create()),
244-
divider_(cc::UIResourceLayer::Create()),
263+
start_divider_(cc::UIResourceLayer::Create()),
264+
end_divider_(cc::UIResourceLayer::Create()),
245265
decoration_tab_(cc::NinePatchLayer::Create()),
246266
tab_outline_(cc::NinePatchLayer::Create()),
247267
brightness_(1.0f),
@@ -255,7 +275,8 @@ TabHandleLayer::TabHandleLayer(LayerTitleCache* layer_title_cache)
255275
// The divider is added as a separate child so its opacity can be controlled
256276
// separately from the other tab items.
257277
layer_->AddChild(tab_);
258-
layer_->AddChild(divider_);
278+
layer_->AddChild(start_divider_);
279+
layer_->AddChild(end_divider_);
259280
}
260281

261282
TabHandleLayer::~TabHandleLayer() {

chrome/browser/android/compositor/layer/tab_handle_layer.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ class TabHandleLayer : public Layer {
5151
float bottom_offset_y,
5252
float close_button_padding,
5353
float close_button_alpha,
54-
float divider_alpha,
54+
bool is_start_divider_visible,
55+
bool is_end_divider_visible,
5556
bool is_loading,
5657
float spinner_rotation,
5758
float brightness,
@@ -69,7 +70,8 @@ class TabHandleLayer : public Layer {
6970
scoped_refptr<cc::Layer> layer_;
7071
scoped_refptr<cc::Layer> tab_;
7172
scoped_refptr<cc::UIResourceLayer> close_button_;
72-
scoped_refptr<cc::UIResourceLayer> divider_;
73+
scoped_refptr<cc::UIResourceLayer> start_divider_;
74+
scoped_refptr<cc::UIResourceLayer> end_divider_;
7375
scoped_refptr<cc::NinePatchLayer> decoration_tab_;
7476
scoped_refptr<cc::NinePatchLayer> tab_outline_;
7577
scoped_refptr<cc::Layer> title_layer_;

chrome/browser/android/compositor/scene_layer/tab_strip_scene_layer.cc

+5-4
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,8 @@ void TabStripSceneLayer::PutStripTabLayer(
410410
jfloat bottom_offset_y,
411411
jfloat close_button_padding,
412412
jfloat close_button_alpha,
413-
jfloat divider_alpha,
413+
jboolean is_start_divider_visible,
414+
jboolean is_end_divider_visible,
414415
jboolean is_loading,
415416
jfloat spinner_rotation,
416417
jfloat brightness,
@@ -437,9 +438,9 @@ void TabStripSceneLayer::PutStripTabLayer(
437438
id, close_button_resource, divider_resource, tab_handle_resource,
438439
tab_handle_outline_resource, foreground, close_pressed, toolbar_width, x,
439440
y, width, height, content_offset_x, content_offset_y, divider_offset_x,
440-
bottom_offset_y, close_button_padding, close_button_alpha, divider_alpha,
441-
is_loading, spinner_rotation, brightness, opacity,
442-
tab_strip_redesign_enabled);
441+
bottom_offset_y, close_button_padding, close_button_alpha,
442+
is_start_divider_visible, is_end_divider_visible, is_loading,
443+
spinner_rotation, brightness, opacity, tab_strip_redesign_enabled);
443444
}
444445

445446
scoped_refptr<TabHandleLayer> TabStripSceneLayer::GetNextLayer(

chrome/browser/android/compositor/scene_layer/tab_strip_scene_layer.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ class TabStripSceneLayer : public SceneLayer {
151151
jfloat bottom_offset_y,
152152
jfloat close_button_padding,
153153
jfloat close_button_alpha,
154-
jfloat divider_alpha,
154+
jboolean is_start_divider_visible,
155+
jboolean is_end_divider_visible,
155156
jboolean is_loading,
156157
jfloat spinner_rotation,
157158
jfloat brightness,

0 commit comments

Comments
 (0)