Skip to content

Commit

Permalink
[BottomSheet] Fix bottom sheets in EXPANDED state when the expanded h…
Browse files Browse the repository at this point in the history 
…eight is the same as the collapsed height

Resolves #2335

PiperOrigin-RevId: 435127325
  • Loading branch information
@drchen @dsn5ft
drchen authored and dsn5ft committed Mar 17, 2022
1 parent 380778f commit 493243e
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 14 deletions.
33 changes: 30 additions & 3 deletions lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java
View file
Expand Up @@ -277,6 +277,7 @@ void onLayout(@NonNull View bottomSheet) {}
boolean hideable;

private boolean skipCollapsed;
private boolean collapsible = true;

private boolean draggable = true;

Expand Down Expand Up @@ -552,6 +553,7 @@ public boolean onLayoutChild(
fitToContentsOffset = max(0, parentHeight - childHeight);
calculateHalfExpandedOffset();
calculateCollapsedOffset();
onOffsetsUpdated();

if (state == STATE_EXPANDED) {
ViewCompat.offsetTopAndBottom(child, getExpandedOffset());
Expand Down Expand Up @@ -725,10 +727,12 @@ public void onNestedPreScroll(
consumed[1] = dy;
ViewCompat.offsetTopAndBottom(child, -dy);
setStateInternal(STATE_DRAGGING);
} else {
} else if (!shouldSkipCollapsed()) {
consumed[1] = currentTop - collapsedOffset;
ViewCompat.offsetTopAndBottom(child, -consumed[1]);
setStateInternal(STATE_COLLAPSED);
} else {
// Do nothing, the bottom sheet should be fixed.
}
}
}
Expand Down Expand Up @@ -871,6 +875,7 @@ public void setFitToContents(boolean fitToContents) {
}
// Fix incorrect expanded settings depending on whether or not we are fitting sheet to contents.
setStateInternal((this.fitToContents && state == STATE_HALF_EXPANDED) ? STATE_EXPANDED : state);
onOffsetsUpdated();

