Skip to content

Commit 493243e

Browse files
drchendsn5ft
authored andcommitted
[BottomSheet] Fix bottom sheets in EXPANDED state when the expanded height is the same as the collapsed height
Resolves #2335 PiperOrigin-RevId: 435127325
1 parent 380778f commit 493243e

File tree

3 files changed

+129
-14
lines changed

3 files changed

+129
-14
lines changed

lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ void onLayout(@NonNull View bottomSheet) {}
277277
boolean hideable;
278278

279279
private boolean skipCollapsed;
280+
private boolean collapsible = true;
280281

281282
private boolean draggable = true;
282283

@@ -552,6 +553,7 @@ public boolean onLayoutChild(
552553
fitToContentsOffset = max(0, parentHeight - childHeight);
553554
calculateHalfExpandedOffset();
554555
calculateCollapsedOffset();
556+
onOffsetsUpdated();
555557

556558
if (state == STATE_EXPANDED) {
557559
ViewCompat.offsetTopAndBottom(child, getExpandedOffset());
@@ -725,10 +727,12 @@ public void onNestedPreScroll(
725727
consumed[1] = dy;
726728
ViewCompat.offsetTopAndBottom(child, -dy);
727729
setStateInternal(STATE_DRAGGING);
728-
} else {
730+
} else if (!shouldSkipCollapsed()) {
729731
consumed[1] = currentTop - collapsedOffset;
730732
ViewCompat.offsetTopAndBottom(child, -consumed[1]);
731733
setStateInternal(STATE_COLLAPSED);
734+
} else {
735+
// Do nothing, the bottom sheet should be fixed.
732736
}
733737
}
734738
}
@@ -871,6 +875,7 @@ public void setFitToContents(boolean fitToContents) {
871875
}
872876
// Fix incorrect expanded settings depending on whether or not we are fitting sheet to contents.
873877
setStateInternal((this.fitToContents && state == STATE_HALF_EXPANDED) ? STATE_EXPANDED : state);
878+
onOffsetsUpdated();
874879

