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

[Feature] Location Google Maps view #4390

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lopezjurip
Copy link

@lopezjurip lopezjurip commented Jul 22, 2017

Description of changes

Added a Google Map view for Location field.

  • It requires google api key for displaying it.
  • Built with https://github.com/tomchentw/react-google-maps.
  • Supports lazy and asynchronous loading of the vendor google maps scripts.
  • Left click marks a spot and sets the lat/lng attributes.
  • Right click on any place removes the marked spot.

preview

Issues

If we only set the geo values, the Autodetect and improve location on save is not working.

Testing

  • Please confirm npm run test-all ran successfully.

This test is not passing on my machine. Maybe I have not the correct setup.

1) language must allow Accept-Language selection:

      AssertionError: "en-US" must be equivalent to "zh-CN"
      + expected - actual

      -en-US
      +zh-CN

      at Context.<anonymous> (test/unit/lib/middleware/language.js:79:34)

@lopezjurip lopezjurip changed the title Feature/location google maps [Feature] Location Google Maps view Jul 22, 2017
@Noviny
Copy link
Contributor

Noviny commented Sep 22, 2017

This looks sweet. So happy to see you documented the usage as well.

A couple of small things. What happens when no google maps api key is provided? I think we want to section off the map itself much more firmly, so I would move all logic around map state into AsyncGoogleMaps, including everything going in to the renderMap function.

Then line 378 can become:

{browserApiKey ? <AsyncGoogleMaps browserApiKey={browserApiKey} /> : null}

Does this update make sense to you, to ensure maps are hidden when they should be?

@Noviny Noviny self-assigned this Sep 24, 2017
@tomcardoso
Copy link

👍
FWIW, would love to see this merged in.

@radames
Copy link
Contributor

radames commented Feb 28, 2018

it works like a charm, just need to use the old react-google-maps version 6.3.0, the newest version seems not working

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

Successfully merging this pull request may close these issues.

5 participants