From bd0f311ef5a2e61da16d7c3da8360f99307163e3 Mon Sep 17 00:00:00 2001 From: Neil Coronado Date: Wed, 7 Dec 2022 01:35:40 +0000 Subject: [PATCH] [Merge 109] Fix reorder mode autoscroll for tab groups on strip Fix reorder mode autoscroll for tab groups on strip. -Restore auto-scroll upon entering reorder mode. -Allocate additional space to auto-scroll when the tabs do not fill the tab strip. Long tabstrip vid: https://drive.google.com/file/d/1tsqAx95D0xtj6TqPG8zKd2shTQb5Nk-B/view?usp=sharing Short tabstrip vid: https://drive.google.com/file/d/1V2UcUMOKQlbFsUiy1cqUDP_-OT-zsGhU/view?usp=sharing (cherry picked from commit 23587924887fc8ab5b1d0c701b39ee779b8e3eb4) Bug: 1374918, 1368742 Change-Id: I6acf7cdfe56987d824de107d1f75029f55b669f7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4063359 Commit-Queue: Neil Coronado Reviewed-by: Sirisha Kavuluru Cr-Original-Commit-Position: refs/heads/main@{#1077748} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4080600 Cr-Commit-Position: refs/branch-heads/5414@{#497} Cr-Branched-From: 4417ee59d7bf6df7a9c9ea28f7722d2ee6203413-refs/heads/main@{#1070088} --- .../overlays/strip/StripLayoutHelper.java | 26 +++++++++++++--- .../overlays/strip/StripLayoutHelperTest.java | 31 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java index 3b9723af093705..8e3f2fa23a1314 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java @@ -184,6 +184,7 @@ public Float get(StripLayoutHelper object) { private float mLastReorderX; private float mTabMarginWidth; private float mStripStartMarginForReorder; + private float mReorderExtraMinScrollOffset; private long mLastReorderScrollTime; private long mLastUpdateTime; private long mHoverStartTime; @@ -1278,7 +1279,7 @@ private void updateSpinners(long time) { private void updateScrollOffsetPosition(float pos) { float oldScrollOffset = mScrollOffset; - mScrollOffset = MathUtils.clamp(pos, mMinScrollOffset, 0); + mScrollOffset = MathUtils.clamp(pos, mMinScrollOffset - mReorderExtraMinScrollOffset, 0); if (mInReorderMode && mScroller.isFinished()) { float delta = MathUtils.flipSignIf( @@ -1830,7 +1831,13 @@ private void autoScrollForTabGroupMargins( float startValue = mScrollOffset - startMarginDelta; float endValue = startValue - delta; - if (startValue < mMinScrollOffset) return; + // If the current tab width is at its max, this means there are not enough tabs to fill the + // visible area on the tab strip. In this case, there is not enough room to auto-scroll for + // tab group margins. Allocate additional space to account for this. See + // http://crbug.com/1374918 for additional details. + if (mCachedTabWidth == mMaxTabWidth) { + mReorderExtraMinScrollOffset = mStripStartMarginForReorder + Math.abs(delta); + } if (animationList != null) { CompositorAnimator scrollAnimator = @@ -1842,7 +1849,7 @@ private void autoScrollForTabGroupMargins( } } - private AnimatorListener getTabGroupMarginAnimatorListener() { + private AnimatorListener getTabGroupMarginAnimatorListener(boolean resetExtraMinScrollOffset) { return new AnimatorListenerAdapter() { @Override public void onAnimationStart(Animator animation) { @@ -1852,6 +1859,7 @@ public void onAnimationStart(Animator animation) { @Override public void onAnimationEnd(Animator animation) { mTabGroupMarginAnimRunning = false; + if (resetExtraMinScrollOffset) mReorderExtraMinScrollOffset = 0.f; } }; } @@ -1905,7 +1913,7 @@ private void computeAndUpdateTabGroupMargins(boolean autoScroll, boolean animate // 4. Begin slide-out and scroll animation. Update tab positions. if (animationList != null) { - startAnimationList(animationList, getTabGroupMarginAnimatorListener()); + startAnimationList(animationList, getTabGroupMarginAnimatorListener(false)); } else { computeTabInitialPositions(); } @@ -1934,7 +1942,7 @@ private void resetTabGroupMargins() { mStripStartMarginForReorder = 0f; // 3. Begin slide-out and scroll animation. - startAnimationList(animationList, getTabGroupMarginAnimatorListener()); + startAnimationList(animationList, getTabGroupMarginAnimatorListener(true)); } private void setCompositorButtonsVisible(boolean visible) { @@ -2583,6 +2591,14 @@ float getMinimumScrollOffset() { return mMinScrollOffset; } + /** + * @return The strip's additional minimum scroll offset for reorder mode. + */ + @VisibleForTesting + float getReorderExtraMinScrollOffset() { + return mReorderExtraMinScrollOffset; + } + /** * @return The scroller. */ diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java index dd9e176d939624..59ce7121ab5786 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java @@ -1399,6 +1399,37 @@ public void testReorder_MergeToGroup() { .mergeTabsToGroup(eq(thirdTab.getId()), eq(oldSecondTabId), eq(true)); } + @Test + @Feature("Tab Groups on Tab Strip") + public void testReorder_NoExtraMinScroll() { + // Mock 3 tabs. Group the first two tabs. + initializeTest(false, false, true, 0, 3); + mStripLayoutHelper.onSizeChanged(SCREEN_WIDTH, SCREEN_HEIGHT, false, TIMESTAMP); + + // Start reorder mode on third tab. + mStripLayoutHelper.startReorderModeAtIndexForTesting(2); + + // Verify extra scroll offset. + assertEquals("Extra min offset should not be set.", 0f, + mStripLayoutHelper.getReorderExtraMinScrollOffset(), EPSILON); + } + + @Test + @Feature("Tab Groups on Tab Strip") + public void testReorder_ExtraMinScroll() { + // Mock 3 tabs. Group the first two tabs. + initializeTest(false, false, true, 0, 3); + mStripLayoutHelper.onSizeChanged(SCREEN_WIDTH, SCREEN_HEIGHT, false, TIMESTAMP); + groupTabs(0, 2); + + // Start reorder mode on third tab. + mStripLayoutHelper.startReorderModeAtIndexForTesting(2); + + // Verify extra scroll offset. + assertNotEquals("Extra min offset should be set.", 0f, + mStripLayoutHelper.getReorderExtraMinScrollOffset(), EPSILON); + } + @Test @Feature("Tab Strip Improvements") public void testTabClosing_NoTabResize() {