-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat: adding ember leaflet #5429
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/j7futdjkm |
Codecov Report
@@ Coverage Diff @@
## development #5429 +/- ##
===============================================
- Coverage 23.25% 23.24% -0.01%
===============================================
Files 493 494 +1
Lines 5182 5162 -20
Branches 38 38
===============================================
- Hits 1205 1200 -5
+ Misses 3972 3957 -15
Partials 5 5
Continue to review full report at Codecov.
|
Really cool. I'll check what's the issue once I'm free |
I don't understand. Do you mean it is showing wrong location for a latitude and longitude? Then try switching lat and lng |
{{/g-map-address-marker}} | ||
</GMap> | ||
{{/if}} | ||
<LeafletMap @lat={{this.lat}} @lng={{this.lng}} @zoom={{this.zoom}} as |layers|> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be shown only if event has lat lng. Otherwise, fallback to previous method
</GMap> | ||
{{/if}} | ||
<LeafletMap @lat={{this.lat}} @lng={{this.lng}} @zoom={{this.zoom}} as |layers|> | ||
<layers.tile @url="https://{s}.basemaps.cartocdn.com/light_all/{z}/{x}/{y}.png"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a free to use basemap, this is only free for non-commercial projects
Our basemaps can be used for free in applications and visualizations for non-commercial use. For commercial purposes, you will need an Enterprise license to ensure you have the best basemaps available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
{{#if (eq this.mapConfig.display 'embed') }} | ||
<iframe title="Map" class="g-map" src="https://maps.google.com/maps?q={{this.event.locationName}}&t=&z=15&ie=UTF8&iwloc=&output=embed" frameborder="0" scrolling="no"></iframe> | ||
{{else}} | ||
<GMap @markersFitMode="live" @lat={{37.744}} @lng={{-122.4367}} @address={{this.event.locationName}} @zoom={{2}} @class="google-maps" as |context|> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this cause our goal is to remove ember-g-map
app/components/public/event-map.js
Outdated
@@ -11,7 +11,7 @@ export default class EventMap extends Component { | |||
|
|||
@tracked | |||
lng = this.event.longitude; | |||
|
|||
@tracked | |||
zoom = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is read only?, it should not have tracked decorator
app/components/public/event-map.js
Outdated
@@ -11,7 +11,7 @@ export default class EventMap extends Component { | |||
|
|||
@tracked | |||
lng = this.event.longitude; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read only properties don't need tracked
package.json
Outdated
@@ -165,6 +166,7 @@ | |||
}, | |||
"private": true, | |||
"dependencies": { | |||
"ember-leaflet": "^4.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be in devDependencies?
@maze-runnar What's missing here? |
I still don't understand |
i mean lat and lng values are not coming from backend. Map is showing correctly them on putting any lat and lng manually. |
Try on this https://eventyay.com/e/45da88b7 |
showing correctly - |
Zoom level is very bad, that's why |
But it's with Fixed that for you. It's paid, not free |
There were several things wrong and missing:
|
You also did not follow It also would have automatically installed leaflet in devDependencies |
Fixes #591
Checklist
development
branch.