-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
ScaffoldMessenger Migration Guide #4527
Conversation
Sorry, @Piinks, but I just landed a largish PR that updates the breaking change files. Please fix the conflicts at your convenience. :) (As this change hasn't landed, I see we are in no rush.) |
…into scaffoldMessenger
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.
Once we get the timeline info, it looks good!
), | ||
)); | ||
}, | ||
child: const Text('SHOW SNACK'), |
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.
Any particular reason to use all caps here and above? I'm guessing your intent is to make it obvious that the developer can change these, but it just feels shouty... =)
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.
Oh I was just copying a sample from the API docs. 😜
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.
Fair 😋
Update: @sfshaza2 this is still WIP, the change was reverted so we don't have a build tag yet. :) |
@Sfshaza we finally have a build tag for this one, ready to go with your approval! 🎉 |
Please include the error message that you're likely to see if you haven't migrated, so that people can find this by Googling the error message. |
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.
A couple minor nits but, overall, looks great!
); | ||
``` | ||
|
||
When presenting a `SnackBar` during a transition, the `SnackBar` will `Hero`, animating smoothly to |
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.
Never heard "Hero" as a verb. Is this a common usage?
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.
I have heard it, but it may mot be common. Updated.
I've added the error message and applied your suggestion @sfshaza2. Thanks! |
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
* migration guide * Apply suggestions from code review * Updates * Update src/docs/release/breaking-changes/scaffold-messenger.md * ++
Migration guide for change: flutter/flutter#64101
Change with framework migrations: flutter/flutter#64170
Issue: flutter/flutter#62921