updateAccessibilityActions();
}
Expand Down Expand Up @@ -966,6 +971,7 @@ public final void setPeekHeight(int peekHeight, boolean animate) {
private void updatePeekHeight(boolean animate) {
if (viewRef != null) {
calculateCollapsedOffset();
onOffsetsUpdated();
if (state == STATE_COLLAPSED) {
V view = viewRef.get();
if (view != null) {
Expand Down Expand Up @@ -1204,6 +1210,10 @@ public void setState(@StableState int state) {
Log.w(TAG, "Cannot set state: " + state);
return;
}
if (ensureExpandedStateIfNotCollapsible(state)) {
// setState() will be called again.
return;
}
final int finalState;
if (state == STATE_HALF_EXPANDED
&& fitToContents
Expand Down Expand Up @@ -1278,7 +1288,7 @@ void setStateInternal(@State int state) {
return;
}
this.state = state;
if (state == STATE_COLLAPSED
if ((collapsible && state == STATE_COLLAPSED)
|| state == STATE_EXPANDED
|| state == STATE_HALF_EXPANDED
|| (hideable && state == STATE_HIDDEN)) {
Expand Down Expand Up @@ -1352,6 +1362,19 @@ private void calculateCollapsedOffset() {
}
}

private void onOffsetsUpdated() {
collapsible = collapsedOffset != getExpandedOffset();
ensureExpandedStateIfNotCollapsible(state);
}

private boolean ensureExpandedStateIfNotCollapsible(@State int state) {
if (!collapsible && state == STATE_COLLAPSED) {
setState(STATE_EXPANDED);
return true;
}
return false;
}

private void calculateHalfExpandedOffset() {
this.halfExpandedOffset = (int) (parentHeight * (1 - halfExpandedRatio));
}
Expand Down Expand Up @@ -1385,7 +1408,7 @@ private void restoreOptionalState(@NonNull SavedState ss) {
}

boolean shouldHide(@NonNull View child, float yvel) {
if (skipCollapsed) {
if (shouldSkipCollapsed()) {
return true;
}
if (child.getTop() < collapsedOffset) {
Expand All @@ -1397,6 +1420,10 @@ boolean shouldHide(@NonNull View child, float yvel) {
return Math.abs(newTop - collapsedOffset) / (float) peek > HIDE_THRESHOLD;
}

private boolean shouldSkipCollapsed() {
return !collapsible || skipCollapsed;
}

@Nullable
@VisibleForTesting
View findScrollingChild(View view) {
Expand Down
60 changes: 49 additions & 11 deletions tests/javatests/com/google/android/material/bottomsheet/BottomSheetBehaviorTest.java
View file
Expand Up @@ -433,10 +433,7 @@ public void testSkipCollapsedFullyExpanded() throws Throwable {
testSkipCollapsed();
}

private void testSkipCollapsed_smallSwipe(int expectedState, float swipeViewHeightPercentage)
throws Throwable {
getBehavior().setSkipCollapsed(true);
checkSetState(BottomSheetBehavior.STATE_EXPANDED, ViewMatchers.isDisplayed());
private void testSkipCollapsed_smallSwipe(int expectedState, float swipeViewHeightPercentage) {
Espresso.onView(ViewMatchers.withId(R.id.bottom_sheet))
.perform(
DesignViewActions.withCustomConstraints(
Expand Down Expand Up @@ -476,16 +473,23 @@ private void testSkipCollapsed_smallSwipe(int expectedState, float swipeViewHeig
}
}

private void setSkipCollapsed() throws Throwable {
getBehavior().setSkipCollapsed(true);
checkSetState(BottomSheetBehavior.STATE_EXPANDED, ViewMatchers.isDisplayed());
}

@Test
@MediumTest
public void testSkipCollapsed_smallSwipe_remainsExpanded() throws Throwable {
setSkipCollapsed();
testSkipCollapsed_smallSwipe(
BottomSheetBehavior.STATE_EXPANDED, /* swipeViewHeightPercentage = */ 0.5f);
}

@Test
@MediumTest
public void testSkipCollapsedFullyExpanded_smallSwipe_remainsExpanded() throws Throwable {
setSkipCollapsed();
getBehavior().setFitToContents(false);
testSkipCollapsed_smallSwipe(
BottomSheetBehavior.STATE_HALF_EXPANDED, /* swipeViewHeightPercentage = */ 0.5f);
Expand All @@ -494,18 +498,41 @@ public void testSkipCollapsedFullyExpanded_smallSwipe_remainsExpanded() throws T
@Test
@MediumTest
public void testSkipCollapsed_smallSwipePastThreshold_getsHidden() throws Throwable {
setSkipCollapsed();
testSkipCollapsed_smallSwipe(
BottomSheetBehavior.STATE_HIDDEN, /* swipeViewHeightPercentage = */ 0.75f);
}

@Test
@MediumTest
public void testSkipCollapsedFullyExpanded_smallSwipePastThreshold_getsHidden() throws Throwable {
setSkipCollapsed();
getBehavior().setFitToContents(false);
testSkipCollapsed_smallSwipe(
BottomSheetBehavior.STATE_HIDDEN, /* swipeViewHeightPercentage = */ 0.75f);
}

private void makeBottomSheetNotCollapsible() {
// Set a peek height that equals to expanded height so it always stays EXPANDED.
getBehavior().setPeekHeight(5000);
}

@Test
@MediumTest
public void testNotCollapsible_smallSwipe_remainsExpanded() {
makeBottomSheetNotCollapsible();
testSkipCollapsed_smallSwipe(
BottomSheetBehavior.STATE_EXPANDED, /* swipeViewHeightPercentage = */ 0.5f);
}

@Test
@MediumTest
public void testNotCollapsible_smallSwipePastThreshold_getsHidden() {
makeBottomSheetNotCollapsible();
testSkipCollapsed_smallSwipe(
BottomSheetBehavior.STATE_HIDDEN, /* swipeViewHeightPercentage = */ 0.75f);
}

@Test
@MediumTest
public void testSwipeUpToExpand() {
Expand Down Expand Up @@ -935,16 +962,22 @@ public void testDynamicContent() throws Throwable {
@Test
@MediumTest
public void testExpandedPeekHeight() throws Throwable {
registerIdlingResourceCallback();
activityTestRule.runOnUiThread(
() -> {
// Make the peek height as tall as the bottom sheet.
BottomSheetBehavior<?> behavior = getBehavior();
behavior.setPeekHeight(getBottomSheet().getHeight());
assertThat(behavior.getState(), is(BottomSheetBehavior.STATE_COLLAPSED));
getBehavior().setPeekHeight(getBottomSheet().getHeight());
});
// Both of these will not animate the sheet , but the state should be changed.
Espresso.onView(ViewMatchers.withId(R.id.bottom_sheet))
.check(ViewAssertions.matches(ViewMatchers.isDisplayed()));
assertThat(getBehavior().getState(), is(BottomSheetBehavior.STATE_EXPANDED));
unregisterIdlingResourceCallback();
// Both of these will not animate the sheet, and the state should be fixed as EXPANDED.
checkSetState(BottomSheetBehavior.STATE_EXPANDED, ViewMatchers.isDisplayed());
checkSetState(BottomSheetBehavior.STATE_COLLAPSED, ViewMatchers.isDisplayed());
checkSetState(
BottomSheetBehavior.STATE_COLLAPSED,
BottomSheetBehavior.STATE_EXPANDED,
ViewMatchers.isDisplayed());
}

@Test
Expand All @@ -962,12 +995,17 @@ public void testFindScrollingChildEnabled() {
}

private void checkSetState(final int state, Matcher<View> matcher) throws Throwable {
checkSetState(state, state, matcher);
}

private void checkSetState(
final int stateToSet, final int stateToExpect, Matcher<View> matcher) throws Throwable {
registerIdlingResourceCallback();
try {
activityTestRule.runOnUiThread(() -> getBehavior().setState(state));
activityTestRule.runOnUiThread(() -> getBehavior().setState(stateToSet));
Espresso.onView(ViewMatchers.withId(R.id.bottom_sheet))
.check(ViewAssertions.matches(matcher));
assertThat(getBehavior().getState(), is(state));
assertThat(getBehavior().getState(), is(stateToExpect));
assertAccessibilityActions(getBehavior(), getBottomSheet());
} finally {
unregisterIdlingResourceCallback();
Expand Down
50 changes: 50 additions & 0 deletions tests/javatests/com/google/android/material/bottomsheet/BottomSheetDialogTest.java
View file
Expand Up @@ -188,6 +188,9 @@ public void testHideThenShow() throws Throwable {
final DialogInterface.OnCancelListener onCancelListener =
mock(DialogInterface.OnCancelListener.class);
showDialog();
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
.perform(setShortPeekHeight())
.check(ViewAssertions.matches(ViewMatchers.isDisplayed()));
dialog.setOnCancelListener(onCancelListener);
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
.perform(setState(BottomSheetBehavior.STATE_HIDDEN));
Expand All @@ -205,6 +208,33 @@ public void testHideThenShow() throws Throwable {
.onStateChanged(any(View.class), eq(BottomSheetBehavior.STATE_COLLAPSED));
}

@Test
@MediumTest
public void testHideThenShowNotCollapsible() throws Throwable {
// Hide the bottom sheet and wait for the dialog to be canceled.
final DialogInterface.OnCancelListener onCancelListener =
mock(DialogInterface.OnCancelListener.class);
showDialog();
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
.perform(setTallPeekHeight())
.check(ViewAssertions.matches(ViewMatchers.isDisplayed()));
dialog.setOnCancelListener(onCancelListener);
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
.perform(setState(BottomSheetBehavior.STATE_HIDDEN));
verify(onCancelListener, timeout(3000)).onCancel(any(DialogInterface.class));
// Reshow the same dialog instance and wait for the bottom sheet to be collapsed.
final BottomSheetBehavior.BottomSheetCallback callback =
mock(BottomSheetBehavior.BottomSheetCallback.class);
BottomSheetBehavior.from(dialog.findViewById(R.id.design_bottom_sheet))
.addBottomSheetCallback(callback);
// Show the same dialog again.
activityTestRule.runOnUiThread(() -> dialog.show());
verify(callback, timeout(3000))
.onStateChanged(any(View.class), eq(BottomSheetBehavior.STATE_SETTLING));
verify(callback, timeout(3000))
.onStateChanged(any(View.class), eq(BottomSheetBehavior.STATE_EXPANDED));
}

private void showDialog() throws Throwable {
activityTestRule.runOnUiThread(
() -> {
Expand Down Expand Up @@ -239,6 +269,26 @@ public void perform(UiController uiController, View view) {
};
}

private static ViewAction setShortPeekHeight() {
return new ViewAction() {
@Override
public Matcher<View> getConstraints() {
return ViewMatchers.isDisplayed();
}

@Override
public String getDescription() {
return "set tall peek height";
}

@Override
public void perform(UiController uiController, View view) {
BottomSheetBehavior<?> behavior = BottomSheetBehavior.from(view);
behavior.setPeekHeight(view.getHeight() / 2);
}
};
}

private static ViewAction setState(@BottomSheetBehavior.State final int state) {
return new ViewAction() {
@Override
Expand Down

0 comments on commit 493243e

Please sign in to comment.