Fix libraries being repeated when using heatmapLibrary #934
Conversation
Supports places, visualization, places, and geomerty libs. keeps support for previous heatMapLibrary prop to avoid breaking older usage.
API.md
Outdated
| @@ -384,8 +387,24 @@ interface heatmapProp { | |||
|
|
|||
| #### Important Note | |||
|
|
|||
| If you have multiple `GoogleMapReact` components in project and you want to use heatmap layer so provide `heatmapLibrary={true}` for all `GoogleMapReact` components so component will load heatmap library at the beginning with google map api. | |||
| If you have multiple `GoogleMapReact` components in project and you want to use heatmap layer provide `libraries:['visualization']` to `bootstrapURLKeys` so that the visualization library will be loaded with the google map api. | |||
There was a problem hiding this comment.
I think we should do something like:
If you have multiple instances of the map in your project and you are willing to use the heatmap layer in at least one of them, make sure to provide
libraries:['visualization']tobootstrapURLKeysso that the visualization library will be loaded with the google map api.
Maybe we could ask @NerdCowboy or @karldanninger to help us with the wording as they're native
There was a problem hiding this comment.
Oh a native touch could be good! wording docs is always tricky.
There was a problem hiding this comment.
Also: @itsmichaeldiego you have a better perspective of the project so I assume your wording reflects the use-cases related better. On my part I just wanted the libraries prop from bootstrapURLKeys to not be "lost between the chairs" of the docs.
We wait for some english natives touch and i edit it. 🎉
There was a problem hiding this comment.
I've updated it with my recommendations in case they take some time to respond, I will merge it if we don't get an answer in the following days.
I will hit up @karldanninger in a PM to check if he will be available soon!
There was a problem hiding this comment.
Updated! Let me know what you think @karldanninger @aviyadeveloper
There was a problem hiding this comment.
looks great @itsmichaeldiego @karldanninger 🚀
There was a problem hiding this comment.
@aviyadeveloper Did you test everything thoroughly? I need some time to give it a test and I will merge it and release after that
There was a problem hiding this comment.
@itsmichaeldiego I would prefer you testing it than me. Since we already had the problems in the last PR and as I am new to the code-base and also pretty busy with work. When I have more time I will check out the testing mechanism and familiarize myself with that process better for future PRs to be stricter and more reliable. But this one should definitely pass your hands first.
There was a problem hiding this comment.
Of course! I will test it thoroughly this time, just wondering if you were able to check those!
| // Support for older version using heatMapLibrary option | ||
| if (heatmapLibrary) { | ||
| bootstrapURLKeys.libraries | ||
| ? bootstrapURLKeys.libraries.append('visualization') |
There was a problem hiding this comment.
Here you're appending even if the library is present!
There was a problem hiding this comment.
because duplicates get cleaned out anyways in:
if (bootstrapURLKeys.libraries) {
bootstrapURLKeys.libraries = bootstrapURLKeys.libraries.filter(
(lib, i) =>
bootstrapURLKeys.libraries.indexOf(lib) === i &&
googleMapsLibs.includes(lib)
);
}
|
@aviyadeveloper Hey! I created a new PR with parts of your PR and some fixes, here it is #946, I kindly ask you if you can review it and let me know if its okey! Thanks. |
|
@aviyadeveloper Merged in #946 and Released in version |
Fixes #922