875880
updateAccessibilityActions();
876881
}
@@ -966,6 +971,7 @@ public final void setPeekHeight(int peekHeight, boolean animate) {
966971
private void updatePeekHeight(boolean animate) {
967972
if (viewRef != null) {
968973
calculateCollapsedOffset();
974+
onOffsetsUpdated();
969975
if (state == STATE_COLLAPSED) {
970976
V view = viewRef.get();
971977
if (view != null) {
@@ -1204,6 +1210,10 @@ public void setState(@StableState int state) {
12041210
Log.w(TAG, "Cannot set state: " + state);
12051211
return;
12061212
}
1213+
if (ensureExpandedStateIfNotCollapsible(state)) {
1214+
// setState() will be called again.
1215+
return;
1216+
}
12071217
final int finalState;
12081218
if (state == STATE_HALF_EXPANDED
12091219
&& fitToContents
@@ -1278,7 +1288,7 @@ void setStateInternal(@State int state) {
12781288
return;
12791289
}
12801290
this.state = state;
1281-
if (state == STATE_COLLAPSED
1291+
if ((collapsible && state == STATE_COLLAPSED)
12821292
|| state == STATE_EXPANDED
12831293
|| state == STATE_HALF_EXPANDED
12841294
|| (hideable && state == STATE_HIDDEN)) {
@@ -1352,6 +1362,19 @@ private void calculateCollapsedOffset() {
13521362
}
13531363
}
13541364

1365+
private void onOffsetsUpdated() {
1366+
collapsible = collapsedOffset != getExpandedOffset();
1367+
ensureExpandedStateIfNotCollapsible(state);
1368+
}
1369+
1370+
private boolean ensureExpandedStateIfNotCollapsible(@State int state) {
1371+
if (!collapsible && state == STATE_COLLAPSED) {
1372+
setState(STATE_EXPANDED);
1373+
return true;
1374+
}
1375+
return false;
1376+
}
1377+
13551378
private void calculateHalfExpandedOffset() {
13561379
this.halfExpandedOffset = (int) (parentHeight * (1 - halfExpandedRatio));
13571380
}
@@ -1385,7 +1408,7 @@ private void restoreOptionalState(@NonNull SavedState ss) {
13851408
}
13861409

13871410
boolean shouldHide(@NonNull View child, float yvel) {
1388-
if (skipCollapsed) {
1411+
if (shouldSkipCollapsed()) {
13891412
return true;
13901413
}
13911414
if (child.getTop() < collapsedOffset) {
@@ -1397,6 +1420,10 @@ boolean shouldHide(@NonNull View child, float yvel) {
13971420
return Math.abs(newTop - collapsedOffset) / (float) peek > HIDE_THRESHOLD;
13981421
}
13991422

1423+
private boolean shouldSkipCollapsed() {
1424+
return !collapsible || skipCollapsed;
1425+
}
1426+
14001427
@Nullable
14011428
@VisibleForTesting
14021429
View findScrollingChild(View view) {

tests/javatests/com/google/android/material/bottomsheet/BottomSheetBehaviorTest.java

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -433,10 +433,7 @@ public void testSkipCollapsedFullyExpanded() throws Throwable {
433433
testSkipCollapsed();
434434
}
435435

436-
private void testSkipCollapsed_smallSwipe(int expectedState, float swipeViewHeightPercentage)
437-
throws Throwable {
438-
getBehavior().setSkipCollapsed(true);
439-
checkSetState(BottomSheetBehavior.STATE_EXPANDED, ViewMatchers.isDisplayed());
436+
private void testSkipCollapsed_smallSwipe(int expectedState, float swipeViewHeightPercentage) {
440437
Espresso.onView(ViewMatchers.withId(R.id.bottom_sheet))
441438
.perform(
442439
DesignViewActions.withCustomConstraints(
@@ -476,16 +473,23 @@ private void testSkipCollapsed_smallSwipe(int expectedState, float swipeViewHeig
476473
}
477474
}
478475

476+
private void setSkipCollapsed() throws Throwable {
477+
getBehavior().setSkipCollapsed(true);
478+
checkSetState(BottomSheetBehavior.STATE_EXPANDED, ViewMatchers.isDisplayed());
479+
}
480+
479481
@Test
480482
@MediumTest
481483
public void testSkipCollapsed_smallSwipe_remainsExpanded() throws Throwable {
484+
setSkipCollapsed();
482485
testSkipCollapsed_smallSwipe(
483486
BottomSheetBehavior.STATE_EXPANDED, /* swipeViewHeightPercentage = */ 0.5f);
484487
}
485488

486489
@Test
487490
@MediumTest
488491
public void testSkipCollapsedFullyExpanded_smallSwipe_remainsExpanded() throws Throwable {
492+
setSkipCollapsed();
489493
getBehavior().setFitToContents(false);
490494
testSkipCollapsed_smallSwipe(
491495
BottomSheetBehavior.STATE_HALF_EXPANDED, /* swipeViewHeightPercentage = */ 0.5f);
@@ -494,18 +498,41 @@ public void testSkipCollapsedFullyExpanded_smallSwipe_remainsExpanded() throws T
494498
@Test
495499
@MediumTest
496500
public void testSkipCollapsed_smallSwipePastThreshold_getsHidden() throws Throwable {
501+
setSkipCollapsed();
497502
testSkipCollapsed_smallSwipe(
498503
BottomSheetBehavior.STATE_HIDDEN, /* swipeViewHeightPercentage = */ 0.75f);
499504
}
500505

501506
@Test
502507
@MediumTest
503508
public void testSkipCollapsedFullyExpanded_smallSwipePastThreshold_getsHidden() throws Throwable {
509+
setSkipCollapsed();
504510
getBehavior().setFitToContents(false);
505511
testSkipCollapsed_smallSwipe(
506512
BottomSheetBehavior.STATE_HIDDEN, /* swipeViewHeightPercentage = */ 0.75f);
507513
}
508514

515+
private void makeBottomSheetNotCollapsible() {
516+
// Set a peek height that equals to expanded height so it always stays EXPANDED.
517+
getBehavior().setPeekHeight(5000);
518+
}
519+
520+
@Test
521+
@MediumTest
522+
public void testNotCollapsible_smallSwipe_remainsExpanded() {
523+
makeBottomSheetNotCollapsible();
524+
testSkipCollapsed_smallSwipe(
525+
BottomSheetBehavior.STATE_EXPANDED, /* swipeViewHeightPercentage = */ 0.5f);
526+
}
527+
528+
@Test
529+
@MediumTest
530+
public void testNotCollapsible_smallSwipePastThreshold_getsHidden() {
531+
makeBottomSheetNotCollapsible();
532+
testSkipCollapsed_smallSwipe(
533+
BottomSheetBehavior.STATE_HIDDEN, /* swipeViewHeightPercentage = */ 0.75f);
534+
}
535+
509536
@Test
510537
@MediumTest
511538
public void testSwipeUpToExpand() {
@@ -935,16 +962,22 @@ public void testDynamicContent() throws Throwable {
935962
@Test
936963
@MediumTest
937964
public void testExpandedPeekHeight() throws Throwable {
965+
registerIdlingResourceCallback();
938966
activityTestRule.runOnUiThread(
939967
() -> {
940968
// Make the peek height as tall as the bottom sheet.
941-
BottomSheetBehavior<?> behavior = getBehavior();
942-
behavior.setPeekHeight(getBottomSheet().getHeight());
943-
assertThat(behavior.getState(), is(BottomSheetBehavior.STATE_COLLAPSED));
969+
getBehavior().setPeekHeight(getBottomSheet().getHeight());
944970
});
945-
// Both of these will not animate the sheet , but the state should be changed.
971+
Espresso.onView(ViewMatchers.withId(R.id.bottom_sheet))
972+
.check(ViewAssertions.matches(ViewMatchers.isDisplayed()));
973+
assertThat(getBehavior().getState(), is(BottomSheetBehavior.STATE_EXPANDED));
974+
unregisterIdlingResourceCallback();
975+
// Both of these will not animate the sheet, and the state should be fixed as EXPANDED.
946976
checkSetState(BottomSheetBehavior.STATE_EXPANDED, ViewMatchers.isDisplayed());
947-
checkSetState(BottomSheetBehavior.STATE_COLLAPSED, ViewMatchers.isDisplayed());
977+
checkSetState(
978+
BottomSheetBehavior.STATE_COLLAPSED,
979+
BottomSheetBehavior.STATE_EXPANDED,
980+
ViewMatchers.isDisplayed());
948981
}
949982

950983
@Test
@@ -962,12 +995,17 @@ public void testFindScrollingChildEnabled() {
962995
}
963996

964997
private void checkSetState(final int state, Matcher<View> matcher) throws Throwable {
998+
checkSetState(state, state, matcher);
999+
}
1000+
1001+
private void checkSetState(
1002+
final int stateToSet, final int stateToExpect, Matcher<View> matcher) throws Throwable {
9651003
registerIdlingResourceCallback();
9661004
try {
967-
activityTestRule.runOnUiThread(() -> getBehavior().setState(state));
1005+
activityTestRule.runOnUiThread(() -> getBehavior().setState(stateToSet));
9681006
Espresso.onView(ViewMatchers.withId(R.id.bottom_sheet))
9691007
.check(ViewAssertions.matches(matcher));
970-
assertThat(getBehavior().getState(), is(state));
1008+
assertThat(getBehavior().getState(), is(stateToExpect));
9711009
assertAccessibilityActions(getBehavior(), getBottomSheet());
9721010
} finally {
9731011
unregisterIdlingResourceCallback();

tests/javatests/com/google/android/material/bottomsheet/BottomSheetDialogTest.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,9 @@ public void testHideThenShow() throws Throwable {
188188
final DialogInterface.OnCancelListener onCancelListener =
189189
mock(DialogInterface.OnCancelListener.class);
190190
showDialog();
191+
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
192+
.perform(setShortPeekHeight())
193+
.check(ViewAssertions.matches(ViewMatchers.isDisplayed()));
191194
dialog.setOnCancelListener(onCancelListener);
192195
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
193196
.perform(setState(BottomSheetBehavior.STATE_HIDDEN));
@@ -205,6 +208,33 @@ public void testHideThenShow() throws Throwable {
205208
.onStateChanged(any(View.class), eq(BottomSheetBehavior.STATE_COLLAPSED));
206209
}
207210

211+
@Test
212+
@MediumTest
213+
public void testHideThenShowNotCollapsible() throws Throwable {
214+
// Hide the bottom sheet and wait for the dialog to be canceled.
215+
final DialogInterface.OnCancelListener onCancelListener =
216+
mock(DialogInterface.OnCancelListener.class);
217+
showDialog();
218+
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
219+
.perform(setTallPeekHeight())
220+
.check(ViewAssertions.matches(ViewMatchers.isDisplayed()));
221+
dialog.setOnCancelListener(onCancelListener);
222+
onView(ViewMatchers.withId(R.id.design_bottom_sheet))
223+
.perform(setState(BottomSheetBehavior.STATE_HIDDEN));
224+
verify(onCancelListener, timeout(3000)).onCancel(any(DialogInterface.class));
225+
// Reshow the same dialog instance and wait for the bottom sheet to be collapsed.
226+
final BottomSheetBehavior.BottomSheetCallback callback =
227+
mock(BottomSheetBehavior.BottomSheetCallback.class);
228+
BottomSheetBehavior.from(dialog.findViewById(R.id.design_bottom_sheet))
229+
.addBottomSheetCallback(callback);
230+
// Show the same dialog again.
231+
activityTestRule.runOnUiThread(() -> dialog.show());
232+
verify(callback, timeout(3000))
233+
.onStateChanged(any(View.class), eq(BottomSheetBehavior.STATE_SETTLING));
234+
verify(callback, timeout(3000))
235+
.onStateChanged(any(View.class), eq(BottomSheetBehavior.STATE_EXPANDED));
236+
}
237+
208238
private void showDialog() throws Throwable {
209239
activityTestRule.runOnUiThread(
210240
() -> {
@@ -239,6 +269,26 @@ public void perform(UiController uiController, View view) {
239269
};
240270
}
241271

272+
private static ViewAction setShortPeekHeight() {
273+
return new ViewAction() {
274+
@Override
275+
public Matcher<View> getConstraints() {
276+
return ViewMatchers.isDisplayed();
277+
}
278+
279+
@Override
280+
public String getDescription() {
281+
return "set tall peek height";
282+
}
283+
284+
@Override
285+
public void perform(UiController uiController, View view) {
286+
BottomSheetBehavior<?> behavior = BottomSheetBehavior.from(view);
287+
behavior.setPeekHeight(view.getHeight() / 2);
288+
}
289+
};
290+
}
291+
242292
private static ViewAction setState(@BottomSheetBehavior.State final int state) {
243293
return new ViewAction() {
244294
@Override

0 commit comments

Comments
 (0)