Revert to defaultClipBehavior #20578
Conversation
HansMuller
left a comment
There was a problem hiding this comment.
LGTM
I'm not happy about rolling this into Beta. Although this change fixes the FAB problem, the existence of #20483 implies that no one has reviewed the Flutter gallery since the clipping changes landed.
| this.padding, | ||
| this.shape, | ||
| this.clipBehavior = Clip.none, | ||
| this.clipBehavior = ui.defaultClipBehavior, // ignore: deprecated_member_use |
There was a problem hiding this comment.
You provide the same default value for clipBehavior in RawMaterialButton, so there's no need to specify it here.
There was a problem hiding this comment.
Because clipBehavior is defined as a default parameter on RawMaterialButton and we unconditionally pass clipBehavior from all parent widgets, it will end up as null unless we specify it at every level.
There was a problem hiding this comment.
~~ don't think we should include a clipBehavior parameter here (and etc). ~~
Never mind that. If we clip or otherwise make the splash (the InkWell) conform to the button's shape by default then I can understand adding a clipBehavior to buttons, since typically they don't need to be clipped anyway.
| this.disabledElevation = 0.0, | ||
| this.shape, | ||
| this.clipBehavior = Clip.none, | ||
| this.clipBehavior = ui.defaultClipBehavior, // ignore: deprecated_member_use |
There was a problem hiding this comment.
You provide the same default value for clipBehavior in RawMaterialButton, so there's no need to specify it here.
There was a problem hiding this comment.
same as above
| this.padding, | ||
| this.shape, | ||
| this.clipBehavior = Clip.none, | ||
| this.clipBehavior = ui.defaultClipBehavior, // ignore: deprecated_member_use, |
There was a problem hiding this comment.
You provide the same default value for clipBehavior in RawMaterialButton, so there's no need to specify it here.
There was a problem hiding this comment.
same as above
|
Thanks @jonahwilliams . Sorry that I couldn't be in the office today to fix the issue... |
|
This reverts us to the old behavior right? |
|
This has caused a regression in the perf benchmark |
|
The revert was necessary to fix several instances of overly aggressive clipping. If the performance is no worse than the last google3 roll and the last beta roll we should still proceed. |
|
The performance is no worse than the last beta roll |
This reverts commit 8de0e15.
Fixes #20483