[BottomSheetBehavior] Bottom Sheet State always set to `STATE_EXPANDED` when swiping up

[SilverEnd]

Description: When Bottom Sheet's flag fitToContents is set to default value true, the bottom sheet will always set its state to STATE_EXPANDED whenever a swipe up gesture is performed on the bottom sheet, even tho the content is not long enough to be expanded to full screen, i.e. swiping up result in NO VISUAL CHANGES to the UI.

A side effect of the above is that, in a "non-expandable" bottom sheet, when we do swipe up gesture, it will think itself been set to STATE_EXPANDED and then invalidate the bottom sheet drawable, which will make the top corners of the bottom sheet flat (from rounded top corners)

Take the blow recording as a example:

STATE_COLLAPSED -> STATE_EXPANDED by swiping up

https://user-images.githubusercontent.com/5491995/132157081-d8cb280d-9c87-4f3c-85de-bcdd6c5c9b4e.mp4

STATE_EXPANDED -> STATE_COLLAPSED by swiping down

https://user-images.githubusercontent.com/5491995/132157228-fd3bed03-eabf-4015-b851-704024990131.mp4

STATE_EXPANDED -> STATE_COLLAPSED by swiping down on BottomSheetDialog, release it and let itself auto settle

https://user-images.githubusercontent.com/5491995/132157269-dfcb7fc3-7b1d-43be-b4a5-bc019ce3a35e.mp4

Expected behavior: This might be controversial but there are two directions we would expect:

• NEVER have STATE_COLLAPSED for a short bottom sheet that fitToContent == true, (this will involve fix for other use cases to prevent the state becomes STATE_COLLAPSED) OR
• Prevent the bottom sheet's state to become STATE_EXPANDED when we tries to swipe up

Source code:

@Override
public void onViewReleased(@NonNull View releasedChild, float xvel, float yvel) {
  int top;
  @State int targetState;
  if (yvel < 0) { // Moving up
    if (fitToContents) {
      top = fitToContentsOffset;
      targetState = STATE_EXPANDED;    <<<-------- This is where we need to ad extra logic
    } else {
      int currentTop = releasedChild.getTop();
      if (currentTop > halfExpandedOffset) {
        top = halfExpandedOffset;
        targetState = STATE_HALF_EXPANDED;
      } else {
        top = getExpandedOffset();
        targetState = STATE_EXPANDED;
      }
    }
  } else if (hideable && shouldHide(releasedChild, yvel)) {
    ...
  } else if (yvel == 0.f || Math.abs(xvel) > Math.abs(yvel)) {
    // If the Y velocity is 0 or the swipe was mostly horizontal indicated by the X velocity
    // being greater than the Y velocity, settle to the nearest correct height.
    ...
  } else { // Moving Down
   ...
  }
  startSettlingAnimation(releasedChild, targetState, top, true);
}

Proposed Solution for the 2nd approach above (as it will not impact other uses cases)

if (yvel < 0) { // Moving up
  if (fitToContents) {
    // Keep it at collapsed state when the collapsed height and expanded height are the same,
    // i.e. content is not long enough to visually expand the bottom sheet
    if (fitToContentsOffset == collapsedOffset) {
      top = collapsedOffset;
      targetState = STATE_COLLAPSED;
    } else {
      top = fitToContentsOffset;
      targetState = STATE_EXPANDED;
    }
  }
} else if (...) { ... } 

Android API version: API 29

Material Library version: 1.5.0-alpha02

Device: ONE PLUS 8 PRO

To help us triage faster, please check to make sure you are using the latest version of the library.

We also happily accept pull requests.

[drchen]

Hi, thanks for the report and the proposed solutions.

I think Solution 1 makes more sense to me but Solution 2 are much safer and less likely to break existing clients.

Do you mind to create a pull request of solution 2 and I can use it to discuss with the team? Or I can also create one. : )

[drchen]

Hi, after some investigation, I think I need to raise this to our designers to confirm the desired behavior here. Will get back to you when I know the answer, thank you!

[drchen]

Reopen this since we reverted the change.

[SilverEnd]

@drchen thanks for the reply. I am actually curious what is the correct design behavior is?

[drchen]

It's kind of tricky due to the current implementation of clients usually assume it is in COLLAPSED state when the situation happens (COLLAPSED and EXPANDED states has same offsets). We are still debating if the state should be COLLAPSED or EXPANDED.

Good news is we are aiming to update the design so that basically both states will have rounded corner in the new design so the look and feel will be more consistent.