Repository navigation
Deprecate Scaffold resizeToAvoidBottomPadding, now resizeToAvoidBottomInset - #26259
Conversation
|
Is there a bug number. Not sure if I understood completely but isn't the bug already 'incurred' higher than the scaffold layout? i.e. the outer scaffold should just remove the media query padding and inset and then put the inner scaffold into the zero media query no? Otherwise, wouldn't other non-scaffold things inside the scaffold still be wrong? |
zoechi
added
framework
|
Should we document this nested behavior in the doc comment of Scaffold? |
HansMuller
changed the title
HansMuller
changed the title
HansMuller
changed the title
c09adf6 to
5226a6e
Compare
|
The implementation LGTM. Just some doc comments since it's such a central and tricky part of the framework. |
|
I have trouble keeping up with the changes. Though general LGTM. Feel free to submit when the comments are addressed. |
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
Implementation LGTM. Just come comments about dart doc comments...
There was a problem hiding this comment.
uber-nit: The phrase that starts with Better to... doesn't seem to be a sentence?
There was a problem hiding this comment.
Should this clearify that instead of having a separate app bar in each tab there will only be one global app bar for all tabs in this case?
There was a problem hiding this comment.
Given the confusion around nested Scaffolds, I am wondering if we should expand this example more so it shows that there is only one AppBar and one Scaffold instantiated.
There was a problem hiding this comment.
Given the number of potentially worthy scaffold examples we could include here, I'd really prefer to not make a big (bigger) example out of this.
There was a problem hiding this comment.
[resizeToAvoidBottomInset] with square brackets to directly link to it?
There was a problem hiding this comment.
+1 for adding this explanation.
There was a problem hiding this comment.
Maybe add a "see also" section that links to the explanation given on the top level of MediaQueryData that illustrates the difference between the two?
There was a problem hiding this comment.
Good idea, done.
There was a problem hiding this comment.
Should the doc for padding somehow also mention its independence from viewInsets and how the two relate (even if it's only in a "see also" bullet)?
There was a problem hiding this comment.
I added a see also bullet per the previous comment.
8f5f874 to
3e8b8ea
Compare
| /// | ||
| /// ```dart | ||
| /// tabController = TabController(vsync this, length tabCount)..addListener(() { | ||
| /// tabController = TabController(vsync tickerProvider, length tabCount)..addListener(() { |
There was a problem hiding this comment.
Is this one missing : after vsync and length?
There was a problem hiding this comment.
Yes and oops. Thanks for pointing that out so quickly.
5 participants
Fixes #23106 per the simplified test case from #20295.
Fixes #25758
Fixes #12084
Fixes #15424
Scaffold Layout
Originally the
resizeToAvoidBottomPaddingflag referred toMediaQueryData.padding. As of #13437 it refers toMediaQueryData.viewInsets. This has caused confusion among developers (me included). Added a Scaffold property calledresizeToAvoidBottomInsetand deprecated resizeToAvoidBottomPadding. Hopefully this will make things a little clearer to readers. There's a similar property on CupertinoPageScaffold so perhaps we're incrementally more consistent as well.Updated the MediaQuery docs to clarify viewInsets and padding. Updated the Scaffold docs to further clarify resizeToAvoidBottomInset and its relationship to MediaQuery. Updated the MediaQuery docs similarly.
Nested Layout
Nested scaffolds are inset within the parent's scaffold's insets by viewInset.bottom, to avoid the keyboard). Previously, when the keyboard appeared, the bottom of the inner scaffold was adjusted upwards by 2x the keyboard's height (2x the MediaQuery's viewInsets.bottom value). This is because the body's MediaQuery (added by the Scaffold) did not clear the viewInsets.bottom property, even when it had resized body by as much.
Scaffold now clears the body's MediaQuery viewInsets.bottom value if the body was resized.
Updated the Scaffold docs to discourage developers from using nested scaffolds.