Repository navigation
Fixes popuntilwithresult drops results - #190596
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the NavigatorState class in navigator.dart to check candidate.route.willHandlePopInternally instead of next.route.willHandlePopInternally when determining whether to pop with a result. Additionally, a new widget test has been added to verify that popUntilWithResult correctly returns a value to the last popped route when the destination route contains local history entries, and an existing test has been reformatted. There are no review comments, and I have no feedback to provide.
justinmc
left a comment
There was a problem hiding this comment.
Looks good, but +1 to @navaronbracke's comments about not using Material in the test.
| TestWidgetsApp( | ||
| initialRoute: '/', | ||
| onGenerateRoute: (RouteSettings settings) { | ||
| final String? routeName = settings.name; |
There was a problem hiding this comment.
Nit: this could be a switch expression, but I don't mind it
return switch (settings.name) {
'/' => TestRoute<bool>(
...
),
...
_ => null,
};
There was a problem hiding this comment.
I will keep it as is
as title, it is a typo and should have checked current route's will handle pop internally
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.