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

fix: Fixed maps on events & explore page #3271

Merged
merged 9 commits into from
Jul 31, 2019

Conversation

mrsaicharan1
Copy link
Member

@mrsaicharan1 mrsaicharan1 commented Jul 12, 2019

Fixes #3159
Fixes #3270

Short description of what this resolves:

Fixes the map issue by allowing the Maps API to handle the location->(lat,lng) conversion (Event Page)

Screenshot 2019-07-12 at 12 51 15 PM

Refactored the map component to ensure that it is dynamic enough to pan to the correct map bounds. (Explore Page)
Screenshot 2019-07-12 at 1 42 52 PM

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@auto-label auto-label bot added the fix label Jul 12, 2019
@mrsaicharan1 mrsaicharan1 force-pushed the map-update branch 2 times, most recently from 72bfec2 to 095d1ee Compare July 12, 2019 07:51
.env.example Outdated Show resolved Hide resolved
@@ -1,8 +1,12 @@
{{#if event.isMapShown}}
<div class="eight wide column event-map">
<h1>{{t 'Getting Here'}}</h1>
{{#g-map lat=event.latitude lng=event.longitude zoom=15 gestureHandling='cooperative' streetView='StreetViewPanorama' as |context|}}
{{g-map-marker context lat=event.latitude lng=event.longitude}}
{{#g-map markersFitMode='live' lat=37.744 lng=-122.4367 address=event.locationName zoom=2 class='google-maps' as |context|}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have hardcoded lat/lng here ?

Copy link
Member Author

@mrsaicharan1 mrsaicharan1 Jul 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the hardcoded lat/lng gives an empty map. According to the docs, there seems to be an initial value given to the component everywhere.
https://www.emberobserver.com/addons/ember-g-map
There needs to be an initial value given I guess.

@@ -1,5 +1,5 @@
<div class="map item">
{{#g-map lat=37.7833 lng=-122.4167 zoom=2 address=location class='google-maps' as |context|}}
{{#g-map markersFitMode='live' lat=37.7833 lng=-122.4167 zoom=2 address=location class='google-maps' as |context|}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove hardcoded lat/lng

Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrsaicharan1 please update your branch. I think netlify is fixed now so tests can run properly
Also, one thing which few devs faced earlier was during event creation. The lat and long. of the event was not getting stored instantly and so the map was not getting rendered. Is it still persistent?

@mrsaicharan1
Copy link
Member Author

@mrsaicharan1 please update your branch. I think netlify is fixed now so tests can run properly
Also, one thing which few devs faced earlier was during event creation. The lat and long. of the event was not getting stored instantly and so the map was not getting rendered. Is it still persistent?

Done! Yeah, It's not getting stored there. I will open a separate issue for this & work on it.

@mrsaicharan1 mrsaicharan1 requested a review from niranjan94 July 18, 2019 04:57
@mrsaicharan1
Copy link
Member Author

@uds5501 @shreyanshdwivedi @CosmicCoder96 This is ready for another review

Copy link
Contributor

@uds5501 uds5501 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the hard-coded lat/long aren't any issue, it looks fine

@abhinavk96 abhinavk96 merged commit 3cb7a1b into fossasia:development Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maps do not shift to vicinity of the area Event maps showing wrong location
6 participants