Skip to content

Commit 9fb73b5

Browse files
Material Design Teamdsn5ft
authored andcommitted
[BottomSheetBehavior] Apply the max width/height during measure instead of modifying layout params
Previously the BottomSheetBehavior was checking the measured size during onLayout and if it exceeded the max width or height configured it would "adjust" them by directly modifying the width and height on the layout params and then posting a setLayoutParams. This has many problems and is fundamentally the wrong way to do it. First off, a view should never modify the layout params that are configured on it for two reasons, one, they are properties of the parent view group, not the view itself, and two, they are almost always intended for users of the view group to configure, not the view group itself. Modifying the layout params directly is clobbering any previously configured parameter and permanently changing the layout (for example consider the case where the max width is set to 100dp and then later set to 200dp, the view would not be allowed to grow to 200dp because its layout params would already have been clobbered). Secondly the post is a very bad way to apply this max, not only does it trigger an entire layout pass a second time but it does so after having completed the first layout pass so it will cause the bottom sheet to render at the original size and then the capped size, it's both inefficient and will cause user visible jank. The correct way to apply a maximum width/height is to do it during layout by incorporating it into the measure specs that are passed to the child. This doesn't clobber any state that the user of the view configured (i.e. when the parent measure specs change, e.g. due to screen rotation, the views correctly relayout) and it avoids doing multiple layout passes, the child can measure itself to the correct size the first time. PiperOrigin-RevId: 387379138
1 parent 47868d8 commit 9fb73b5

File tree

1 file changed

+63
-36
lines changed

1 file changed

+63
-36
lines changed

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

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@
4343
import android.view.MotionEvent;
4444
import android.view.VelocityTracker;
4545
import android.view.View;
46+
import android.view.View.MeasureSpec;
4647
import android.view.ViewConfiguration;
4748
import android.view.ViewGroup;
49+
import android.view.ViewGroup.MarginLayoutParams;
4850
import android.view.ViewParent;
4951
import android.view.accessibility.AccessibilityEvent;
5052
import androidx.annotation.FloatRange;
@@ -194,9 +196,7 @@ public abstract static class BottomSheetCallback {
194196

195197
private static final int CORNER_ANIMATION_DURATION = 500;
196198

197-
private static final int NO_WIDTH = -1;
198-
199-
private static final int NO_HEIGHT = -1;
199+
private static final int NO_MAX_SIZE = -1;
200200

201201
private boolean fitToContents = true;
202202

@@ -221,9 +221,9 @@ public abstract static class BottomSheetCallback {
221221

222222
private MaterialShapeDrawable materialShapeDrawable;
223223

224-
private int maxWidth = NO_WIDTH;
224+
private int maxWidth = NO_MAX_SIZE;
225225

226-
private int maxHeight = NO_HEIGHT;
226+
private int maxHeight = NO_MAX_SIZE;
227227

228228
private int gestureInsetBottom;
229229
private boolean gestureInsetBottomIgnored;
@@ -326,13 +326,13 @@ public BottomSheetBehavior(@NonNull Context context, @Nullable AttributeSet attr
326326
if (a.hasValue(R.styleable.BottomSheetBehavior_Layout_android_maxWidth)) {
327327
setMaxWidth(
328328
a.getDimensionPixelSize(
329-
R.styleable.BottomSheetBehavior_Layout_android_maxWidth, NO_WIDTH));
329+
R.styleable.BottomSheetBehavior_Layout_android_maxWidth, NO_MAX_SIZE));
330330
}
331331

332332
if (a.hasValue(R.styleable.BottomSheetBehavior_Layout_android_maxHeight)) {
333333
setMaxHeight(
334334
a.getDimensionPixelSize(
335-
R.styleable.BottomSheetBehavior_Layout_android_maxHeight, NO_HEIGHT));
335+
R.styleable.BottomSheetBehavior_Layout_android_maxHeight, NO_MAX_SIZE));
336336
}
337337

338338
TypedValue value = a.peekValue(R.styleable.BottomSheetBehavior_Layout_behavior_peekHeight);
@@ -421,6 +421,62 @@ public void onDetachedFromLayoutParams() {
421421
viewDragHelper = null;
422422
}
423423

