Skip to content

Conversation

@johnpryan
Copy link
Contributor

@johnpryan johnpryan commented Sep 20, 2022

Updates the navigation_and_routing sample to go_router 5.

Depends on flutter/flutter#111981

Pre-launch Checklist

  • I read the [Flutter Style Guide] recently, and have followed its advice.
  • I signed the [CLA].
  • I read the [Contributors Guide].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.

return FadeTransitionPage<dynamic>(
key: state.pageKey,
// Use a builder to get the correct BuildContext
// TODO (johnpryan): remove when https://github.com/flutter/flutter/issues/108177 lands
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this PR be blocked on this issue being resolved?

@domesticmouse
Copy link
Contributor

I'm curious why CI is red, any ideas?

@domesticmouse domesticmouse changed the title Update navigation_and_routing sample to go_router 5 Update navigation_and_routing sample to go_router 5 Dec 13, 2022
onTap: (idx) {
switch (idx) {
case 1:
GoRouter.of(context).go('/books/new');
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: Would it be possible to use context.go() extension function instead of GoRouter.of(context).go? so it aligns with https://pub.dev/packages/go_router/example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That fine with me

@miquelbeltran
Copy link
Member

I wanted to enable Material 3 on this sample, but since I saw this PR open, I didn't want to mess with the work and cause a merge conflict, so I wanted to ask first. How do you prefer me to proceed?

@johnpryan
Copy link
Contributor Author

Brett, do you see #1437 (comment) as a blocker or should we try to merge this PR?

@johnpryan johnpryan closed this Apr 26, 2023
johnpryan added a commit that referenced this pull request Nov 1, 2023
This is a new PR to update the navigation_and_routing sample to the
latest version of go_router. It is a based on
#1437

---------

Co-authored-by: Brett Morgan <brettmorgan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants