Skip to content
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

feat: allow reassignment of MapController to FlutterMap multiple times #1915

Conversation

josxha
Copy link
Contributor

@josxha josxha commented Jun 13, 2024

Description

Allows the MapController to get assigned multiple times to a view (only one a every time).
Can't tell much about potential side effects. Did some quick tests and everything worked alright so far but additional testing would be good.

Issue

@pagetronic
Copy link

pagetronic commented Jun 28, 2024

Perfect for me.

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I think. Thanks :)

Tbh I'm not 100% confident that all of the users experiencing this issue are using the MapController as per the documentation in the first place, especially when external state is involved, users seem to commonly get the setup wrong by initialising/constructing/defining the MapController in the state initially, then passing that to the map, rather than the reverse: https://docs.fleaflet.dev/usage/programmatic-interaction/external-custom-controllers#usage-within-a-state-system-model. Therefore, I'm not planning to change the documentation for this: I'm not sure how this will affect the need for those instructions, but they should ensure more stability than other options for the future.

@JaffaKetchup JaffaKetchup changed the title feat: assign MapController to the map widget multiple times feat: allow reassignment of MapController to FlutterMap multiple times Jul 2, 2024
@JaffaKetchup JaffaKetchup merged commit 74110db into fleaflet:master Jul 2, 2024
6 checks passed
@josxha josxha deleted the feat/assign-controller-to-map-widget-multiple-times branch July 2, 2024 14:45
@josxha
Copy link
Contributor Author

josxha commented Jul 2, 2024

Tbh I'm not 100% confident that all of the users experiencing this issue are using the MapController as per the documentation in the first place, especially when external state is involved

Yes, I agree. All the reported issues that I've seen on this were caused by incorrect usage of the MapController. But anyways, now we have unofficial support for those implementations. 👍

Thanks for the code review (:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG] External MapController lifecycle not synced with widget when destroyed and rebuilt
3 participants