-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Actually consume top padding in bottomsheet if scrollcontrolled #66257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Actually consume top padding in bottomsheet if scrollcontrolled #66257
Conversation
|
Not sure if this is considered a breaking, I don't think any tests will fail but anyone that is using a workaround like proposed in #39205 will get a double padding by this fix |
This does not break any existing flutter or customer tests, so it does not appear to be breaking. :) Once approved, we can run it through google testing to be sure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Triage note: I'm waiting on the google testing shard to start before applying the auto-merge label. |
|
@Kavantix it looks like this wasn't merged due to a tree closure, and now there are merge conflicts. Can you update this so we can land your change? Thank you! |
Removed extra line break in comment Co-authored-by: Kate Lovett <[email protected]>
16bb28c to
6e897ed
Compare
|
@Piinks just rebased it on master |

Description
Currently
showModalBottomSheetalways removes the top padding from theMediaQuerysince it would be 'consumed' since the bottomsheet wont span the entire height of the window.This is however only correct if the sheet is not scroll controlled, because if it is scroll controlled then the bottomsheet will get a maximum size of the whole window again.
So this PR makes
showModalBottomSheetactually consume the padding if the bottomsheet is scrollcontrolled.Related Issues
Fixes #39205
Tests
I added the following tests:
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.
Not sure if this is considered a breaking, I don't think any tests will fail but anyone that is using a workaround like proposed in #39205 will get a double padding by this fix