Commit 94e25227 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Chromium LUCI CQ

Allow controlling the bottom sheet position.

This change introduces a way for content to set arbitrary
bottom sheet offsets while the content is active. The offset
set that way stays as long as the sheet doesn't change state
or need to be refreshed (onLayoutChange)

This change also ignore onContentSizeChanged calls that report
no changes, as this would otherwise reset the sheet offset for
no reasons.

The immediate goal of this change is to allow controlling the
height of the Autofill Assistant bottom sheet to have it scroll
in sync with the browser.

Change-Id: I020f393a580df0e85921ec7dd9388b72944f566d
Bug: b/171792266
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599863
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842453}
parent 923cf5e5
......@@ -7,7 +7,10 @@ package org.chromium.chrome.browser.widget.bottomsheet;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.chromium.base.test.util.CriteriaHelper.pollUiThread;
import static org.chromium.chrome.browser.flags.ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE;
......@@ -20,6 +23,7 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.Callback;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Restriction;
......@@ -32,7 +36,9 @@ import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent.HeightM
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.SheetState;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.StateChangeReason;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetTestSupport;
import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
import org.chromium.ui.test.util.UiRestriction;
import java.util.concurrent.ExecutionException;
......@@ -246,6 +252,48 @@ public class BottomSheetTest {
mSheetController.getSheetState());
}
@Test
@MediumTest
public void testOffsetController() {
mLowPriorityContent.setContentControlsOffset(true);
BottomSheetObserver forbidStateChanges = new EmptyBottomSheetObserver() {
@Override
public void onSheetOpened(@StateChangeReason int reason) {
fail("onSheetOpened unexpected");
}
@Override
public void onSheetClosed(@StateChangeReason int reason) {
fail("onSheetClosed unexpected");
}
};
runOnUiThreadBlocking(() -> {
assertTrue(mSheetController.requestShowContent(mLowPriorityContent, false));
Callback<Integer> offsetController = mLowPriorityContent.getOffsetController();
assertNotNull(offsetController);
mSheetController.addObserver(forbidStateChanges);
int startOffset = mSheetController.getCurrentOffset();
int modifiedOffset = startOffset / 2;
offsetController.onResult(modifiedOffset);
assertEquals(modifiedOffset, mSheetController.getCurrentOffset());
offsetController.onResult(0);
assertEquals(0, mSheetController.getCurrentOffset());
offsetController.onResult(startOffset);
assertEquals(startOffset, mSheetController.getCurrentOffset());
mSheetController.removeObserver(forbidStateChanges);
mSheetController.hideContent(mLowPriorityContent, false);
assertNull(mLowPriorityContent.getOffsetController());
});
}
private void hideSheet() {
runOnUiThreadBlocking(() -> mTestSupport.setSheetState(SheetState.HIDDEN, false));
}
......
......@@ -12,6 +12,7 @@ import android.view.ViewGroup;
import androidx.annotation.Nullable;
import org.chromium.base.Callback;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
......@@ -54,6 +55,13 @@ public class TestBottomSheetContent implements BottomSheetContent {
/** Whether this content intercepts back button presses. */
private boolean mHandleBackPress;
/** Set to true to ask for an offset controller. */
private boolean mContentControlsOffset;
/** Current offset controller. */
@Nullable
private Callback<Integer> mOffsetController;
/**
* @param context A context to inflate views with.
* @param priority The content's priority.
......@@ -202,4 +210,22 @@ public class TestBottomSheetContent implements BottomSheetContent {
public int getSheetClosedAccessibilityStringId() {
return android.R.string.copy;
}
@Override
public boolean contentControlsOffset() {
return mContentControlsOffset;
}
@Override
public void setOffsetController(Callback<Integer> offsetController) {
mOffsetController = offsetController;
}
public Callback<Integer> getOffsetController() {
return mOffsetController;
}
public void setContentControlsOffset(boolean value) {
mContentControlsOffset = value;
}
}
......@@ -180,6 +180,12 @@ class BottomSheet extends FrameLayout
*/
private int mScrollableHeight;
/**
* The instance passed to the current content that is allowed to
* change the sheet offset.
*/
private Callback<Integer> mOffsetController;
@Override
public boolean shouldGestureMoveSheet(MotionEvent initialEvent, MotionEvent currentEvent) {
// If the sheet is scrolling off-screen or in the process of hiding, gestures should not
......@@ -526,6 +532,10 @@ class BottomSheet extends FrameLayout
if (mSheetContent != null) {
mSheetContent.setContentSizeListener(null);
mSheetContent.getContentView().removeOnLayoutChangeListener(this);
if (mOffsetController != null) {
mSheetContent.setOffsetController(null);
mOffsetController = null;
}
}
if (content != null && getParent() == null) {
......@@ -667,8 +677,21 @@ class BottomSheet extends FrameLayout
/**
* Sets the sheet's offset relative to the bottom of the screen.
* @param offset The offset that the sheet should be.
* @param reason The reason for the sheet offset to change to report to listeners.
*/
void setSheetOffsetFromBottom(float offset, @StateChangeReason int reason) {
setSheetOffsetFromBottom(offset, reason, /* reportOpenClosed= */ true);
}
/**
* Sets the sheet's offset relative to the bottom of the screen.
* @param offset The offset that the sheet should be.
* @param reason The reason for the sheet offset to change to report to listeners.
* @param reportOpenClosed {@code true} to allow reporting the sheet opened or closed as a
* result of this change. {@code reason} is never used when this is {@code false}.
*/
void setSheetOffsetFromBottom(
float offset, @StateChangeReason int reason, boolean reportOpenClosed) {
mCurrentOffsetPx = offset;
// The browser controls offset is added here so that the sheet's toolbar behaves like the
......@@ -679,28 +702,32 @@ class BottomSheet extends FrameLayout
setTranslationY(translationY);
// Do open/close computation based on the minimum allowed state by the sheet's content.
// Note that when transitioning from hidden to peek, even dismissable sheets may want
// to have a peek state.
@SheetState
int minSwipableState = getMinSwipableSheetState();
if (isPeekStateEnabled() && (!isSheetOpen() || mTargetState == SheetState.PEEK)) {
minSwipableState = SheetState.PEEK;
}
if (reportOpenClosed) {
// Do open/close computation based on the minimum allowed state by the sheet's content.
// Note that when transitioning from hidden to peek, even dismissable sheets may want
// to have a peek state.
@SheetState
int minSwipableState = getMinSwipableSheetState();
if (isPeekStateEnabled() && (!isSheetOpen() || mTargetState == SheetState.PEEK)) {
minSwipableState = SheetState.PEEK;
}
float minScrollableHeight = getSheetHeightForState(minSwipableState);
boolean isAtMinHeight = MathUtils.areFloatsEqual(getCurrentOffsetPx(), minScrollableHeight);
boolean heightLessThanPeek = getCurrentOffsetPx() < minScrollableHeight;
// Trigger the onSheetClosed event when the sheet is moving toward the hidden state if peek
// is disabled. This should be fine since touch is disabled when the sheet's target is
// hidden.
boolean triggerCloseWithHidden = !isPeekStateEnabled() && mTargetState == SheetState.HIDDEN;
if (isSheetOpen() && (heightLessThanPeek || isAtMinHeight || triggerCloseWithHidden)) {
onSheetClosed(reason);
} else if (!isSheetOpen() && mTargetState != SheetState.HIDDEN
&& getCurrentOffsetPx() > minScrollableHeight) {
onSheetOpened(reason);
float minScrollableHeight = getSheetHeightForState(minSwipableState);
boolean isAtMinHeight =
MathUtils.areFloatsEqual(getCurrentOffsetPx(), minScrollableHeight);
boolean heightLessThanPeek = getCurrentOffsetPx() < minScrollableHeight;
// Trigger the onSheetClosed event when the sheet is moving toward the hidden state if
// peek is disabled. This should be fine since touch is disabled when the sheet's target
// is hidden.
boolean triggerCloseWithHidden =
!isPeekStateEnabled() && mTargetState == SheetState.HIDDEN;
if (isSheetOpen() && (heightLessThanPeek || isAtMinHeight || triggerCloseWithHidden)) {
onSheetClosed(reason);
} else if (!isSheetOpen() && mTargetState != SheetState.HIDDEN
&& getCurrentOffsetPx() > minScrollableHeight) {
onSheetOpened(reason);
}
}
sendOffsetChangeEvents();
......@@ -1228,18 +1255,34 @@ class BottomSheet extends FrameLayout
protected void onSheetContentChanged(@Nullable final BottomSheetContent content) {
mSheetContent = content;
if (content != null && isFullHeightWrapContent()) {
// Listen for layout/size changes.
if (!content.setContentSizeListener(this::onContentSizeChanged)) {
content.getContentView().addOnLayoutChangeListener(this);
}
if (content != null) {
if (isFullHeightWrapContent()) {
// Listen for layout/size changes.
if (!content.setContentSizeListener(this::onContentSizeChanged)) {
content.getContentView().addOnLayoutChangeListener(this);
}
invalidateContentDesiredHeight();
ensureContentIsWrapped(/* animate= */ true);
invalidateContentDesiredHeight();
ensureContentIsWrapped(/* animate= */ true);
// HALF state is forbidden when wrapping the content.
if (mCurrentState == SheetState.HALF) {
setSheetState(SheetState.FULL, /* animate= */ true);
}
}
if (content.contentControlsOffset()) {
mOffsetController = new Callback<Integer>() {
@Override
public void onResult(Integer offsetPx) {
if (this != mOffsetController) return;
// HALF state is forbidden when wrapping the content.
if (mCurrentState == SheetState.HALF) {
setSheetState(SheetState.FULL, /* animate= */ true);
cancelAnimation();
setSheetOffsetFromBottom(
MathUtils.clamp(offsetPx, 0, (int) getMaxOffsetPx()),
StateChangeReason.NONE, /* reportOpenClosed=*/false);
}
};
content.setOffsetController(mOffsetController);
}
}
......@@ -1267,6 +1310,12 @@ class BottomSheet extends FrameLayout
*/
private void onContentSizeChanged(int width, int height, int oldWidth, int oldHeight) {
boolean heightChanged = mContentDesiredHeight != height;
boolean widthChanged = mContentWidth != width;
// onContentSizeChanged() is sometimes called when there's no size change, because of
// animations running in the content. Ignore these calls.
if (!heightChanged && !widthChanged) return;
mContentDesiredHeight = height;
mContentWidth = width;
......
......@@ -9,6 +9,8 @@ import android.view.View;
import androidx.annotation.IntDef;
import androidx.annotation.Nullable;
import org.chromium.base.Callback;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
......@@ -205,4 +207,23 @@ public interface BottomSheetContent {
* typically the name of your feature followed by 'closed'.
*/
int getSheetClosedAccessibilityStringId();
/**
* Return {@code true} if the content expects {@link #setOffsetController} to be called.
*
* This is an experimental feature. Use it at your own risks. TODO(b/177037825): Remove or
* cleanup.
*/
default boolean contentControlsOffset() {
return false;
}
/**
* Set or reset the set offset callback.
*
* The active content can use this callback to move the sheet to the given offset.
*
* Only called if {@link #contentControlsOffset} returns {@code true}.
*/
default void setOffsetController(@Nullable Callback<Integer> setOffset) {}
}
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