react-native-map-link icon indicating copy to clipboard operation
react-native-map-link copied to clipboard

v2.12.0 (issue210-address-as-parameter) - Adding ADDRESS as a potential parameter

Open trevorrd opened this issue 1 year ago • 3 comments

This PR is to address issue210-address-as-parameter. I've made the changes to now allow developers to use an address instead of latitude and longitude coordinates. This is what I had commented on earlier and you, Thomas Schoffelen, had commented and said, It's not a use case I personally have, because I want to send users to a specific location rather than doing a search, but I can understand why that would be useful. I'm still happy to accept a PR for that functionality if you'd like to add it!

So here is my pull request. Please review it, give me any feedback, if you see anything I need to update or change please let me know.

trevorrd avatar Mar 01 '24 22:03 trevorrd

I just noticed you have another PR that is going to convert this library into TypeScript (v3.0.0). If you want to do that first then we can go in and make similar changes we can do that too.

trevorrd avatar Mar 01 '24 22:03 trevorrd

Hi @trevorrd - apologies for the delay! The Typescript migration has now been completed, so if you're willing to update this, I'm happy to get this merged. Apologies for the duplicated work!

tschoffelen avatar Mar 29 '24 18:03 tschoffelen

Hi @trevorrd - apologies for the delay! The Typescript migration has now been completed, so if you're willing to update this, I'm happy to get this merged. Apologies for the duplicated work!

Hey Thomas (@tschoffelen), I've made the changes and pushed them up. This PR should be ready for a review. I've also been pretty busy in the last few weeks but I still feel these changes would be very beneficial.

Thanks!

trevorrd avatar Apr 19 '24 23:04 trevorrd

It's taken me way too long (apologies again), but it's merged! Thanks again @trevorrd for the amazing work on this, quite a big new feature you've shipped here!

tschoffelen avatar May 14 '24 07:05 tschoffelen

:tada: This PR is included in version 3.4.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar May 14 '24 07:05 github-actions[bot]

Hi,

Is anybody able to confirm this works with google maps on ios?

Every time we use the address field it, then launch maps it gets "google maps cant open this link"

image

IMG_9A05AED0C74A-1

OwenMelbz avatar May 24 '24 09:05 OwenMelbz

Update:

After some debugging it looks like the url structure is wrong.

Referring to the google docs here: https://developers.google.com/maps/documentation/urls/get-started it says all urls should have api=1 in, which we can see is used later on:

image

However for the address line it instead uses ?q=$address

image

whereas the docs about says it should be ?api=1&query=$address upon changing that, it worked, so i believe there is a bug there

image

PR to fix is here: https://github.com/includable/react-native-map-link/pull/294

Would be great to get this merged asap if possible as we caught this issue during testing for a big app update which meant to be going out today :D

OwenMelbz avatar May 24 '24 10:05 OwenMelbz

Update:

After some debugging it looks like the url structure is wrong.

Referring to the google docs here: https://developers.google.com/maps/documentation/urls/get-started it says all urls should have api=1 in, which we can see is used later on:

image However for the address line it instead uses `?q=$address` image whereas the docs about says it should be `?api=1&query=$address` upon changing that, it worked, so i believe there is a bug there image PR to fix is here: #294

Would be great to get this merged asap if possible as we caught this issue during testing for a big app update which meant to be going out today :D

Nice catch! Yes, open a bug ticket. I'm not sure how I missed that small query string parameter. I know I used it in my project for work and must have missed it as I was adding it to this library. If we could get the ticket opened and change that, it would be great. Thanks!

trevorrd avatar May 24 '24 15:05 trevorrd

There's already a PR up for it here: https://github.com/includable/react-native-map-link/pull/294

OwenMelbz avatar May 24 '24 15:05 OwenMelbz