Skip to content

Commit

Permalink
[Tab Strip Redesign] Set new tab button to a fixed position
Browse files Browse the repository at this point in the history 
  - Add padding between tab strip end and model selector button = touch_target_size - button_bg_size) / 2.
  - Add padding between model selector button and new tab button = 8dp.

Final result(Folio example, detached right end padding would be 5dp because detached NTB bg size is 2dp larger than that of folio):

Width without MSB:
tab | 16dp | NTB | 6dp

Width with MSB:
tab | 16dp | NTB | 8dp | MSB | 6dp

incognito off: https://screenshot.googleplex.com/5oPeMwnzbohLcW2
incognito on: https://screenshot.googleplex.com/4ViPBVoW2uBnvdZ

RTL incognito on: https://screenshot.googleplex.com/BvFKZWFbtRCo3Qi
RTL incognito off: https://screenshot.googleplex.com/8ZCpRFqyArpET8k

Recording for tab scroll and reorder mode:
https://drive.google.com/file/d/1U6gled8A4D5XxE6rpnbP7bukOKcUMhDw/view?usp=sharing&resourcekey=0--h9MnZOEs-DacD-KEt9K5A


Bug: 1408257
Change-Id: I93927773f85232c596f43dd1d87f5f0be6669e12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189702
Commit-Queue: Zhe Li <zheliooo@google.com>
Reviewed-by: Sirisha Kavuluru <skavuluru@google.com>
Reviewed-by: Neil Coronado <nemco@google.com>
Cr-Commit-Position: refs/heads/main@{#1100798}
  • Loading branch information
Zhe Li authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent 2f610ec commit d8871a4
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 52 deletions.
78 changes: 51 additions & 27 deletions ...oid/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public Float get(StripLayoutHelper object) {
private static final float FOLIO_ATTACHED_BOTTOM_MARGIN_DP = 0.f;
private static final float FOLIO_ANIM_INTERMEDIATE_MARGIN_DP = -12.f;
private static final float FOLIO_DETACHED_BOTTOM_MARGIN_DP = 4.f;
private static final float NEW_TAB_BUTTON_DESIRED_TOUCH_TARGET_SIZE = 48.f;
private static final float BUTTON_DESIRED_TOUCH_TARGET_SIZE = 48.f;
private static final float NEW_TAB_BUTTON_DEFAULT_PRESSED_OPACITY = 0.2f;
private static final float NEW_TAB_BUTTON_DARK_DETACHED_OPACITY = 0.15f;
static final float TAB_OPACITY_HIDDEN = 0.f;
Expand Down Expand Up @@ -225,6 +225,10 @@ public Float get(StripLayoutHelper object) {
private long mLastSpinnerUpdate;
private float mLeftMargin;
private float mRightMargin;

// New tab button with tab strip end padding
private float mNewTabButtonWithTabStripEndPadding;

private final boolean mIncognito;
// Whether the CascadingStripStacker should be used.
private boolean mShouldCascadeTabs;
Expand Down Expand Up @@ -271,12 +275,20 @@ public StripLayoutHelper(Context context, LayoutUpdateHost updateHost,
mModelSelectorButton = modelSelectorButton;
mTabStripImpEnabled = ChromeFeatureList.sTabStripImprovements.isEnabled();

if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
mNewTabButtonWithTabStripEndPadding =
(BUTTON_DESIRED_TOUCH_TARGET_SIZE - mNewTabButtonWidth) / 2;
} else {
mNewTabButtonWithTabStripEndPadding = NEW_TAB_BUTTON_PADDING_DP;
}

mRightMargin = LocalizationUtils.isLayoutRtl()
? 0
: mNewTabButtonWidth + NEW_TAB_BUTTON_PADDING_DP;
: mNewTabButtonWithTabStripEndPadding + mNewTabButtonWidth;
mLeftMargin = LocalizationUtils.isLayoutRtl()
? mNewTabButtonWidth + NEW_TAB_BUTTON_PADDING_DP
? mNewTabButtonWithTabStripEndPadding + mNewTabButtonWidth
: 0;

mMinTabWidth = TabUiFeatureUtilities.getTabMinWidth();

mMaxTabWidth = TabUiThemeUtil.getMaxTabStripTabWidthDp();
Expand Down Expand Up @@ -443,7 +455,7 @@ public float getNewTabButtonTouchTargetOffset() {
boolean isRtl = LocalizationUtils.isLayoutRtl();
if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
float newTabButtonTouchTargetOffsetTSR =
(NEW_TAB_BUTTON_DESIRED_TOUCH_TARGET_SIZE - mNewTabButtonWidth) / 2;
(BUTTON_DESIRED_TOUCH_TARGET_SIZE - mNewTabButtonWidth) / 2;
return isRtl ? newTabButtonTouchTargetOffsetTSR : -newTabButtonTouchTargetOffsetTSR;
}
return isRtl ? NEW_TAB_BUTTON_TOUCH_TARGET_OFFSET : -NEW_TAB_BUTTON_TOUCH_TARGET_OFFSET;
Expand Down Expand Up @@ -526,9 +538,9 @@ public void setTabStacker(StripStacker stacker) {
*/
public void setEndMargin(float margin) {
if (LocalizationUtils.isLayoutRtl()) {
mLeftMargin = margin + mNewTabButtonWidth + NEW_TAB_BUTTON_PADDING_DP;
mLeftMargin = margin + mNewTabButtonWithTabStripEndPadding + mNewTabButtonWidth;
} else {
mRightMargin = margin + mNewTabButtonWidth + NEW_TAB_BUTTON_PADDING_DP;
mRightMargin = margin + mNewTabButtonWithTabStripEndPadding + mNewTabButtonWidth;
}
}

Expand Down Expand Up @@ -1279,8 +1291,12 @@ private void resizeStripOnTabClose(int tabId, Tab nextTab) {
}

// 4. Add new tab button offset animation.
CompositorAnimator newTabButtonOffsetAnimator = updateNewTabButtonState(true);
if (newTabButtonOffsetAnimator != null) tabStripAnimators.add(newTabButtonOffsetAnimator);
if (!ChromeFeatureList.sTabStripRedesign.isEnabled()) {
CompositorAnimator newTabButtonOffsetAnimator = updateNewTabButtonState(true);
if (newTabButtonOffsetAnimator != null) {
tabStripAnimators.add(newTabButtonOffsetAnimator);
};
}

// 5. Add animation completion listener and start animations.
startAnimationList(tabStripAnimators, new AnimatorListenerAdapter() {
Expand Down Expand Up @@ -1716,29 +1732,37 @@ private void createRenderList() {
}

private CompositorAnimator updateNewTabButtonState(boolean animate) {
// 1. The new tab button is faded out/in when entering/exiting reorder mode.
if (mInReorderMode || mStripTabs.length == 0) return null;
if (!ChromeFeatureList.sTabStripRedesign.isEnabled()) {
// 1. The new tab button is faded out/in when entering/exiting reorder mode.
if (mInReorderMode || mStripTabs.length == 0) return null;

// 2. Get offset from strip stacker.
float offset = mStripStacker.computeNewTabButtonOffset(mStripTabs, mTabOverlapWidth,
mLeftMargin, mRightMargin, mWidth, mNewTabButtonWidth,
Math.abs(getNewTabButtonTouchTargetOffset()), mCachedTabWidth, animate);
// 2. Get offset from strip stacker.
float offset = mStripStacker.computeNewTabButtonOffset(mStripTabs, mTabOverlapWidth,
mLeftMargin, mRightMargin, mWidth, mNewTabButtonWidth,
Math.abs(getNewTabButtonTouchTargetOffset()), mCachedTabWidth, animate);

// 3. Hide the new tab button if it's not visible on the screen.
boolean isRtl = LocalizationUtils.isLayoutRtl();
if ((isRtl && offset + mNewTabButtonWidth < 0) || (!isRtl && offset > mWidth)) {
mNewTabButton.setVisible(false);
return null;
}
mNewTabButton.setVisible(true);
// 3. Hide the new tab button if it's not visible on the screen.
boolean isRtl = LocalizationUtils.isLayoutRtl();
if ((isRtl && offset + mNewTabButtonWidth < 0) || (!isRtl && offset > mWidth)) {
mNewTabButton.setVisible(false);
return null;
}
mNewTabButton.setVisible(true);

// 4. Position the new tab button.
if (animate) {
return CompositorAnimator.ofFloatProperty(mUpdateHost.getAnimationHandler(),
mNewTabButton, CompositorButton.DRAW_X, mNewTabButton.getX(), offset,
NEW_TAB_BUTTON_OFFSET_MOVE_MS);
// 4. Position the new tab button.
if (animate) {
return CompositorAnimator.ofFloatProperty(mUpdateHost.getAnimationHandler(),
mNewTabButton, CompositorButton.DRAW_X, mNewTabButton.getX(), offset,
NEW_TAB_BUTTON_OFFSET_MOVE_MS);
} else {
mNewTabButton.setX(offset);
}
} else {
mNewTabButton.setX(offset);
if (!LocalizationUtils.isLayoutRtl()) {
mNewTabButton.setX(mWidth - mRightMargin);
} else {
mNewTabButton.setX(mLeftMargin - mNewTabButtonWidth);
}
}
return null;
}
Expand Down
21 changes: 17 additions & 4 deletions ...a/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public class StripLayoutHelperManager implements SceneOverlay, PauseResumeWithNa
private static final float MODEL_SELECTOR_BUTTON_BACKGROUND_WIDTH_DP_DETACHED = 38.f;
private static final float MODEL_SELECTOR_BUTTON_BACKGROUND_HEIGHT_DP_DETACHED = 38.f;
private static final float MODEL_SELECTOR_BUTTON_CLICK_SLOP_DP = 12.f;
private static final float NEW_TAB_BUTTON_WITH_MODEL_SELECTOR_BUTTON_PADDING = 8.f;
private static final float BUTTON_DESIRED_TOUCH_TARGET_SIZE = 48.f;

// External influences
private TabModelSelector mTabModelSelector;
Expand Down Expand Up @@ -459,11 +461,10 @@ public void onSizeChanged(
}
if (!LocalizationUtils.isLayoutRtl()) {
mModelSelectorButton.setX(
mWidth - mModelSelectorWidth - MODEL_SELECTOR_BUTTON_PADDING_DP);
mWidth - mModelSelectorWidth - getModelSelectorButtonWithTabStripEndPadding());
} else {
mModelSelectorButton.setX(MODEL_SELECTOR_BUTTON_PADDING_DP);
mModelSelectorButton.setX(getModelSelectorButtonWithTabStripEndPadding());
}

updateStripScrim();

mNormalHelper.onSizeChanged(mWidth, mHeight, orientationChanged, LayoutManagerImpl.time());
Expand All @@ -490,8 +491,20 @@ private void updateStripScrim() {
mStripScrim.setX(drawX);
}

private float getModelSelectorButtonWithTabStripEndPadding() {
if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
return (BUTTON_DESIRED_TOUCH_TARGET_SIZE - mModelSelectorWidth) / 2;
} else {
return MODEL_SELECTOR_BUTTON_PADDING_DP;
}
}

private float getModelSelectorButtonWidthWithPadding() {
return mModelSelectorWidth + (MODEL_SELECTOR_BUTTON_PADDING_DP * 2);
if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
return mModelSelectorWidth + NEW_TAB_BUTTON_WITH_MODEL_SELECTOR_BUTTON_PADDING;
} else {
return mModelSelectorWidth + (MODEL_SELECTOR_BUTTON_PADDING_DP * 2);
}
}

public TintedCompositorButton getNewTabButton() {
Expand Down
36 changes: 36 additions & 0 deletions ...c/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.chromium.chrome.browser.tasks.tab_management.TabManagementFieldTrial;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.styles.ChromeColors;
import org.chromium.ui.base.LocalizationUtils;

/** Tests for {@link StripLayoutHelperManager}. */
@RunWith(BaseRobolectricTestRunner.class)
Expand All @@ -59,6 +60,10 @@ public class StripLayoutHelperManagerTest {

private StripLayoutHelperManager mStripLayoutHelperManager;
private Context mContext;
private static final float SCREEN_WIDTH = 800.f;
private static final float SCREEN_HEIGHT = 1600.f;
private static final float VISBLE_VIEWPORT_Y = 200.f;
private static final int ORIENTATION = 2;

@Before
public void beforeTest() {
Expand Down Expand Up @@ -96,4 +101,35 @@ public void testGetBackgroundColorFolio() {
assertEquals(ChromeColors.getSurfaceColor(mContext, R.dimen.default_elevation_2),
mStripLayoutHelperManager.getBackgroundColor());
}

@Test
@Feature("Tab Strip Redesign")
public void testModelSelectorButtonPosition() {
// setup
TabManagementFieldTrial.TAB_STRIP_REDESIGN_ENABLE_DETACHED.setForTesting(true);

// Set model selector button position.
mStripLayoutHelperManager.onSizeChanged(
SCREEN_WIDTH, SCREEN_HEIGHT, VISBLE_VIEWPORT_Y, ORIENTATION);

// Verify model selector button position.
assertEquals("Model selector button position is not as expected", 757.f,
mStripLayoutHelperManager.getModelSelectorButton().getX(), 0.0);
}

@Test
@Feature("Tab Strip Redesign")
public void testModelSelectorButtonPosition_RTL() {
// setup
TabManagementFieldTrial.TAB_STRIP_REDESIGN_ENABLE_DETACHED.setForTesting(true);

// Set model selector button position.
LocalizationUtils.setRtlForTesting(true);
mStripLayoutHelperManager.onSizeChanged(
SCREEN_WIDTH, SCREEN_HEIGHT, VISBLE_VIEWPORT_Y, ORIENTATION);

// Verify model selector button position.
assertEquals("Model selector button position is not as expected", 5.f,
mStripLayoutHelperManager.getModelSelectorButton().getX(), 0.0);
}
}

0 comments on commit d8871a4

Please sign in to comment.