-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[go_router] Use the correct configuration to build the state passed to the onExit
#6623
Changes from 3 commits
c75b949
4a9dfd1
7aee926
743c99b
560d3f4
bb8ea59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -233,7 +233,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
exitingMatches.length - 1, | ||
context: navigatorContext, | ||
matches: exitingMatches, | ||
configuration: configuration, | ||
// configuration: configuration, | ||
).then<void>((bool exit) { | ||
if (!exit) { | ||
return SynchronousFuture<void>(null); | ||
|
@@ -254,7 +254,6 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
int index, { | ||
required BuildContext context, | ||
required List<RouteMatch> matches, | ||
required RouteMatchList configuration, | ||
}) { | ||
if (index < 0) { | ||
return SynchronousFuture<bool>(true); | ||
|
@@ -266,7 +265,6 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
index - 1, | ||
context: context, | ||
matches: matches, | ||
configuration: configuration, | ||
); | ||
} | ||
|
||
|
@@ -276,15 +274,14 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
index - 1, | ||
context: context, | ||
matches: matches, | ||
configuration: configuration, | ||
); | ||
} | ||
return SynchronousFuture<bool>(false); | ||
} | ||
|
||
final FutureOr<bool> exitFuture = goRoute.onExit!( | ||
context, | ||
match.buildState(_configuration, configuration), | ||
match.buildState(_configuration, currentConfiguration), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so the difference is whether we use the new RouteMatchList after the change vs old RouteMatchList? I think for redirect we use the new RouteMatchList. it seems to previous implementation is more inline with the redirect API no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I maybe wrong here. do you have use case that using the old routematchList is more preferred? on the other hand, I plan to redesign redirect, so maybe we should just provide both before and after? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I found one in #6614. I was making The issue was when using The test I wrote in this PR 'It should provide the correct path parameters to the onExit callback during a go' fails without my changes.
Sure, do you want me to change something then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will need to redesign the onExit in the future. It also has issue with popscope and cupertinobackswipe. Thing may become a lot different after I resolve all these issues. After thinking about it more, I think we should just keep it simple for now. I think what you have is fine. |
||
); | ||
if (exitFuture is bool) { | ||
return handleOnExitResult(exitFuture); | ||
|
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.
?!
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.
Oh my ! Sorry about that. I removed it in refactor: Remove commented code