-
Notifications
You must be signed in to change notification settings - Fork 28.7k
PopScope example improvements #142163
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
PopScope example improvements #142163
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.
I looked at https://main-api.flutter.dev/flutter/widgets/PopScope-class.html the example description only says "This sample demonstrates how to use this widget to handle nested navigation in a bottom navigation bar.".
If I was looking for information on how to use an async function in PopScope I would probably turn away upon reading that description since it doesn't sound that interesting.
/// Shows a dialog and resolves to true when the user has indicated that they | ||
/// want to pop. | ||
/// | ||
/// A null indicates a desire not to pop. |
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.
Where's null introduced?
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.
showDialog can return a Future that resolves to null. Dismissing the dialog (such as by tapping the barrier) will resolve to null. In those cases, the user doesn't want to leave the page.
I'll change that to "A return value of null indicates a desire not to pop, such as when the user has dismissed the modal without tapping a button."
@LongCatIsLooong Good catch, that example description is actually totally wrong. I must have copied it from one of the navigator_pop_handler examples by mistake. I've updated it. |
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
Manual roll requested by [email protected] flutter/flutter@a8efa77...2f6fdf2 2024-01-26 [email protected] Start renaming by adding a new `bringup: true` as an Android emulator. (flutter/flutter#142257) 2024-01-26 [email protected] Instrument ImageInfo. (flutter/flutter#141411) 2024-01-26 [email protected] Fix `SegmentedButton` default size and default tappable size (flutter/flutter#142243) 2024-01-26 [email protected] Update name for android_defines_test. (flutter/flutter#142273) 2024-01-26 [email protected] Enable native compilation for windows-arm64 (flutter/flutter#141930) 2024-01-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 4b145d041560 to 44f26274bfbe (6 revisions)" (flutter/flutter#142274) 2024-01-25 [email protected] Run a few mac tests only on arm. (flutter/flutter#142188) 2024-01-25 [email protected] fix Ink not updating on TextField newline (flutter/flutter#140700) 2024-01-25 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 3.1.4 to 3.1.5 (flutter/flutter#142259) 2024-01-25 [email protected] Roll Flutter Engine from 4b145d041560 to 44f26274bfbe (6 revisions) (flutter/flutter#142264) 2024-01-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Rename `integration_tests/external_ui` but do not touch anything else..."" (flutter/flutter#142268) 2024-01-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Rename `integration_tests/external_ui` but do not touch anything else..." (flutter/flutter#142265) 2024-01-25 [email protected] Roll Flutter Engine from b2167a93c1a0 to 4b145d041560 (3 revisions) (flutter/flutter#142256) 2024-01-25 [email protected] Implementing `switch` expressions in the `cupertino/` directory (flutter/flutter#141591) 2024-01-25 [email protected] Rename `integration_tests/external_ui` but do not touch anything else... (flutter/flutter#142238) 2024-01-25 [email protected] Roll Flutter Engine from 55eefd5bd255 to b2167a93c1a0 (2 revisions) (flutter/flutter#142252) 2024-01-25 [email protected] Roll Flutter Engine from 3b4779324b44 to 55eefd5bd255 (6 revisions) (flutter/flutter#142245) 2024-01-25 [email protected] Fix incorrect zh-cn translation for Look Up Label in selection controls (flutter/flutter#142158) 2024-01-25 [email protected] PopScope example improvements (flutter/flutter#142163) 2024-01-25 [email protected] Roll Flutter Engine from 1d3f16b0d62e to 3b4779324b44 (1 revision) (flutter/flutter#142225) 2024-01-25 [email protected] Roll Packages from 8fbdf65 to 21b5abb (6 revisions) (flutter/flutter#142224) 2024-01-25 [email protected] Don't show legacy welcome message when analytics are disabled (flutter/flutter#140956) 2024-01-25 [email protected] Roll Flutter Engine from 7c4ed15cb271 to 1d3f16b0d62e (1 revision) (flutter/flutter#142223) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Updates two PopScope examples to a more clear usage of showDialog.
This came up in #138614, where many users were confused about how to use a back confirmation dialog with PopScope. Two examples of this already exist, but now they are cleaner and follow more typical navigation patterns, so users should recognize their own code in them more easily.