From 04c483cf3466cd0a7bf4835cadccc2372ba0da2b Mon Sep 17 00:00:00 2001 From: conradchen Date: Mon, 7 Mar 2022 17:25:53 +0000 Subject: [PATCH] [CleanUp][BottomSheet] Simplify state transition logic Reduces and restructures the calling chain when a state is set to improve readability and robustness. This CL also introduces the following fixes: 1. Limit the possible input values of setState() and throws IllegalArgumentException if invalid values are provided. 2. Block the state to be set as HIDDEN when hideable is false. 3. Fix the bug that SettleRunnable could be posted multiple times due to isPosted flag being incorrectly set to false when the settling is still going on. 4. Fix the bug that dragging the bottom sheet when settling would result the bottom sheet in an incorrect state (the target state of settling instead of DRAGGING). PiperOrigin-RevId: 432963533 --- .../bottomsheet/BottomSheetBehavior.java | 243 ++++++++---------- 1 file changed, 104 insertions(+), 139 deletions(-) diff --git a/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java b/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java index 6f3e66f4eb7..afd716bce7f 100644 --- a/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java +++ b/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java @@ -142,6 +142,17 @@ void onLayout(@NonNull View bottomSheet) {} @Retention(RetentionPolicy.SOURCE) public @interface State {} + /** + * Stable states that can be set by the {@link #setState(int)} method. These includes all the + * possible states a bottom sheet can be in when it's settled. + * + * @hide + */ + @RestrictTo(LIBRARY_GROUP) + @IntDef({STATE_EXPANDED, STATE_COLLAPSED, STATE_HIDDEN, STATE_HALF_EXPANDED}) + @Retention(RetentionPolicy.SOURCE) + public @interface StableState {} + /** * Peek at the 16:9 ratio keyline of its parent. * @@ -245,7 +256,7 @@ void onLayout(@NonNull View bottomSheet) {} private boolean isShapeExpanded; - private SettleRunnable settleRunnable = null; + private final StateSettlingTracker stateSettlingTracker = new StateSettlingTracker(); @Nullable private ValueAnimator interpolatorAnimator; @@ -742,76 +753,61 @@ public void onStopNestedScroll( || !nestedScrolled)) { return; } - int top; - int targetState; + @StableState int targetState; if (lastNestedScrollDy > 0) { if (fitToContents) { - top = fitToContentsOffset; targetState = STATE_EXPANDED; } else { int currentTop = child.getTop(); if (currentTop > halfExpandedOffset) { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } else { - top = getExpandedOffset(); targetState = STATE_EXPANDED; } } } else if (hideable && shouldHide(child, getYVelocity())) { - top = parentHeight; targetState = STATE_HIDDEN; } else if (lastNestedScrollDy == 0) { int currentTop = child.getTop(); if (fitToContents) { if (Math.abs(currentTop - fitToContentsOffset) < Math.abs(currentTop - collapsedOffset)) { - top = fitToContentsOffset; targetState = STATE_EXPANDED; } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } else { if (currentTop < halfExpandedOffset) { if (currentTop < Math.abs(currentTop - collapsedOffset)) { - top = getExpandedOffset(); targetState = STATE_EXPANDED; } else { if (shouldSkipHalfExpandedStateWhenDragging()) { - top = collapsedOffset; targetState = STATE_COLLAPSED; } else { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } } } else { if (Math.abs(currentTop - halfExpandedOffset) < Math.abs(currentTop - collapsedOffset)) { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } } } else { if (fitToContents) { - top = collapsedOffset; targetState = STATE_COLLAPSED; } else { // Settle to nearest height. int currentTop = child.getTop(); if (Math.abs(currentTop - halfExpandedOffset) < Math.abs(currentTop - collapsedOffset)) { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } } - startSettlingAnimation(child, targetState, top, false); + startSettling(child, targetState, false); nestedScrolled = false; } @@ -974,7 +970,7 @@ private void updatePeekHeight(boolean animate) { V view = viewRef.get(); if (view != null) { if (animate) { - settleToStatePendingLayout(state); + setState(STATE_COLLAPSED); } else { view.requestLayout(); } @@ -1197,22 +1193,53 @@ public void removeBottomSheetCallback(@NonNull BottomSheetCallback callback) { * @param state One of {@link #STATE_COLLAPSED}, {@link #STATE_EXPANDED}, {@link #STATE_HIDDEN}, * or {@link #STATE_HALF_EXPANDED}. */ - public void setState(@State int state) { - if (state == this.state) { + public void setState(@StableState int state) { + if (state == STATE_DRAGGING || state == STATE_SETTLING) { + throw new IllegalArgumentException( + "STATE_" + + (state == STATE_DRAGGING ? "DRAGGING" : "SETTLING") + + " should not be set externally."); + } + if (!hideable && state == STATE_HIDDEN) { + Log.w(TAG, "Cannot set state: " + state); return; } - if (viewRef == null) { + final int finalState; + if (state == STATE_HALF_EXPANDED + && fitToContents + && getTopOffsetForState(state) <= fitToContentsOffset) { + // Skip to the expanded state if we would scroll past the height of the contents. + finalState = STATE_EXPANDED; + } else { + finalState = state; + } + if (viewRef == null || viewRef.get() == null) { // The view is not laid out yet; modify mState and let onLayoutChild handle it later - if (state == STATE_COLLAPSED - || state == STATE_EXPANDED - || state == STATE_HALF_EXPANDED - || (hideable && state == STATE_HIDDEN)) { - this.state = state; - this.lastStableState = state; - } - return; + setStateInternal(state); + } else { + final V child = viewRef.get(); + runAfterLayout( + child, + new Runnable() { + @Override + public void run() { + startSettling(child, finalState, false); + } + }); + } + } + + private void runAfterLayout(V child, Runnable runnable) { + if (isLayouting(child)) { + child.post(runnable); + } else { + runnable.run(); } - settleToStatePendingLayout(state); + } + + private boolean isLayouting(V child) { + ViewParent parent = child.getParent(); + return parent != null && parent.isLayoutRequested() && ViewCompat.isAttachedToWindow(child); } /** @@ -1235,27 +1262,6 @@ public boolean isGestureInsetBottomIgnored() { return gestureInsetBottomIgnored; } - private void settleToStatePendingLayout(@State int state) { - final V child = viewRef.get(); - if (child == null) { - return; - } - // Start the animation; wait until a pending layout if there is one. - ViewParent parent = child.getParent(); - if (parent != null && parent.isLayoutRequested() && ViewCompat.isAttachedToWindow(child)) { - final int finalState = state; - child.post( - new Runnable() { - @Override - public void run() { - settleToState(child, finalState); - } - }); - } else { - settleToState(child, state); - } - } - /** * Gets the current state of the bottom sheet. * @@ -1549,62 +1555,39 @@ private float getYVelocity() { return velocityTracker.getYVelocity(activePointerId); } - void settleToState(@NonNull View child, @State int state) { - int top; - if (state == STATE_COLLAPSED) { - top = collapsedOffset; - } else if (state == STATE_HALF_EXPANDED) { - top = halfExpandedOffset; - if (fitToContents && top <= fitToContentsOffset) { - // Skip to the expanded state if we would scroll past the height of the contents. - state = STATE_EXPANDED; - top = fitToContentsOffset; - } - } else if (state == STATE_EXPANDED) { - top = getExpandedOffset(); - } else if (hideable && state == STATE_HIDDEN) { - top = parentHeight; - } else { - // TODO(b/204062131): Possible illegal state when hideable is modified while the state is - // being updated. - Log.w( - TAG, - "The bottom sheet may be in an invalid state. Ensure `hideable` is true when using" - + " `STATE_HIDDEN`."); - return; - } - startSettlingAnimation(child, state, top, false); - } - - void startSettlingAnimation( - View child, @State int state, int top, boolean settleFromViewDragHelper) { - boolean startedSettling = + private void startSettling(View child, @StableState int state, boolean isReleasingView) { + int top = getTopOffsetForState(state); + boolean settling = viewDragHelper != null - && (settleFromViewDragHelper + && (isReleasingView ? viewDragHelper.settleCapturedViewAt(child.getLeft(), top) : viewDragHelper.smoothSlideViewTo(child, child.getLeft(), top)); - if (startedSettling) { + if (settling) { setStateInternal(STATE_SETTLING); // STATE_SETTLING won't animate the material shape, so do that here with the target state. updateDrawableForTargetState(state); - if (settleRunnable == null) { - // If the singleton SettleRunnable instance has not been instantiated, create it. - settleRunnable = new SettleRunnable(child, state); - } - // If the SettleRunnable has not been posted, post it with the correct state. - if (!settleRunnable.isPosted) { - settleRunnable.targetState = state; - ViewCompat.postOnAnimation(child, settleRunnable); - settleRunnable.isPosted = true; - } else { - // Otherwise, if it has been posted, just update the target state. - settleRunnable.targetState = state; - } + stateSettlingTracker.continueSettlingToState(state); } else { setStateInternal(state); } } + private int getTopOffsetForState(@StableState int state) { + switch (state) { + case STATE_COLLAPSED: + return collapsedOffset; + case STATE_EXPANDED: + return getExpandedOffset(); + case STATE_HALF_EXPANDED: + return halfExpandedOffset; + case STATE_HIDDEN: + return parentHeight; + default: + // Fall through + } + throw new IllegalArgumentException("Invalid state to get top offset: " + state); + } + private final ViewDragHelper.Callback dragCallback = new ViewDragHelper.Callback() { @@ -1649,11 +1632,9 @@ private boolean releasedLow(@NonNull View child) { @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; } else { int currentTop = releasedChild.getTop(); @@ -1663,18 +1644,14 @@ public void onViewReleased(@NonNull View releasedChild, float xvel, float yvel) float yPositionPercentage = currentTop * 100f / parentHeight; if (shouldExpandOnUpwardDrag(dragDurationMillis, yPositionPercentage)) { - top = expandedOffset; targetState = STATE_EXPANDED; } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } else { if (currentTop > halfExpandedOffset) { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } else { - top = getExpandedOffset(); targetState = STATE_EXPANDED; } } @@ -1684,17 +1661,13 @@ public void onViewReleased(@NonNull View releasedChild, float xvel, float yvel) // otherwise settle to closest expanded state. if ((Math.abs(xvel) < Math.abs(yvel) && yvel > SIGNIFICANT_VEL_THRESHOLD) || releasedLow(releasedChild)) { - top = parentHeight; targetState = STATE_HIDDEN; } else if (fitToContents) { - top = fitToContentsOffset; targetState = STATE_EXPANDED; } else if (Math.abs(releasedChild.getTop() - getExpandedOffset()) < Math.abs(releasedChild.getTop() - halfExpandedOffset)) { - top = getExpandedOffset(); targetState = STATE_EXPANDED; } else { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } } else if (yvel == 0.f || Math.abs(xvel) > Math.abs(yvel)) { @@ -1704,23 +1677,18 @@ public void onViewReleased(@NonNull View releasedChild, float xvel, float yvel) if (fitToContents) { if (Math.abs(currentTop - fitToContentsOffset) < Math.abs(currentTop - collapsedOffset)) { - top = fitToContentsOffset; targetState = STATE_EXPANDED; } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } else { if (currentTop < halfExpandedOffset) { if (currentTop < Math.abs(currentTop - collapsedOffset)) { - top = getExpandedOffset(); targetState = STATE_EXPANDED; } else { if (shouldSkipHalfExpandedStateWhenDragging()) { - top = collapsedOffset; targetState = STATE_COLLAPSED; } else { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } } @@ -1728,21 +1696,17 @@ public void onViewReleased(@NonNull View releasedChild, float xvel, float yvel) if (Math.abs(currentTop - halfExpandedOffset) < Math.abs(currentTop - collapsedOffset)) { if (shouldSkipHalfExpandedStateWhenDragging()) { - top = collapsedOffset; targetState = STATE_COLLAPSED; } else { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } } } else { // Moving Down if (fitToContents) { - top = collapsedOffset; targetState = STATE_COLLAPSED; } else { // Settle to the nearest correct height. @@ -1750,19 +1714,16 @@ public void onViewReleased(@NonNull View releasedChild, float xvel, float yvel) if (Math.abs(currentTop - halfExpandedOffset) < Math.abs(currentTop - collapsedOffset)) { if (shouldSkipHalfExpandedStateWhenDragging()) { - top = collapsedOffset; targetState = STATE_COLLAPSED; } else { - top = halfExpandedOffset; targetState = STATE_HALF_EXPANDED; } } else { - top = collapsedOffset; targetState = STATE_COLLAPSED; } } } - startSettlingAnimation(releasedChild, targetState, top, shouldSkipSmoothAnimation()); + startSettling(releasedChild, targetState, shouldSkipSmoothAnimation()); } @Override @@ -1888,29 +1849,33 @@ public int getLastStableState() { return lastStableState; } - private class SettleRunnable implements Runnable { + private class StateSettlingTracker { + @State private int targetState; + private boolean isContinueSettlingRunnablePosted; - private final WeakReference viewRef; - - private boolean isPosted; - - @State int targetState; + private final Runnable continueSettlingRunnable = + new Runnable() { + @Override + public void run() { + isContinueSettlingRunnablePosted = false; + if (viewDragHelper != null && viewDragHelper.continueSettling(true)) { + continueSettlingToState(targetState); + } else if (state == STATE_SETTLING) { + setStateInternal(targetState); + } + // In other cases, settling has been interrupted by certain UX interactions. Do nothing. + } + }; - SettleRunnable(View view, @State int targetState) { - this.viewRef = new WeakReference<>(view); + void continueSettlingToState(@State int targetState) { + if (viewRef == null || viewRef.get() == null) { + return; + } this.targetState = targetState; - } - - @Override - public void run() { - if (viewRef.get() != null - && viewDragHelper != null - && viewDragHelper.continueSettling(true)) { - ViewCompat.postOnAnimation(viewRef.get(), this); - } else { - setStateInternal(targetState); + if (!isContinueSettlingRunnablePosted) { + ViewCompat.postOnAnimation(viewRef.get(), continueSettlingRunnable); + isContinueSettlingRunnablePosted = true; } - this.isPosted = false; } }