Commit 7e85f3d6 authored by spdonghao's avatar spdonghao Committed by Commit Bot

Polish the start surface toolbar scrolling.

In this CL, we do two changes:
1. Add a start surface check in
ToolbarManager#ToolbarNtpDelegate#isLocationBarShown.

Before[1], this returns true even if it's in start surface v2 homepage,
that causes the ToolbarPhone[2] set background color transparent[3].
[1] Demo:
https://drive.google.com/file/d/1dPJwI4lK-9ch5Wwnx06oY9IIXenLIGzr/view?usp=sharing
[2] https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=2366
[3] https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=712

2. Add a shrink animation for omnibox when it's about to be scrolled up
and toolbar container is showing.

Please see demo (The left one is the fixed one):
https://drive.google.com/file/d/1jRfYGx9lD3UfxaTabhaNe9Cs_YWTJt2J/view?usp=sharing

Bug: 1113852, 1136595
Change-Id: I549223c7a8b7c2e2f56ab62e0d0a090e1185fc96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517544
Commit-Queue: Hao Dong <spdonghao@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarXi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827029}
parent c3f7fbfe
...@@ -159,6 +159,13 @@ public class StartSurfaceCoordinator implements StartSurface { ...@@ -159,6 +159,13 @@ public class StartSurfaceCoordinator implements StartSurface {
} }
} }
@Override
public void destroy() {
if (mTasksSurface != null) {
mTasksSurface.removeFakeSearchBoxShrinkAnimation();
}
}
@Override @Override
public void addHeaderOffsetChangeListener( public void addHeaderOffsetChangeListener(
AppBarLayout.OnOffsetChangedListener onOffsetChangedListener) { AppBarLayout.OnOffsetChangedListener onOffsetChangedListener) {
...@@ -351,6 +358,7 @@ public class StartSurfaceCoordinator implements StartSurface { ...@@ -351,6 +358,7 @@ public class StartSurfaceCoordinator implements StartSurface {
mScrimCoordinator, mPropertyModel, tabSwitcherType, mParentTabSupplier, mScrimCoordinator, mPropertyModel, tabSwitcherType, mParentTabSupplier,
!excludeMVTiles, hasTrendyTerms); !excludeMVTiles, hasTrendyTerms);
mTasksSurface.getView().setId(R.id.primary_tasks_surface_view); mTasksSurface.getView().setId(R.id.primary_tasks_surface_view);
mTasksSurface.addFakeSearchBoxShrinkAnimation();
mTasksSurfacePropertyModelChangeProcessor = PropertyModelChangeProcessor.create( mTasksSurfacePropertyModelChangeProcessor = PropertyModelChangeProcessor.create(
mPropertyModel, mPropertyModel,
......
...@@ -54,17 +54,23 @@ import android.view.KeyEvent; ...@@ -54,17 +54,23 @@ import android.view.KeyEvent;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.espresso.UiController; import androidx.test.espresso.UiController;
import androidx.test.espresso.ViewAction; import androidx.test.espresso.ViewAction;
import androidx.test.espresso.action.GeneralLocation; import androidx.test.espresso.action.GeneralLocation;
import androidx.test.espresso.action.GeneralSwipeAction; import androidx.test.espresso.action.GeneralSwipeAction;
import androidx.test.espresso.action.Press; import androidx.test.espresso.action.Press;
import androidx.test.espresso.action.ScrollToAction;
import androidx.test.espresso.action.Swipe; import androidx.test.espresso.action.Swipe;
import androidx.test.espresso.action.ViewActions; import androidx.test.espresso.action.ViewActions;
import androidx.test.espresso.contrib.RecyclerViewActions; import androidx.test.espresso.contrib.RecyclerViewActions;
import androidx.test.espresso.matcher.ViewMatchers;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
import org.hamcrest.Description;
import org.hamcrest.Matcher; import org.hamcrest.Matcher;
import org.hamcrest.Matchers;
import org.hamcrest.TypeSafeMatcher;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
...@@ -100,6 +106,7 @@ import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache; ...@@ -100,6 +106,7 @@ import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache;
import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter; import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities; import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper; import org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper;
import org.chromium.chrome.browser.toolbar.top.ToolbarPhone;
import org.chromium.chrome.start_surface.R; import org.chromium.chrome.start_surface.R;
import org.chromium.chrome.test.ChromeActivityTestRule; import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate; import org.chromium.chrome.test.ChromeJUnit4RunnerDelegate;
...@@ -1385,6 +1392,85 @@ public class StartSurfaceTest { ...@@ -1385,6 +1392,85 @@ public class StartSurfaceTest {
onView(withId(R.id.search_box_text)).check(matches(isDisplayed())); onView(withId(R.id.search_box_text)).check(matches(isDisplayed()));
onView(withId(R.id.voice_search_button)).check(matches(isDisplayed())); onView(withId(R.id.voice_search_button)).check(matches(isDisplayed()));
} }
private static Matcher<View> isView(final View targetView) {
return new TypeSafeMatcher<View>() {
@Override
public void describeTo(Description description) {
description.appendText("is the targetView: ");
description.appendValue(targetView);
}
@Override
public boolean matchesSafely(View view) {
return view == targetView;
}
};
}
@Test
@MediumTest
@Feature({"StartSurface"})
// clang-format off
@CommandLineFlags.Add({BASE_PARAMS + "/single/show_stack_tab_switcher/true"
+ "/open_ntp_instead_of_start/true"})
public void testScrollToolbar() {
// clang-format on
// We need to check toolbar background color with open_ntp_instead_of_start on. This flag
// requires mImmediateReturn to be true.
assumeTrue(mImmediateReturn);
onViewWaiting(allOf(withId(R.id.feed_stream_recycler_view), isDisplayed()));
// Default scrollTo() cannot be used for RecyclerView. Add a customized scrollTo for
// scrolling to the last item of Feed.
ViewAction customizedScrollTo = new ViewAction() {
@Override
public Matcher<View> getConstraints() {
return Matchers.allOf(
ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE),
ViewMatchers.isDescendantOfA(
ViewMatchers.isAssignableFrom(RecyclerView.class)));
}
@Override
public String getDescription() {
return "scroll to";
}
@Override
public void perform(UiController uiController, View view) {
new ScrollToAction().perform(uiController, view);
}
};
RecyclerView feedView =
mActivityTestRule.getActivity().findViewById(R.id.feed_stream_recycler_view);
View lastChild = feedView.getLayoutManager().findViewByPosition(
feedView.getAdapter().getItemCount() - 1);
// Scroll to the last item of Feed. Somehow RecyclerViewActions#scrollToPosition couldn't be
// performed.
onView(isView(lastChild)).perform(customizedScrollTo, click());
// The start surface toolbar should be scrolled up and not be displayed.
assertTrue(mActivityTestRule.getActivity()
.findViewById(R.id.tab_switcher_toolbar)
.getTranslationY()
< -mActivityTestRule.getActivity().getResources().getDimensionPixelOffset(
R.dimen.toolbar_height_no_shadow));
onView(withId(R.id.tab_switcher_toolbar)).check(matches(not(isDisplayed())));
// Toolbar container view should show.
onView(withId(R.id.toolbar_container)).check(matches(isDisplayed()));
// Check the toolbar's background color.
ToolbarPhone toolbar =
mActivityTestRule.getActivity().findViewById(org.chromium.chrome.R.id.toolbar);
Assert.assertEquals(toolbar.getToolbarDataProvider().getPrimaryColor(),
toolbar.getBackgroundDrawable().getColor());
}
} }
// TODO(crbug.com/1033909): Add more integration tests. // TODO(crbug.com/1033909): Add more integration tests.
...@@ -21,6 +21,11 @@ public interface StartSurface { ...@@ -21,6 +21,11 @@ public interface StartSurface {
*/ */
void initialize(); void initialize();
/**
* Called when activity is being destroyed.
*/
void destroy();
/** /**
* An observer that is notified when the start surface internal state, excluding * An observer that is notified when the start surface internal state, excluding
* the states notified in {@link OverviewModeObserver}, is changed. * the states notified in {@link OverviewModeObserver}, is changed.
......
...@@ -90,4 +90,14 @@ public interface TasksSurface { ...@@ -90,4 +90,14 @@ public interface TasksSurface {
*/ */
void removeHeaderOffsetChangeListener( void removeHeaderOffsetChangeListener(
AppBarLayout.OnOffsetChangedListener onOffsetChangedListener); AppBarLayout.OnOffsetChangedListener onOffsetChangedListener);
/**
* Add the fake search box shrink animation.
*/
void addFakeSearchBoxShrinkAnimation();
/**
* Remove the omnibox shrink animation.
*/
void removeFakeSearchBoxShrinkAnimation();
} }
...@@ -178,4 +178,14 @@ public class TasksSurfaceCoordinator implements TasksSurface { ...@@ -178,4 +178,14 @@ public class TasksSurfaceCoordinator implements TasksSurface {
AppBarLayout.OnOffsetChangedListener onOffsetChangedListener) { AppBarLayout.OnOffsetChangedListener onOffsetChangedListener) {
mView.removeHeaderOffsetChangeListener(onOffsetChangedListener); mView.removeHeaderOffsetChangeListener(onOffsetChangedListener);
} }
@Override
public void addFakeSearchBoxShrinkAnimation() {
mView.addFakeSearchBoxShrinkAnimation();
}
@Override
public void removeFakeSearchBoxShrinkAnimation() {
mView.removeFakeSearchBoxShrinkAnimation();
}
} }
...@@ -25,6 +25,7 @@ import androidx.core.view.ViewCompat; ...@@ -25,6 +25,7 @@ import androidx.core.view.ViewCompat;
import com.google.android.material.appbar.AppBarLayout; import com.google.android.material.appbar.AppBarLayout;
import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.MathUtils;
import org.chromium.chrome.browser.feed.shared.FeedFeatures; import org.chromium.chrome.browser.feed.shared.FeedFeatures;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher; import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.ntp.IncognitoDescriptionView; import org.chromium.chrome.browser.ntp.IncognitoDescriptionView;
...@@ -44,6 +45,7 @@ class TasksView extends CoordinatorLayoutForPointer { ...@@ -44,6 +45,7 @@ class TasksView extends CoordinatorLayoutForPointer {
private FrameLayout mBodyViewContainer; private FrameLayout mBodyViewContainer;
private FrameLayout mCarouselTabSwitcherContainer; private FrameLayout mCarouselTabSwitcherContainer;
private AppBarLayout mHeaderView; private AppBarLayout mHeaderView;
private AppBarLayout.OnOffsetChangedListener mFakeSearchBoxShrinkAnimation;
private SearchBoxCoordinator mSearchBoxCoordinator; private SearchBoxCoordinator mSearchBoxCoordinator;
private IncognitoDescriptionView mIncognitoDescriptionView; private IncognitoDescriptionView mIncognitoDescriptionView;
private View.OnClickListener mIncognitoDescriptionLearnMoreListener; private View.OnClickListener mIncognitoDescriptionLearnMoreListener;
...@@ -360,4 +362,71 @@ class TasksView extends CoordinatorLayoutForPointer { ...@@ -360,4 +362,71 @@ class TasksView extends CoordinatorLayoutForPointer {
mHeaderView.removeOnOffsetChangedListener(onOffsetChangedListener); mHeaderView.removeOnOffsetChangedListener(onOffsetChangedListener);
} }
} }
/**
* Create the fake box shrink animation if it doesn't exist yet and add the omnibox shrink
* animation when the homepage is scrolled.
*/
void addFakeSearchBoxShrinkAnimation() {
if (mHeaderView == null) return;
if (mFakeSearchBoxShrinkAnimation == null) {
int fakeSearchBoxHeight =
getResources().getDimensionPixelSize(R.dimen.ntp_search_box_height);
int toolbarContainerTopMargin =
getResources().getDimensionPixelSize(R.dimen.location_bar_vertical_margin);
View fakeSearchBoxView = findViewById(R.id.search_box);
if (fakeSearchBoxView == null) return;
// If fake search box view is not null when creating this animation, it will not change.
// So checking it once above is enough.
mFakeSearchBoxShrinkAnimation = (appbarLayout, headerVerticalOffset)
-> updateFakeSearchBoxShrinkAnimation(headerVerticalOffset, fakeSearchBoxHeight,
toolbarContainerTopMargin, fakeSearchBoxView);
}
mHeaderView.addOnOffsetChangedListener(mFakeSearchBoxShrinkAnimation);
}
/** Remove the fake box shrink animation. */
void removeFakeSearchBoxShrinkAnimation() {
if (mHeaderView != null) {
mHeaderView.removeOnOffsetChangedListener(mFakeSearchBoxShrinkAnimation);
}
}
/**
* When the start surface toolbar is about to be scrolled out of the screen and the fake search
* box is almost at the screen top, start to reduce its height to make it finally the same as
* toolbar container view's height. This makes fake search box exactly overlap the toolbar
* container view and makes the transition smooth.
*
* <p>This function should be called together with
* StartSurfaceToolbarMediator#updateTranslationY, which scroll up the start surface toolbar
* together with the header.
*
* @param headerOffset The current offset of the header.
* @param originalFakeSearchBoxHeight The height of fake search box.
* @param toolbarContainerTopMargin The top margin of toolbar container view.
* @param fakeSearchBox The fake search box in start surface homepage.
*/
private void updateFakeSearchBoxShrinkAnimation(int headerOffset,
int originalFakeSearchBoxHeight, int toolbarContainerTopMargin, View fakeSearchBox) {
// When the header is scrolled up by fake search box height or so, reduce the fake search
// box height.
int reduceHeight = MathUtils.clamp(
-headerOffset - originalFakeSearchBoxHeight, 0, toolbarContainerTopMargin);
ViewGroup.LayoutParams layoutParams = fakeSearchBox.getLayoutParams();
if (layoutParams.height == originalFakeSearchBoxHeight - reduceHeight) {
return;
}
layoutParams.height = originalFakeSearchBoxHeight - reduceHeight;
// Update the top margin of the fake search box.
ViewGroup.MarginLayoutParams marginLayoutParams =
(ViewGroup.MarginLayoutParams) fakeSearchBox.getLayoutParams();
marginLayoutParams.setMargins(marginLayoutParams.leftMargin, reduceHeight,
marginLayoutParams.rightMargin, marginLayoutParams.bottomMargin);
fakeSearchBox.setLayoutParams(layoutParams);
}
} }
...@@ -2141,6 +2141,10 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent ...@@ -2141,6 +2141,10 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
mAppIndexingUtil = null; mAppIndexingUtil = null;
} }
if (mStartSurfaceSupplier.get() != null) {
mStartSurfaceSupplier.get().destroy();
}
IncognitoTabHostRegistry.getInstance().unregister(mIncognitoTabHost); IncognitoTabHostRegistry.getInstance().unregister(mIncognitoTabHost);
TabObscuringHandler tabObscuringHandler = getTabObscuringHandler(); TabObscuringHandler tabObscuringHandler = getTabObscuringHandler();
......
...@@ -809,6 +809,13 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve ...@@ -809,6 +809,13 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
@Override @Override
public boolean isLocationBarShown() { public boolean isLocationBarShown() {
// Without this check, ToolbarPhone#computeVisualState may return
// VisualState.NEW_TAB_NORMAL even if it's in start surface homepage, which leads
// ToolbarPhone#getToolbarColorForVisualState to return transparent color.
if (StartSurfaceConfiguration.isStartSurfaceEnabled()
&& mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE) {
return false;
}
NewTabPage ntp = getNewTabPageForCurrentTab(); NewTabPage ntp = getNewTabPageForCurrentTab();
return ntp != null && ntp.isLocationBarShownInNTP(); return ntp != null && ntp.isLocationBarShownInNTP();
} }
......
...@@ -553,7 +553,7 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC ...@@ -553,7 +553,7 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
* @return The background drawable for the toolbar view. * @return The background drawable for the toolbar view.
*/ */
@VisibleForTesting @VisibleForTesting
ColorDrawable getBackgroundDrawable() { public ColorDrawable getBackgroundDrawable() {
return mToolbarBackground; return mToolbarBackground;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment