Commit b8e41424 authored by gogerald's avatar gogerald Committed by Commit Bot

[StartSurface] Update Tab carousel header in single pane start surface

Before:
https://drive.google.com/file/d/1A89i4BC32cSpzCuhW2jmhQR0OQKa73WO/view?usp=sharing
https://drive.google.com/file/d/16jQU90GJZxdKXhG8Vhdzp4U1tELQtJ62/view?usp=sharing

After:
https://drive.google.com/file/d/17CHZtgW-ygZ7lFcHftGOkjBuWeuwu6Ds/view?usp=sharing
https://drive.google.com/file/d/1X8T4OE9e4MlX0CmA7tXKWuc6s6LwbiYI/view?usp=sharing

It visually aligns the first Tab in the Tabs carousel with the explore surface cards.
It also makes the Tabs carousel use the entire device width when scroll is needed.

Screenshots confirmed by UX designer.

Bug: 996381
Change-Id: Ic9d65b5eac6f771140b44f0562e3e1e9452f3cbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768865
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Auto-Submit: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690842}
parent 9a104735
...@@ -7,7 +7,6 @@ package org.chromium.chrome.features.start_surface; ...@@ -7,7 +7,6 @@ package org.chromium.chrome.features.start_surface;
import android.app.Activity; import android.app.Activity;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.view.MotionEvent; import android.view.MotionEvent;
import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import com.google.android.libraries.feed.api.client.stream.Stream; import com.google.android.libraries.feed.api.client.stream.Stream;
...@@ -24,8 +23,6 @@ import org.chromium.ui.modelutil.PropertyModelChangeProcessor; ...@@ -24,8 +23,6 @@ import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
/** The coordinator to control the explore surface. */ /** The coordinator to control the explore surface. */
class ExploreSurfaceCoordinator implements FeedSurfaceCoordinator.FeedSurfaceDelegate { class ExploreSurfaceCoordinator implements FeedSurfaceCoordinator.FeedSurfaceDelegate {
private final ChromeActivity mActivity; private final ChromeActivity mActivity;
@Nullable
private final View mHeaderView;
private final PropertyModelChangeProcessor mPropertyModelChangeProcessor; private final PropertyModelChangeProcessor mPropertyModelChangeProcessor;
private final FeedSurfaceCreator mFeedSurfaceCreator; private final FeedSurfaceCreator mFeedSurfaceCreator;
...@@ -44,12 +41,12 @@ class ExploreSurfaceCoordinator implements FeedSurfaceCoordinator.FeedSurfaceDel ...@@ -44,12 +41,12 @@ class ExploreSurfaceCoordinator implements FeedSurfaceCoordinator.FeedSurfaceDel
} }
ExploreSurfaceCoordinator(ChromeActivity activity, ViewGroup parentView, ExploreSurfaceCoordinator(ChromeActivity activity, ViewGroup parentView,
@Nullable View headerView, PropertyModel containerPropertyModel) { @Nullable ViewGroup headerContainerView, PropertyModel containerPropertyModel) {
mActivity = activity; mActivity = activity;
mHeaderView = headerView;
mPropertyModelChangeProcessor = PropertyModelChangeProcessor.create( mPropertyModelChangeProcessor = PropertyModelChangeProcessor.create(containerPropertyModel,
containerPropertyModel, parentView, ExploreSurfaceViewBinder::bind); new ExploreSurfaceViewBinder.ViewHolder(parentView, headerContainerView),
ExploreSurfaceViewBinder::bind);
mFeedSurfaceCreator = new FeedSurfaceCreator() { mFeedSurfaceCreator = new FeedSurfaceCreator() {
@Override @Override
public FeedSurfaceCoordinator createFeedSurfaceCoordinator(boolean isIncognito) { public FeedSurfaceCoordinator createFeedSurfaceCoordinator(boolean isIncognito) {
...@@ -91,7 +88,7 @@ class ExploreSurfaceCoordinator implements FeedSurfaceCoordinator.FeedSurfaceDel ...@@ -91,7 +88,7 @@ class ExploreSurfaceCoordinator implements FeedSurfaceCoordinator.FeedSurfaceDel
: Profile.getLastUsedProfile()), : Profile.getLastUsedProfile()),
FeedProcessScopeFactory.getFeedLoggingBridge()); FeedProcessScopeFactory.getFeedLoggingBridge());
return new FeedSurfaceCoordinator( return new FeedSurfaceCoordinator(
mActivity, null, null, mHeaderView, exploreSurfaceActionHandler, isIncognito, this); mActivity, null, null, null, exploreSurfaceActionHandler, isIncognito, this);
// TODO(crbug.com/982018): Customize surface background for incognito and dark mode. // TODO(crbug.com/982018): Customize surface background for incognito and dark mode.
// TODO(crbug.com/982018): Hide signin promo UI in incognito mode. // TODO(crbug.com/982018): Hide signin promo UI in incognito mode.
} }
......
...@@ -10,6 +10,7 @@ import static org.chromium.chrome.features.start_surface.StartSurfaceProperties. ...@@ -10,6 +10,7 @@ import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.
import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.IS_SHOWING_OVERVIEW; import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.IS_SHOWING_OVERVIEW;
import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.TOP_BAR_HEIGHT; import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.TOP_BAR_HEIGHT;
import android.support.annotation.Nullable;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.FrameLayout; import android.widget.FrameLayout;
...@@ -21,7 +22,21 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -21,7 +22,21 @@ import org.chromium.ui.modelutil.PropertyModel;
/** Binder for the explore surface. */ /** Binder for the explore surface. */
class ExploreSurfaceViewBinder { class ExploreSurfaceViewBinder {
public static void bind(PropertyModel model, ViewGroup view, PropertyKey propertyKey) { /**
* The view holder holds the parent view and the header container view.
*/
public static class ViewHolder {
public final ViewGroup parentView;
@Nullable
public final ViewGroup headerContainerView;
ViewHolder(ViewGroup parentView, @Nullable ViewGroup headerContainerView) {
this.parentView = parentView;
this.headerContainerView = headerContainerView;
}
}
public static void bind(PropertyModel model, ViewHolder view, PropertyKey propertyKey) {
if (propertyKey == IS_EXPLORE_SURFACE_VISIBLE) { if (propertyKey == IS_EXPLORE_SURFACE_VISIBLE) {
setVisibility(view, model, model.get(IS_EXPLORE_SURFACE_VISIBLE)); setVisibility(view, model, model.get(IS_EXPLORE_SURFACE_VISIBLE));
} else if (propertyKey == IS_SHOWING_OVERVIEW) { } else if (propertyKey == IS_SHOWING_OVERVIEW) {
...@@ -36,17 +51,44 @@ class ExploreSurfaceViewBinder { ...@@ -36,17 +51,44 @@ class ExploreSurfaceViewBinder {
} }
} }
/**
* Set the explore surface visibility.
* Note that if the {@link ViewHolder.headerContainerView} is not null, the feed surface view is
* added to the {@link ViewHolder.headerContainerView}, then the {@link
* ViewHolder.headerContainerView} is added to the {@link ViewHolder.parentView}. This is for
* the alignment between {@link ViewHolder.headerContainerView} and the feed surface view to
* avoid another level of view hiearachy. If the {@link ViewHolder.headerContainerView} is null,
* then the feed surface view is added to the {@link ViewHolder.parentView} directly.
* @param viewHolder The view holder holds the parent and possible the header container view.
* @param model The property model.
* @param isShowing Whether set the surface to visible or not.
*/
private static void setVisibility( private static void setVisibility(
ViewGroup containerView, PropertyModel model, boolean isShowing) { ViewHolder viewHolder, PropertyModel model, boolean isShowing) {
if (model.get(FEED_SURFACE_COORDINATOR) == null) return; if (model.get(FEED_SURFACE_COORDINATOR) == null) return;
if (viewHolder.headerContainerView != null) {
if (viewHolder.headerContainerView.getParent() == null) {
FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams(
LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT);
layoutParams.bottomMargin = model.get(BOTTOM_BAR_HEIGHT);
layoutParams.topMargin = model.get(TOP_BAR_HEIGHT);
viewHolder.parentView.addView(viewHolder.headerContainerView, layoutParams);
}
viewHolder.headerContainerView.setVisibility(isShowing ? View.VISIBLE : View.GONE);
}
View feedSurfaceView = model.get(FEED_SURFACE_COORDINATOR).getView(); View feedSurfaceView = model.get(FEED_SURFACE_COORDINATOR).getView();
if (isShowing) { if (isShowing) {
FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams( if (viewHolder.headerContainerView == null) {
LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT); FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams(
layoutParams.bottomMargin = model.get(BOTTOM_BAR_HEIGHT); LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT);
layoutParams.topMargin = model.get(TOP_BAR_HEIGHT); layoutParams.bottomMargin = model.get(BOTTOM_BAR_HEIGHT);
containerView.addView(feedSurfaceView, layoutParams); layoutParams.topMargin = model.get(TOP_BAR_HEIGHT);
viewHolder.parentView.addView(feedSurfaceView, layoutParams);
} else {
viewHolder.headerContainerView.addView(feedSurfaceView);
}
} else { } else {
UiUtils.removeViewFromParent(feedSurfaceView); UiUtils.removeViewFromParent(feedSurfaceView);
} }
......
...@@ -184,7 +184,7 @@ public class StartSurfaceCoordinator implements StartSurface { ...@@ -184,7 +184,7 @@ public class StartSurfaceCoordinator implements StartSurface {
// The tasks surface is added to the explore surface in the single pane mode below. // The tasks surface is added to the explore surface in the single pane mode below.
if (mSurfaceMode != SurfaceMode.SINGLE_PANE) { if (mSurfaceMode != SurfaceMode.SINGLE_PANE) {
mActivity.getCompositorViewHolder().addView(mTasksSurface.getView()); mActivity.getCompositorViewHolder().addView(mTasksSurface.getContainerView());
} }
// There is nothing else to do for SurfaceMode.TASKS_ONLY for now. // There is nothing else to do for SurfaceMode.TASKS_ONLY for now.
...@@ -209,10 +209,10 @@ public class StartSurfaceCoordinator implements StartSurface { ...@@ -209,10 +209,10 @@ public class StartSurfaceCoordinator implements StartSurface {
// the explore surface. Remove it after deciding on where to put the omnibox. // the explore surface. Remove it after deciding on where to put the omnibox.
ViewGroup exploreSurfaceContainer = ViewGroup exploreSurfaceContainer =
(ViewGroup) mActivity.getCompositorViewHolder().getParent(); (ViewGroup) mActivity.getCompositorViewHolder().getParent();
mExploreSurfaceCoordinator = mExploreSurfaceCoordinator = new ExploreSurfaceCoordinator(mActivity,
new ExploreSurfaceCoordinator(mActivity, exploreSurfaceContainer, exploreSurfaceContainer,
mSurfaceMode == SurfaceMode.SINGLE_PANE ? mTasksSurface.getView() : null, mSurfaceMode == SurfaceMode.SINGLE_PANE ? mTasksSurface.getContainerView() : null,
mPropertyModel); mPropertyModel);
} }
private void createAndSetBottomBar() { private void createAndSetBottomBar() {
...@@ -235,7 +235,7 @@ public class StartSurfaceCoordinator implements StartSurface { ...@@ -235,7 +235,7 @@ public class StartSurfaceCoordinator implements StartSurface {
mSecondaryTasksSurfacePropertyModel = new PropertyModel(TasksSurfaceProperties.ALL_KEYS); mSecondaryTasksSurfacePropertyModel = new PropertyModel(TasksSurfaceProperties.ALL_KEYS);
mSecondaryTasksSurface = TabManagementModuleProvider.getDelegate().createTasksSurface( mSecondaryTasksSurface = TabManagementModuleProvider.getDelegate().createTasksSurface(
mActivity, false, mSecondaryTasksSurfacePropertyModel); mActivity, false, mSecondaryTasksSurfacePropertyModel);
mActivity.getCompositorViewHolder().addView(mSecondaryTasksSurface.getView()); mActivity.getCompositorViewHolder().addView(mSecondaryTasksSurface.getContainerView());
if (mOnTabSelectingListener != null) { if (mOnTabSelectingListener != null) {
mSecondaryTasksSurface.setOnTabSelectingListener(mOnTabSelectingListener); mSecondaryTasksSurface.setOnTabSelectingListener(mOnTabSelectingListener);
mOnTabSelectingListener = null; mOnTabSelectingListener = null;
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:visibility="gone" android:visibility="gone"
android:background="@color/modern_primary_color"
android:orientation="horizontal"> android:orientation="horizontal">
<TextView <TextView
android:layout_width="0dp" android:layout_width="0dp"
...@@ -23,7 +24,7 @@ ...@@ -23,7 +24,7 @@
android:layout_gravity="center_vertical" android:layout_gravity="center_vertical"
android:paddingTop="8dp" android:paddingTop="8dp"
android:paddingBottom="8dp" android:paddingBottom="8dp"
android:paddingStart="16dp" android:paddingStart="24dp"
android:paddingEnd="16dp" android:paddingEnd="16dp"
android:textAlignment="viewStart" android:textAlignment="viewStart"
android:textAppearance="@style/TextAppearance.BlackCaption" android:textAppearance="@style/TextAppearance.BlackCaption"
......
...@@ -30,4 +30,5 @@ ...@@ -30,4 +30,5 @@
<dimen name="selection_tab_grid_toggle_button_inset">14dp</dimen> <dimen name="selection_tab_grid_toggle_button_inset">14dp</dimen>
<dimen name="tab_carousel_height">192dp</dimen> <dimen name="tab_carousel_height">192dp</dimen>
<dimen name="tab_carousel_card_width">168dp</dimen> <dimen name="tab_carousel_card_width">168dp</dimen>
<dimen name="tab_carousel_start_margin">-3dp</dimen>
</resources> </resources>
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
package org.chromium.chrome.browser.tasks; package org.chromium.chrome.browser.tasks;
import android.view.View; import android.view.ViewGroup;
import org.chromium.chrome.browser.compositor.layouts.Layout; import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.tasks.tab_management.TabSwitcher; import org.chromium.chrome.browser.tasks.tab_management.TabSwitcher;
...@@ -32,8 +32,8 @@ public interface TasksSurface { ...@@ -32,8 +32,8 @@ public interface TasksSurface {
TabSwitcher.TabListDelegate getTabListDelegate(); TabSwitcher.TabListDelegate getTabListDelegate();
/** /**
* Get the {@link View} of the surface. * Get the container {@link ViewGroup} of the surface.
* @return The surface view. * @return The surface's container {@link ViewGroup}.
*/ */
View getView(); ViewGroup getContainerView();
} }
...@@ -7,7 +7,7 @@ package org.chromium.chrome.browser.tasks; ...@@ -7,7 +7,7 @@ package org.chromium.chrome.browser.tasks;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL; import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.View; import android.view.ViewGroup;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.tasks.tab_management.TabManagementModuleProvider; import org.chromium.chrome.browser.tasks.tab_management.TabManagementModuleProvider;
...@@ -56,7 +56,7 @@ public class TasksSurfaceCoordinator implements TasksSurface { ...@@ -56,7 +56,7 @@ public class TasksSurfaceCoordinator implements TasksSurface {
} }
@Override @Override
public View getView() { public ViewGroup getContainerView() {
return mView; return mView;
} }
} }
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.tasks; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.tasks;
import android.content.Context; import android.content.Context;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.support.v4.view.MarginLayoutParamsCompat;
import android.util.AttributeSet; import android.util.AttributeSet;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
...@@ -16,11 +17,13 @@ import org.chromium.chrome.tab_ui.R; ...@@ -16,11 +17,13 @@ import org.chromium.chrome.tab_ui.R;
// The view of the tasks surface. // The view of the tasks surface.
class TasksView extends LinearLayout { class TasksView extends LinearLayout {
private final Context mContext;
private FrameLayout mTabSwitcherContainer; private FrameLayout mTabSwitcherContainer;
/** Default constructor needed to inflate via XML. */ /** Default constructor needed to inflate via XML. */
public TasksView(Context context, AttributeSet attrs) { public TasksView(Context context, AttributeSet attrs) {
super(context, attrs); super(context, attrs);
mContext = context;
} }
@Override @Override
...@@ -37,12 +40,17 @@ class TasksView extends LinearLayout { ...@@ -37,12 +40,17 @@ class TasksView extends LinearLayout {
void setIsTabCarousel(boolean isTabCarousel) { void setIsTabCarousel(boolean isTabCarousel) {
if (isTabCarousel) { if (isTabCarousel) {
// TODO(crbug.com/982018): Change view according to incognito and dark mode. // TODO(crbug.com/982018): Change view according to incognito and dark mode.
setLayoutParams(new LinearLayout.LayoutParams(LinearLayout.LayoutParams.MATCH_PARENT,
LinearLayout.LayoutParams.WRAP_CONTENT));
findViewById(R.id.tab_switcher_title).setVisibility(View.VISIBLE); findViewById(R.id.tab_switcher_title).setVisibility(View.VISIBLE);
mTabSwitcherContainer.setLayoutParams(
new LinearLayout.LayoutParams(LinearLayout.LayoutParams.MATCH_PARENT, // Add negative margin to start so as to reduce the first Tab card's visual distance to
LinearLayout.LayoutParams.WRAP_CONTENT)); // the start edge to ~16dp.
// TODO(crbug.com/982018): Add test to guard the visual expectation.
LinearLayout.LayoutParams layoutParams = new LinearLayout.LayoutParams(
LinearLayout.LayoutParams.MATCH_PARENT, LinearLayout.LayoutParams.WRAP_CONTENT);
MarginLayoutParamsCompat.setMarginStart(layoutParams,
mContext.getResources().getDimensionPixelSize(
R.dimen.tab_carousel_start_margin));
mTabSwitcherContainer.setLayoutParams(layoutParams);
} }
} }
......
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