Skip to content

Update PopupRoute docs and add an example #106948

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 4 commits into from
Jul 12, 2022
Merged

Update PopupRoute docs and add an example #106948

auto-submit merged 4 commits into from
Jul 12, 2022
+178 −11

Conversation

@TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented Jul 1, 2022

fixes #106650

related #106186

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Jul 1, 2022
@TahaTesser TahaTesser requested review from Piinks and gspencergoog July 1, 2022 11:38
@TahaTesser TahaTesser force-pushed the update_popup_route_docs branch from ffabb31 to ab0ad6a Compare July 4, 2022 08:10
Piinks
Piinks reviewed Jul 7, 2022
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice samples! Thanks for adding those tests to the other one that already existed! :)

packages/flutter/lib/src/widgets/routes.dart Outdated
Copy link
Contributor

@Piinks Piinks Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IT looks like this is documented on ModalRoute.barrierDismissible, can you update those docs and create a template to re-use here?

Copy link
Member Author

@TahaTesser TahaTesser Jul 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! I just updated ModalRoute docs,

I noticed docs for ModalRoute.barrierDismissible is a template itself but it has too many details that we don't need here, and turned the part we need into a smaller template and mentioned macro here.

Copy link
Contributor

@Piinks Piinks Jul 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fab!

Copy link
Member Author

@TahaTesser TahaTesser Jul 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I had to revert the smaller template as it was nested, after updating the PR, the analyzer throws.
So I simply added the entire ModalRoute.barrierDismissible template to PopupRoute docs

@TahaTesser TahaTesser requested a review from Piinks July 8, 2022 14:41
Piinks
Piinks approved these changes Jul 8, 2022
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit. Thank you!

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 8, 2022
@TahaTesser TahaTesser force-pushed the update_popup_route_docs branch from 57571d3 to de1b67b Compare July 12, 2022 07:54
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 12, 2022

Try again when the tree status has been applied to this PR.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 12, 2022
@TahaTesser TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 12, 2022
@auto-submit auto-submit bot merged commit 66cd09d into flutter:master Jul 12, 2022
@TahaTesser TahaTesser deleted the update_popup_route_docs branch July 12, 2022 09:24
@TahaTesser TahaTesser mentioned this pull request Jul 12, 2022
Closed
This was referenced Jul 12, 2022
Merged
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jul 12, 2022
@engine-flutter-autoroll
66cd09d Update PopupRoute docs and add an example (flutter/flutter#…
a9f516c 
…106948)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 12, 2022
@engine-flutter-autoroll
66cd09d Update PopupRoute docs and add an example (flutter/flutter#…
173a70f 
…106948)
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@TahaTesser @camsim99
Update PopupRoute docs and add an example (flutter#106948)
47da424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Piinks Piinks Piinks approved these changes

@gspencergoog gspencergoog Awaiting requested review from gspencergoog

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PopupRoute: Document dismiss action behavior and add an example

2 participants

@TahaTesser @Piinks