input box prototype

[deepanshu44]

Added a prototype input box. Fixes #10

[negillett]

@deepanshu44, to pass the DCO check you'll need to sign the commit (e.g., git commit -s).

[deepanshu44]

@deepanshu44, to pass the DCO check you'll need to sign the commit (e.g., git commit -s).

@negillett Done

[deepanshu44]

@negillett I've pushed some changes and it includes:

• A new component file CoordinatesInput.js
• I've copy pasted the newMap.on("click") logic on the submit button's onClick handler, as I don't know what should be the right approach.

Waiting for your input. And also sorry for the delay.

[joemoorhouse]

Hi @deepanshu44, That behaviour's indeed what we want - thanks! - but as discussed please can we make it more a search box input (i.e. single text entry)? As discussed in the issue, the MUI TextBox is one way to go but in fact the approach in the link that you identified looks like a much better option: https://docs.mapbox.com/mapbox-gl-js/example/mapbox-gl-geocoder-accept-coordinates/ I suggest we use exactly this if that's ok (I thought that was what you were suggesting actually in which case seems a great idea)? It seems to provide a great-looking search feature built into the map and easy to add the geo-coding. By the way, I made some changes in the meantime that cause a conflict, but just necessary to use mapRef.current in place of map. Some of the changes fix a bug that may cause issue on certain mobile browsers so interesting to see if that resolved the issue you were seeing. Thanks, Joe

[deepanshu44]

@joemoorhouse I've implemented the geocoding approach as you can see below

Image


Now I think it would be better to remove the first approach for the input box as I believe you favor the geocoding approach and it also looks more user friendly.

About the mobile issue, I still have it. I think the issue is with the server npm start, as the demo link you've attached was correctly displayed.

[joemoorhouse]

Thanks... yes looks good! And I agree: please go ahead and remove the first approach. Good to hear that sandbox works on your mobile anyway.

On Tue, 2 Aug 2022, 11:33 deepanshu44, @.***> wrote:

@joemoorhouse https://github.com/joemoorhouse I've implemented the geocoding approach as you can see below Image

[image: Screenshot_20220802-153359_Firefox] https://user-images.githubusercontent.com/62979448/182351990-d5050166-47ec-4580-acac-47376313eba5.jpg

Now I think it would be better to remove the first approach for the input box as I believe you favor the geocoding approach and it also looks more user friendly.

About the mobile issue, I still have it. I think the issue is with the server npm start, as the demo link you've attached was correctly displayed.

— Reply to this email directly, view it on GitHub https://github.com/os-climate/physrisk-ui/pull/38#issuecomment-1202308913, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG5YQF6W6HUF7RUKE4FV7DVXD2NXANCNFSM54PMD34A . You are receiving this because you were mentioned.Message ID: @.***>

[deepanshu44]

Hello @joemoorhouse I am getting a bit confused here. Do we really need to add geocoder on the map? As the requirement of this issue was to just implement a simple lat/lon search box. But adding geocoder will also let the user to search for places using simple words (eg. Germany). Is that an overkill? But I think typing "Germany" should also provide the lat/lon for Germany. Please clarify. Thanks

[joemoorhouse]

Hi @deepanshu44, no all good! This issue was indeed for lat/lon only but we also want geocoding and this change gives us both, so great. Makes sense as actually harder not to implement together.

[joemoorhouse]

By the way, are the indents missing from CoordinatesInput.js (and if so was some Prettier setting?)?

[deepanshu44]

Hello @joemoorhouse, I have pushed some changes. Everything seems to be working correctly. Earlier when I added geocoder, we had two markers (one from the user click and the other from geocoder). So in order to resolve this problem, I added false to marker property in geocoder and attatched "result" event listener to the same which will add the marker.

[deepanshu44]

Also thanks @negillett for approving and letting me work on this issue🙂.

[joemoorhouse]

That's great - thanks! Was going to suggest using the result event but you got there first :)

On Wed, 3 Aug 2022, 10:11 deepanshu44, @.***> wrote:

Hello @joemoorhouse https://github.com/joemoorhouse, I have pushed some changes. Everything seems to be working correctly. Earlier when I added geocoder, we had two markers (one from the user click and the other from geocoder). So in order to resolve this problem, I added false to marker property in geocoder and attatched "result" event listener to the same.

— Reply to this email directly, view it on GitHub https://github.com/os-climate/physrisk-ui/pull/38#issuecomment-1204004327, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG5YQDIHW5AVRJQGFQAEDTVXJ4ZVANCNFSM54PMD34A . You are receiving this because you were mentioned.Message ID: @.***>

[joemoorhouse]

Hi @deepanshu44; hi @negillett, sorry just back from holiday and did not notice that this is approved but not merged. @negillett, please feel free to merge changes as well if you are happy by the way! @deepanshu44 thanks again for this (and are you on OS-C Slack actually?)

[deepanshu44]

Thanks @joemoorhouse for merging PR. It was my pleasure to volunteer for this awesome project.