-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[go_router_builder] Add support for Iterable, List and Set to TypedGoRoute #2679
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
Changes from all commits
b2c18aa
280c8dd
a083136
32a4f8a
3dee0f0
94b1497
48cbcf7
f1d39c6
5a52fc0
5f604cd
1d74193
a6a0e1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,5 +125,15 @@ void main() { | |
expect(find.text('UriRoute'), findsOneWidget); | ||
expect(find.text('Param: https://dart.dev'), findsOneWidget); | ||
expect(find.text('Query param: https://dart.dev'), findsOneWidget); | ||
|
||
IterableRoute( | ||
intListField: <int>[1, 2, 3], | ||
).go(scaffoldState.context); | ||
await tester.pumpAndSettle(); | ||
expect(find.text('IterableRoute'), findsOneWidget); | ||
expect( | ||
find.text( | ||
'/iterable-route?int-list-field=1&int-list-field=2&int-list-field=3'), | ||
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. Seems a bit weird to hardcoded the query parameter name, I think this may make it less useful if a company care about the query parameter names. Would it make more sense to fix flutter/flutter#110781 and provide an API that people can set it up for iterable object with minimum code? 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. this seems not addressed yet? |
||
findsOneWidget); | ||
}); | ||
} |
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.
Would be good to modulalize this to be a class. I imagine someone would like to do this
The idea is to be able to choose the query parameter name.
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.
Hi @Skogsfrae WDYT about this? The main concern is the query parameter names are hardcoded which I imagine people would ask for customized query parameter names in the future. Although this is not something we have to fix now, but the current API, as it is written, may not be able the extend to support this feature.