Skip to content

Commit d4c5b7d

Browse files
Sirisha KavuluruChromium LUCI CQ
Sirisha Kavuluru
authored and
Chromium LUCI CQ
committed
[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 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-Commit-Position: refs/heads/main@{#1100203}
1 parent ad4a4bb commit d4c5b7d

File tree

8 files changed

+151
-70
lines changed

8 files changed

+151
-70
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

+33-20
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ void TabHandleLayer::SetProperties(
4848
float bottom_offset_y,
4949
float close_button_padding,
5050
float close_button_alpha,
51-
float divider_alpha,
51+
bool is_start_divider_visible,
52+
bool is_end_divider_visible,
5253
bool is_loading,
5354
float spinner_rotation,
5455
float brightness,
@@ -168,25 +169,36 @@ void TabHandleLayer::SetProperties(
168169
}
169170
}
170171

171-
if (divider_alpha == 0.f) {
172-
divider_->SetIsDrawable(false);
172+
int divider_y;
173+
float divider_y_offset_mid =
174+
(tab_handle_resource->padding().y() + height) / 2 -
175+
start_divider_->bounds().height() / 2;
176+
if (is_tab_strip_redesign_enabled) {
177+
divider_y = content_offset_y;
173178
} else {
174-
divider_->SetIsDrawable(true);
175-
divider_->SetUIResourceId(divider_resource->ui_resource()->id());
176-
divider_->SetBounds(divider_resource->size());
179+
divider_y = divider_y_offset_mid;
180+
}
177181

178-
int divider_y;
179-
float divider_y_offset_mid =
180-
(tab_handle_resource->padding().y() + height) / 2 -
181-
divider_->bounds().height() / 2;
182-
if (is_tab_strip_redesign_enabled) {
183-
divider_y = content_offset_y;
184-
} else {
185-
divider_y = divider_y_offset_mid;
186-
}
182+
if (!is_start_divider_visible) {
183+
start_divider_->SetIsDrawable(false);
184+
} else {
185+
start_divider_->SetIsDrawable(true);
186+
start_divider_->SetUIResourceId(divider_resource->ui_resource()->id());
187+
start_divider_->SetBounds(divider_resource->size());
187188
int divider_x = is_rtl ? width - divider_offset_x : divider_offset_x;
188-
divider_->SetPosition(gfx::PointF(divider_x, divider_y));
189-
divider_->SetOpacity(divider_alpha);
189+
start_divider_->SetPosition(gfx::PointF(divider_x, divider_y));
190+
start_divider_->SetOpacity(1.0f);
191+
}
192+
193+
if (!is_end_divider_visible) {
194+
end_divider_->SetIsDrawable(false);
195+
} else {
196+
end_divider_->SetIsDrawable(true);
197+
end_divider_->SetUIResourceId(divider_resource->ui_resource()->id());
198+
end_divider_->SetBounds(divider_resource->size());
199+
int divider_x = is_rtl ? divider_offset_x : width - divider_offset_x;
200+
end_divider_->SetPosition(gfx::PointF(divider_x, divider_y));
201+
end_divider_->SetOpacity(1.0f);
190202
}
191203

192204
if (title_layer) {
@@ -217,7 +229,6 @@ void TabHandleLayer::SetProperties(
217229
title_layer->SetIsLoading(false);
218230
}
219231
}
220-
221232
if (close_button_alpha == 0.f) {
222233
close_button_->SetIsDrawable(false);
223234
} else {
@@ -255,7 +266,8 @@ TabHandleLayer::TabHandleLayer(LayerTitleCache* layer_title_cache)
255266
layer_(cc::slim::Layer::Create()),
256267
tab_(cc::slim::Layer::Create()),
257268
close_button_(cc::slim::UIResourceLayer::Create()),
258-
divider_(cc::slim::UIResourceLayer::Create()),
269+
start_divider_(cc::slim::UIResourceLayer::Create()),
270+
end_divider_(cc::slim::UIResourceLayer::Create()),
259271
decoration_tab_(cc::slim::NinePatchLayer::Create()),
260272
tab_outline_(cc::slim::NinePatchLayer::Create()),
261273
brightness_(1.0f),
@@ -269,7 +281,8 @@ TabHandleLayer::TabHandleLayer(LayerTitleCache* layer_title_cache)
269281
// The divider is added as a separate child so its opacity can be controlled
270282
// separately from the other tab items.
271283
layer_->AddChild(tab_);
272-
layer_->AddChild(divider_);
284+
layer_->AddChild(start_divider_);
285+
layer_->AddChild(end_divider_);
273286
}
274287

275288
TabHandleLayer::~TabHandleLayer() {

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ class TabHandleLayer : public Layer {
4949
float bottom_offset_y,
5050
float close_button_padding,
5151
float close_button_alpha,
52-
float divider_alpha,
52+
bool is_start_divider_visible,
53+
bool is_end_divider_visible,
5354
bool is_loading,
5455
float spinner_rotation,
5556
float brightness,
@@ -67,7 +68,8 @@ class TabHandleLayer : public Layer {
6768
scoped_refptr<cc::slim::Layer> layer_;
6869
scoped_refptr<cc::slim::Layer> tab_;
6970
scoped_refptr<cc::slim::UIResourceLayer> close_button_;
70-
scoped_refptr<cc::slim::UIResourceLayer> divider_;
71+
scoped_refptr<cc::slim::UIResourceLayer> start_divider_;
72+
scoped_refptr<cc::slim::UIResourceLayer> end_divider_;
7173
scoped_refptr<cc::slim::NinePatchLayer> decoration_tab_;
7274
scoped_refptr<cc::slim::NinePatchLayer> tab_outline_;
7375
scoped_refptr<cc::slim::Layer> title_layer_;

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

+5-4
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,8 @@ void TabStripSceneLayer::PutStripTabLayer(
413413
jfloat bottom_offset_y,
414414
jfloat close_button_padding,
415415
jfloat close_button_alpha,
416-
jfloat divider_alpha,
416+
jboolean is_start_divider_visible,
417+
jboolean is_end_divider_visible,
417418
jboolean is_loading,
418419
jfloat spinner_rotation,
419420
jfloat brightness,
@@ -440,9 +441,9 @@ void TabStripSceneLayer::PutStripTabLayer(
440441
id, close_button_resource, divider_resource, tab_handle_resource,
441442
tab_handle_outline_resource, foreground, close_pressed, toolbar_width, x,
442443
y, width, height, content_offset_x, content_offset_y, divider_offset_x,
443-
bottom_offset_y, close_button_padding, close_button_alpha, divider_alpha,
444-
is_loading, spinner_rotation, brightness, opacity,
445-
tab_strip_redesign_enabled);
444+
bottom_offset_y, close_button_padding, close_button_alpha,
445+
is_start_divider_visible, is_end_divider_visible, is_loading,
446+
spinner_rotation, brightness, opacity, tab_strip_redesign_enabled);
446447
}
447448

448449
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)