Commit db177501 authored by Matt Jones's avatar Matt Jones Committed by Commit Bot

Add method to BottomSheetContent to allow custom peek height

This patch allows users of the BottomSheetContent api to choose the
height that their feature peeks at. Supplementing this change are
a number of height modes that the new getPeekHeight can return; namely
DEFAULT and DISABLED. These changes roll two existing methods into
a single, generic one.

All users of the api have been updated and tests have been added
for the new functionality.

Bug: 986310, 968246, 1012231, 1006029
Change-Id: Ia4b4c2c2839909c3c8f9da39b1b35283dd47eb9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1832603
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarJinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705149}
parent b765774f
...@@ -84,11 +84,6 @@ class AssistantBottomSheetContent implements BottomSheet.BottomSheetContent { ...@@ -84,11 +84,6 @@ class AssistantBottomSheetContent implements BottomSheet.BottomSheetContent {
return false; return false;
} }
@Override
public boolean isPeekStateEnabled() {
return true;
}
@Override @Override
public boolean wrapContentEnabled() { public boolean wrapContentEnabled() {
return true; return true;
......
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.tasks.tab_management; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.tasks.tab_management;
import android.view.View; import android.view.View;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.ContentPriority; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.ContentPriority;
import org.chromium.chrome.tab_ui.R; import org.chromium.chrome.tab_ui.R;
...@@ -54,8 +55,8 @@ public class TabGridSheetContent implements BottomSheetContent { ...@@ -54,8 +55,8 @@ public class TabGridSheetContent implements BottomSheetContent {
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
return false; return BottomSheet.HeightMode.DISABLED;
} }
@Override @Override
......
...@@ -6,8 +6,7 @@ ...@@ -6,8 +6,7 @@
xmlns:android="http://schemas.android.com/apk/res/android" xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginTop="8dp" android:layout_marginTop="8dp">
android:paddingBottom="@dimen/navigation_sheet_toolbar_bottom_padding">
<ImageView <ImageView
android:id="@+id/drag_handlebar" android:id="@+id/drag_handlebar"
android:layout_width="wrap_content" android:layout_width="wrap_content"
......
...@@ -676,10 +676,10 @@ ...@@ -676,10 +676,10 @@
<dimen name="navigation_bubble_text_bottom_padding">5dp</dimen> <dimen name="navigation_bubble_text_bottom_padding">5dp</dimen>
<dimen name="navigation_bubble_text_start_padding">2dp</dimen> <dimen name="navigation_bubble_text_start_padding">2dp</dimen>
<dimen name="navigation_bubble_text_end_padding">8dp</dimen> <dimen name="navigation_bubble_text_end_padding">8dp</dimen>
<dimen name="navigation_sheet_toolbar_bottom_padding">44dp</dimen>
<dimen name="navigation_sheet_content_top_padding">18dp</dimen> <dimen name="navigation_sheet_content_top_padding">18dp</dimen>
<dimen name="navigation_sheet_content_bottom_padding">4dp</dimen> <dimen name="navigation_sheet_content_bottom_padding">4dp</dimen>
<dimen name="navigation_sheet_content_wrap_padding">12dp</dimen> <dimen name="navigation_sheet_content_wrap_padding">12dp</dimen>
<dimen name="navigation_sheet_peek_height">64dp</dimen>
<!-- ChromeTextInputLayout dimensions --> <!-- ChromeTextInputLayout dimensions -->
<dimen name="text_input_layout_padding_start">3dp</dimen> <dimen name="text_input_layout_padding_start">3dp</dimen>
......
...@@ -34,6 +34,8 @@ import org.chromium.ui.base.ActivityWindowAndroid; ...@@ -34,6 +34,8 @@ import org.chromium.ui.base.ActivityWindowAndroid;
* Represents ephemeral tab content and the toolbar, which can be included inside the bottom sheet. * Represents ephemeral tab content and the toolbar, which can be included inside the bottom sheet.
*/ */
public class EphemeralTabSheetContent implements BottomSheet.BottomSheetContent { public class EphemeralTabSheetContent implements BottomSheet.BottomSheetContent {
private static final float PEEK_TOOLBAR_HEIGHT_MULTIPLE = 2.f;
private final Context mContext; private final Context mContext;
private final Runnable mOpenNewTabCallback; private final Runnable mOpenNewTabCallback;
private final Runnable mToolbarClickCallback; private final Runnable mToolbarClickCallback;
...@@ -192,8 +194,10 @@ public class EphemeralTabSheetContent implements BottomSheet.BottomSheetContent ...@@ -192,8 +194,10 @@ public class EphemeralTabSheetContent implements BottomSheet.BottomSheetContent
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
return true; int toolbarHeight =
mContext.getResources().getDimensionPixelSize(R.dimen.toolbar_height_no_shadow);
return (int) (toolbarHeight * PEEK_TOOLBAR_HEIGHT_MULTIPLE);
} }
@Override @Override
......
...@@ -293,11 +293,13 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -293,11 +293,13 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
// Makes peek state as 'not present' when bottom sheet is in expanded state (i.e. animating // Makes peek state as 'not present' when bottom sheet is in expanded state (i.e. animating
// from expanded to close state). It avoids the sheet animating in two distinct steps, which // from expanded to close state). It avoids the sheet animating in two distinct steps, which
// looks awkward. // looks awkward.
return !mBottomSheetController.get().getBottomSheet().isSheetOpen(); return !mBottomSheetController.get().getBottomSheet().isSheetOpen()
? getSizePx(mParentView.getContext(), R.dimen.navigation_sheet_peek_height)
: BottomSheet.HeightMode.DISABLED;
} }
@Override @Override
......
...@@ -52,8 +52,8 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetCon ...@@ -52,8 +52,8 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetCon
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
return false; return BottomSheet.HeightMode.DISABLED;
} }
@Override @Override
......
...@@ -15,6 +15,7 @@ import android.widget.TextView; ...@@ -15,6 +15,7 @@ import android.widget.TextView;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.HeightMode;
/** Microtransaction UI. */ /** Microtransaction UI. */
/* package */ class MicrotransactionView implements BottomSheetContent { /* package */ class MicrotransactionView implements BottomSheetContent {
...@@ -113,8 +114,8 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetCon ...@@ -113,8 +114,8 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetCon
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
return mIsPeekStateEnabled; return mIsPeekStateEnabled ? HeightMode.DEFAULT : HeightMode.DISABLED;
} }
@Override @Override
......
...@@ -105,9 +105,9 @@ public class DevicePickerBottomSheetContent implements BottomSheetContent, OnIte ...@@ -105,9 +105,9 @@ public class DevicePickerBottomSheetContent implements BottomSheetContent, OnIte
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
// Return false to ensure that the entire bottom sheet is shown. // Return false to ensure that the entire bottom sheet is shown.
return false; return BottomSheet.HeightMode.DISABLED;
} }
@Override @Override
......
...@@ -142,10 +142,7 @@ public class BottomSheetController implements Destroyable { ...@@ -142,10 +142,7 @@ public class BottomSheetController implements Destroyable {
public void onScrimClick() { public void onScrimClick() {
if (!mBottomSheet.isSheetOpen()) return; if (!mBottomSheet.isSheetOpen()) return;
mBottomSheet.setSheetState( mBottomSheet.setSheetState(
mBottomSheet.getCurrentSheetContent().isPeekStateEnabled() mBottomSheet.getMinSwipableSheetState(), true, StateChangeReason.TAP_SCRIM);
? BottomSheet.SheetState.PEEK
: BottomSheet.SheetState.HIDDEN,
true, StateChangeReason.TAP_SCRIM);
} }
@Override @Override
...@@ -223,7 +220,7 @@ public class BottomSheetController implements Destroyable { ...@@ -223,7 +220,7 @@ public class BottomSheetController implements Destroyable {
mIsSuppressed = false; mIsSuppressed = false;
if (mBottomSheet.getCurrentSheetContent() != null) { if (mBottomSheet.getCurrentSheetContent() != null) {
mBottomSheet.setSheetState(BottomSheet.SheetState.PEEK, true); mBottomSheet.setSheetState(mBottomSheet.getOpeningState(), true);
} else { } else {
// In the event the previous content was hidden, try to show the next one. // In the event the previous content was hidden, try to show the next one.
showNextContent(true); showNextContent(true);
...@@ -322,7 +319,7 @@ public class BottomSheetController implements Destroyable { ...@@ -322,7 +319,7 @@ public class BottomSheetController implements Destroyable {
BottomSheetContent nextContent = mContentQueue.poll(); BottomSheetContent nextContent = mContentQueue.poll();
mBottomSheet.showContent(nextContent); mBottomSheet.showContent(nextContent);
mBottomSheet.setSheetState(BottomSheet.SheetState.PEEK, animate); mBottomSheet.setSheetState(mBottomSheet.getOpeningState(), animate);
} }
/** /**
......
...@@ -16,6 +16,7 @@ import org.junit.Rule; ...@@ -16,6 +16,7 @@ import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Restriction; import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.UrlUtils; import org.chromium.base.test.util.UrlUtils;
...@@ -33,7 +34,6 @@ import org.chromium.content_public.browser.test.mock.MockNavigationController; ...@@ -33,7 +34,6 @@ import org.chromium.content_public.browser.test.mock.MockNavigationController;
import org.chromium.content_public.browser.test.util.Criteria; import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper; import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.content_public.browser.test.util.TouchCommon;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem; import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.test.util.UiRestriction; import org.chromium.ui.test.util.UiRestriction;
...@@ -138,10 +138,11 @@ public class NavigationSheetTest { ...@@ -138,10 +138,11 @@ public class NavigationSheetTest {
NavigationSheetCoordinator sheet = (NavigationSheetCoordinator) showPopup(controller); NavigationSheetCoordinator sheet = (NavigationSheetCoordinator) showPopup(controller);
ListView listview = sheet.getContentView().findViewById(R.id.navigation_entries); ListView listview = sheet.getContentView().findViewById(R.id.navigation_entries);
CriteriaHelper.pollUiThread(sheet::isExpanded); CriteriaHelper.pollUiThread(() -> listview.getChildCount() >= 2);
Assert.assertEquals(INVALID_NAVIGATION_INDEX, controller.mNavigatedIndex); Assert.assertEquals(INVALID_NAVIGATION_INDEX, controller.mNavigatedIndex);
TouchCommon.singleClickView(listview.getChildAt(1)); ThreadUtils.runOnUiThreadBlocking(() -> listview.getChildAt(1).callOnClick());
CriteriaHelper.pollUiThread(sheet::isHidden); CriteriaHelper.pollUiThread(sheet::isHidden);
CriteriaHelper.pollUiThread( CriteriaHelper.pollUiThread(
Criteria.equals(NAVIGATION_INDEX_2, () -> controller.mNavigatedIndex)); Criteria.equals(NAVIGATION_INDEX_2, () -> controller.mNavigatedIndex));
......
...@@ -85,9 +85,9 @@ public class BottomSheetControllerTest { ...@@ -85,9 +85,9 @@ public class BottomSheetControllerTest {
mHighPriorityContent = new TestBottomSheetContent( mHighPriorityContent = new TestBottomSheetContent(
mActivityTestRule.getActivity(), ContentPriority.HIGH, false); mActivityTestRule.getActivity(), ContentPriority.HIGH, false);
mPeekableContent = new TestBottomSheetContent(mActivityTestRule.getActivity(), true); mPeekableContent = new TestBottomSheetContent(mActivityTestRule.getActivity());
mNonPeekableContent = mNonPeekableContent = new TestBottomSheetContent(mActivityTestRule.getActivity());
new TestBottomSheetContent(mActivityTestRule.getActivity(), false); mNonPeekableContent.setPeekHeight(BottomSheet.HeightMode.DISABLED);
}); });
} }
......
...@@ -30,20 +30,19 @@ public class TestBottomSheetContent implements BottomSheetContent { ...@@ -30,20 +30,19 @@ public class TestBottomSheetContent implements BottomSheetContent {
/** Whether this content is browser specific. */ /** Whether this content is browser specific. */
private boolean mHasCustomLifecycle; private boolean mHasCustomLifecycle;
/** Whether this content's peek state is enabled. */ /** The peek height of this content. */
private boolean mPeekStateEnabled; private int mPeekHeight;
/** /**
* @param context A context to inflate views with. * @param context A context to inflate views with.
* @param priority The content's priority. * @param priority The content's priority.
* @param hasCustomLifecycle Whether the content is browser specific. * @param hasCustomLifecycle Whether the content is browser specific.
* @param peekStateEnabled Whether the content's peek state is enabled.
*/ */
public TestBottomSheetContent(Context context, @ContentPriority int priority, public TestBottomSheetContent(
boolean hasCustomLifecycle, boolean peekStateEnabled) { Context context, @ContentPriority int priority, boolean hasCustomLifecycle) {
mPeekHeight = BottomSheet.HeightMode.DEFAULT;
mPriority = priority; mPriority = priority;
mHasCustomLifecycle = hasCustomLifecycle; mHasCustomLifecycle = hasCustomLifecycle;
mPeekStateEnabled = peekStateEnabled;
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mToolbarView = new View(context); mToolbarView = new View(context);
ViewGroup.LayoutParams params = ViewGroup.LayoutParams params =
...@@ -61,20 +60,9 @@ public class TestBottomSheetContent implements BottomSheetContent { ...@@ -61,20 +60,9 @@ public class TestBottomSheetContent implements BottomSheetContent {
/** /**
* @param context A context to inflate views with. * @param context A context to inflate views with.
* @param priority The content's priority.
* @param hasCustomLifecycle Whether the content is browser specific.
*/ */
public TestBottomSheetContent( public TestBottomSheetContent(Context context) {
Context context, @ContentPriority int priority, boolean hasCustomLifecycle) { this(/*TestBottomSheetContent(*/ context, ContentPriority.LOW, false);
this(context, priority, hasCustomLifecycle, true);
}
/**
* @param context A context to inflate views with.
* @param peekStateEnabled Whether the content's peek state is enabled.
*/
public TestBottomSheetContent(Context context, boolean peekStateEnabled) {
this(/*TestBottomSheetContent(*/ context, ContentPriority.LOW, false, peekStateEnabled);
} }
@Override @Override
...@@ -106,9 +94,13 @@ public class TestBottomSheetContent implements BottomSheetContent { ...@@ -106,9 +94,13 @@ public class TestBottomSheetContent implements BottomSheetContent {
return false; return false;
} }
public void setPeekHeight(int height) {
mPeekHeight = height;
}
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
return mPeekStateEnabled; return mPeekHeight;
} }
@Override @Override
......
...@@ -39,13 +39,6 @@ class TouchToFillView implements BottomSheet.BottomSheetContent { ...@@ -39,13 +39,6 @@ class TouchToFillView implements BottomSheet.BottomSheetContent {
mEventListener.onDismissed(); mEventListener.onDismissed();
mBottomSheetController.getBottomSheet().removeObserver(mBottomSheetObserver); mBottomSheetController.getBottomSheet().removeObserver(mBottomSheetObserver);
} }
@Override
public void onSheetFullyPeeked() {
super.onSheetFullyPeeked();
// Since isPeekStateEnabled doesn't seem to skip the Peek state, force-expand the sheet.
mBottomSheetController.expandSheet();
}
}; };
/** /**
...@@ -150,8 +143,8 @@ class TouchToFillView implements BottomSheet.BottomSheetContent { ...@@ -150,8 +143,8 @@ class TouchToFillView implements BottomSheet.BottomSheetContent {
} }
@Override @Override
public boolean isPeekStateEnabled() { public int getPeekHeight() {
return true; // For some reason, false isn't working properly. Extend it explicitly! return BottomSheet.HeightMode.DISABLED;
} }
@Override @Override
......
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