-
Notifications
You must be signed in to change notification settings - Fork 84
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
migrate to flutter_map 3 #51
Conversation
Thank you very much for the contribution! I made one little comment. Apart from that I noticed that a package source was changed to a github url so I guess this PR is in standby until v3 is published officially? |
V3 is published and this PR is ready! |
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.
Some quick small comments but it's looking almost ready!
@@ -291,4 +284,4 @@ packages: | |||
version: "2.0.0" | |||
sdks: | |||
dart: ">=2.17.1 <3.0.0" | |||
flutter: ">=2.0.0" |
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.
What causes this to jump to 3.3?
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.
Flutter 3.3 is the latest stable release as well as being required by flutter_map due to having completed the migration to the new image provider API.
@@ -31,33 +31,30 @@ class _PopupMarkerLayerWidgetState extends State<PopupMarkerLayerWidget> { | |||
Widget build(BuildContext context) { | |||
return PopupStateWrapper( | |||
builder: (context, popupState) => _layers( | |||
mapState: flutter_map.MapState.maybeOf(context)!, | |||
mapState: flutter_map.FlutterMapState.maybeOf(context)!, |
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.
Maybe now that MapState
is renamed to FlutterMapState
we don't need to disambiguate with flutter_map.FlutterMapState?
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.
Still need to with your custom MarkerLayer class.
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.
Unless we remove the custom MarkerLayer Widget or change its name we need to keep the disambiguation.
Awesome, thanks for the contribution @moonag ! |
fleaflet/flutter_map#1333