-
Notifications
You must be signed in to change notification settings - Fork 28.5k
Stop recommending "shrinkWrap" #104008
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
Stop recommending "shrinkWrap" #104008
Conversation
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.
LGTM!!
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.
L-so-GTM! Thanks!
(from triage) @dnfield Looks like the google check got stuck on this one... |
I've manually marked the status as clean, this can't actually be causing failures... |
Fixes #103085
Using a ShrinkWrapping viewport is pretty much never the right thing to do in the case of this error. It introduces a much more expensive widget to the tree than a
Column
,Row
, orWrap
would be, and the real solution if you can't use those is to use aCustomScrollView
instead to arrange your slivers.One of the main problems introduced by this pattern is that it breaks up a scrollable into many more repaint boundaries than necessary, which makes painting/compositing/rasterization less efficient later in most cases.