dapriett / nativescript-google-maps-sdk

Cross Platform Google Maps SDK for Nativescript
MIT License
244 stars 164 forks source link

fix: Button My Location not working on IOS #446

Closed kefahB closed 3 years ago

kefahB commented 3 years ago

Hi every body,

Finally I found the issue for the MyLocation button on IOS .. it was very simple :D there is no need to CLLocationManagerDelegateat all, just didTapMyLocationButtonForMapView should return false instead of true to run the default behavior "centering map on user position" .

I Pushed this PR because I work on the builder issue, I'll push all commit to my fork, like this if any can help that will be cool.

@dapriett @funder7 @burgov

funder7 commented 3 years ago

Nice find @kefahB!

Sorry I did not understand on what do you need help. If it's not a too long task, maybe I can do something :)

kefahB commented 3 years ago

@dapriett there is an issue with this npm package it is related with #444 this issue it is about NativeClass() it come from build command I think. I've updated this commande with the last PR ts-patch install, can you please merge this and run those before publishing the new version : rm -Rf node_modules npm install npm run build && tsc

If all work fine we shouldn't see this on map-view-.ios.js (this code from the npm 3.0.1) :

IndoorDisplayDelegateImpl.ObjCProtocols = [GMSIndoorDisplayDelegate];
IndoorDisplayDelegateImpl = IndoorDisplayDelegateImpl_1 = __decorate([
    NativeClass()
], IndoorDisplayDelegateImpl);

but this instead :

IndoorDisplayDelegateImpl.ObjCProtocols = [GMSIndoorDisplayDelegate];
    return IndoorDisplayDelegateImpl;
MrSnoozles commented 3 years ago

I can confirm that the changes in lines 219-222 fixes the MyLocation button on iOS. 🎉

@kefahB

I've updated this commande with the last PR ts-patch install, can you please merge this and run those before publishing the new version :

I'm not sure the instructions are clear (at least not for me, sorry :) ). Can you clarify which "command" and which "last PR" you mean (link them maybe)? When you say "merge this" do you mean the PR we're currently in ?

kefahB commented 3 years ago

Hi @MrSnoozles,

Thanks for your confirmation.

Sorry my english is very bad :)

I mean the command from the package.json => this line the ts-patch will normally resolve the issue about ReferenceError: Can't find variable: NativeClass #444.

The last PR that I've pushed it's the #440

Yes it will be cool to merge this PR because it contain the correction of the Mylocation button :)

MrSnoozles commented 3 years ago

@dapriett Would it be possible to merge, compile and bump the version number so iOS users can enjoy your incredible plugin in all its {N}7 glory too? 😀

abhayastudios commented 2 years ago

@dapriett as of now v3.0.2 still does not seems to solve the ReferenceError: Can't find variable: NativeClass error, but replacing the map-view.ios.js that @kefahB provided in #444 directly in node_modules/nativescript-google-maps.sdk does seems to work.

Could it be that it was necessary to run npm run build before releasing the latest npm module that you published after merging this commit? If so, would you be so kind as to publish a newer version that fixes this, so the community can enjoy this plugin again without needing to go through hoops to make it work on iOS?

Cheers!