424+
@Override
425+
public boolean onMeasureChild(
426+
@NonNull CoordinatorLayout parent,
427+
@NonNull V child,
428+
int parentWidthMeasureSpec,
429+
int widthUsed,
430+
int parentHeightMeasureSpec,
431+
int heightUsed) {
432+
MarginLayoutParams lp = (MarginLayoutParams) child.getLayoutParams();
433+
int childWidthMeasureSpec =
434+
getChildMeasureSpec(
435+
parentWidthMeasureSpec,
436+
parent.getPaddingLeft()
437+
+ parent.getPaddingRight()
438+
+ lp.leftMargin
439+
+ lp.rightMargin
440+
+ widthUsed,
441+
maxWidth,
442+
lp.width);
443+
int childHeightMeasureSpec =
444+
getChildMeasureSpec(
445+
parentHeightMeasureSpec,
446+
parent.getPaddingTop()
447+
+ parent.getPaddingBottom()
448+
+ lp.topMargin
449+
+ lp.bottomMargin
450+
+ heightUsed,
451+
maxHeight,
452+
lp.height);
453+
child.measure(childWidthMeasureSpec, childHeightMeasureSpec);
454+
return true; // Child was measured
455+
}
456+
457+
private int getChildMeasureSpec(
458+
int parentMeasureSpec,
459+
int padding,
460+
int maxSize,
461+
int childDimension) {
462+
int result = ViewGroup.getChildMeasureSpec(parentMeasureSpec, padding, childDimension);
463+
if (maxSize == NO_MAX_SIZE) {
464+
return result;
465+
} else {
466+
int mode = MeasureSpec.getMode(result);
467+
int size = MeasureSpec.getSize(result);
468+
switch (mode) {
469+
case MeasureSpec.EXACTLY:
470+
return MeasureSpec.makeMeasureSpec(min(size, maxSize), MeasureSpec.EXACTLY);
471+
case MeasureSpec.AT_MOST:
472+
case MeasureSpec.UNSPECIFIED:
473+
default:
474+
return MeasureSpec.makeMeasureSpec(
475+
size == 0 ? maxSize : min(size, maxSize), MeasureSpec.AT_MOST);
476+
}
477+
}
478+
}
479+
424480
@Override
425481
public boolean onLayoutChild(
426482
@NonNull CoordinatorLayout parent, @NonNull final V child, int layoutDirection) {
@@ -453,8 +509,6 @@ public boolean onLayoutChild(
453509
== ViewCompat.IMPORTANT_FOR_ACCESSIBILITY_AUTO) {
454510
ViewCompat.setImportantForAccessibility(child, ViewCompat.IMPORTANT_FOR_ACCESSIBILITY_YES);
455511
}
456-
457-
adjustChildWidthAndHeightIfNeeded(child);
458512
}
459513
if (viewDragHelper == null) {
460514
viewDragHelper = ViewDragHelper.create(parent, dragCallback);
@@ -1236,33 +1290,6 @@ void setStateInternal(@State int state) {
12361290
updateAccessibilityActions();
12371291
}
12381292

1239-
private void adjustChildWidthAndHeightIfNeeded(@NonNull final V child) {
1240-
final ViewGroup.LayoutParams lp = child.getLayoutParams();
1241-
boolean layoutHasChanges = false;
1242-
1243-
int width = child.getMeasuredWidth();
1244-
if (width > maxWidth && maxWidth != NO_WIDTH) {
1245-
lp.width = maxWidth;
1246-
layoutHasChanges = true;
1247-
}
1248-
1249-
int height = child.getMeasuredHeight();
1250-
if (height > maxHeight && maxHeight != NO_HEIGHT) {
1251-
lp.height = maxHeight;
1252-
layoutHasChanges = true;
1253-
}
1254-
1255-
if (layoutHasChanges) {
1256-
child.post(
1257-
new Runnable() {
1258-
@Override
1259-
public void run() {
1260-
child.setLayoutParams(lp);
1261-
}
1262-
});
1263-
}
1264-
}
1265-
12661293
private void updateDrawableForTargetState(@State int state) {
12671294
if (state == STATE_SETTLING) {
12681295
// Special case: we want to know which state we're settling to, so wait for another call.

0 commit comments

Comments
 (0)