-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[go_router] Fixes a bug in debugLogDiagnostics to support StatefulShellRoute
#4177
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
Conversation
debugLogDiagnostics to support StatefulShellRoutedebugLogDiagnostics to support StatefulShellRoute
|
I'm the reporter and I just tested this, worked as expected! 🎉 |
| sb.writeln(' => ${''.padLeft(depth * 2)}$fullPath'); | ||
| _debugFullPathsFor(route.routes, fullPath, depth + 1, sb); | ||
| } else if (route is ShellRoute) { | ||
| } else if (route is ShellRoute || route is StatefulShellRoute) { |
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.
| } else if (route is ShellRoute || route is StatefulShellRoute) { | |
| } else if (route is ShellRouteBase) { |
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 considered this, but I thought this may break in future if someone writes are new concrete implementation of ShellRouteBase.
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'm pushing changes as you suggested.
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.
It should be fine as long as they follow the same contract of ShellRouteBase.
chunhtai
left a comment
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
hannah-hyj
left a comment
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
…tatefulShellRoute (flutter/packages#4177)
flutter/packages@c9865e8...0507297 2023-06-12 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump com.google.guava:guava from 32.0.0-android to 32.0.1-android in /packages/camera/camera_android_camerax/android (flutter/packages#4195) 2023-06-12 [email protected] [ci] Finish migrating Pigeon tests to LUCI (flutter/packages#3192) 2023-06-12 49699333+dependabot[bot]@users.noreply.github.com [local_auth]: Bump androidx.fragment:fragment from 1.5.7 to 1.6.0 in /packages/local_auth/local_auth_android/android (flutter/packages#4186) 2023-06-12 49699333+dependabot[bot]@users.noreply.github.com [sign_in]: Bump com.google.guava:guava from 32.0.0-android to 32.0.1-android in /packages/google_sign_in/google_sign_in_android/android (flutter/packages#4184) 2023-06-12 [email protected] [go_router] Fixes a bug in `debugLogDiagnostics` to support StatefulShellRoute (flutter/packages#4177) 2023-06-12 [email protected] Roll Flutter from 3df163f to 353b8bc (10 revisions) (flutter/packages#4198) 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-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
PR fixes the following issue:
debugLogDiagnostics: truedoesn't outputStatefulShellRouteroutes flutter#127957Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.