Fix: Vendor Leaflet JS and CSS locally to replace external CDN dependencies#114
Conversation
| <!-- Leaflet Marker Cluster CSS files --> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet.markercluster/1.4.1/MarkerCluster.css" /> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet.markercluster/1.5.0/MarkerCluster.Default.min.css" /> | ||
| <!-- ✅ Leaflet CSS files (vendored locally) --> |
There was a problem hiding this comment.
I don't think use of emojis are needed and/or meaningful here. Personally, I wouldn't even update the comment.
There was a problem hiding this comment.
Apologies for that, sir! I’ve made meaningful updates to the PR based on your suggestions. Thank you for your feedback and response!
| <!-- ✅ Map data setup --> | ||
| <script> | ||
| const singleEventData = {{ .sigleEvent | default "null" }}; | ||
| const eventsData = { | ||
| "type": "FeatureCollection", | ||
| "features": [ | ||
| {{- range $index, $event := $events }} | ||
| { | ||
| "type": "Feature", | ||
| "properties": { | ||
| "name": {{ .Title }}, | ||
| "slug": {{ .Slug }}, | ||
| "date": {{ .Params.event_date }}, | ||
| "end_date": {{ .Params.event_date_end }}, | ||
| "address": {{ if eq .Params.event_type "in_person" }}{{ .Params.venue_address }}{{ else }}{{ .Params.event_type }}{{ end }}, | ||
| "description": {{ .Description }}, | ||
| "event_type": {{ .Params.event_type }}, | ||
| "event_category": {{ .Params.event_category }}, | ||
| "host": {{ .Params.event_host }}, | ||
| "website": {{ .Params.event_url | default .Permalink }}, | ||
| "event_category_label": {{ .Params.event_category }} | ||
| }, | ||
| "geometry": { | ||
| "type": "Point", | ||
| "coordinates": [ | ||
| {{ .Params.longitude }}, | ||
| {{ .Params.latitude }} | ||
| ] | ||
| } | ||
| }{{ if ne (add $index 1) (len $events) }},{{ end }} | ||
| {{- end }} | ||
| ] | ||
| }; | ||
| const singleEventData = {{ .sigleEvent | default "null" }}; |
There was a problem hiding this comment.
Please do not apply any formatting, or cosmetic changes like the comment wording updates. It's better to keep the diff size as small as possible and in the scope of the issue.
README.md
Outdated
|
|
||
| If a new version of Leaflet or MarkerCluster is released, you can update the vendored files using the following PowerShell commands (update version numbers as needed): | ||
|
|
||
| ```powershell |
There was a problem hiding this comment.
I don't think Windows specific instructions are the best way to go here. I'm not even sure these instructions are needed, but if we are going to add something, I'd prefer having commands that uses open source tools - like wget and curl.
| <!-- Leaflet CSS file --> | ||
| <link rel="stylesheet" type="text/css" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" | ||
| integrity="sha256-p4NxAoJBhIIN+hmNHrzRCf9tD/miZyoHS5obTRR9BMY=" crossorigin="" /> | ||
| <!-- Leaflet Marker Cluster CSS files --> |
| <!-- Map --> | ||
| {{ $events := .events }} | ||
| <div id="map" aria-label="Django 20th Anniversary Event Locations"></div> | ||
| <div id="map" aria-label="Django 20th Anniversary Event Locations" style="height: 600px;"></div> |
There was a problem hiding this comment.
Does this change relate to vendored leaflet?
|
Thank you for all the updates. I have two more comments, and they will be the last ones on my side:
|
…prepare for future version updates Fix: vendor Leaflet JS and CSS locally to replace CDN references and prepare for future version updates Fix: vendor Leaflet JS and CSS locally to replace CDN references and prepare for future version updates Fix: vendor Leaflet JS and CSS locally to replace CDN references and prepare for future version updates Fix: vendor Leaflet JS and CSS locally to replace CDN references and prepare for future version updates Final updates before squash
kehach07
force-pushed
the
issue-70-vender-third-party-assets
branch
from
5ab79cd to
f5e96df
Compare
…sue-70-vender-third-party-assets
benjaoming
force-pushed
the
issue-70-vender-third-party-assets
branch
from
d54bca8 to
dc0617f
Compare
benjaoming
left a comment
benjaoming
left a comment
There was a problem hiding this comment.
Amazing! Thanks for getting this in! It works well for me 👍 I made changes so it's clear what versions these originate from.
I don't think we'll be doing much maintenance work on this: From experience, once it's vendored, it stays kind of static until someone takes initiative to update it.
4 participants
This PR addresses issue #70 by vendoring Leaflet’s JS and CSS assets locally within the project.
Summary of Changes:
Added leaflet.js and leaflet.css files under static/vendor/leaflet/
Updated project references to use local paths instead of unpkg CDN URLs
Ensured all Leaflet functionality remains intact using the local assets
Rationale:
Improves long-term reliability by avoiding dependence on external CDNs
Mitigates privacy and security concerns related to third-party hosting
Keeps the project fully functional even in offline or long-term archival contexts
Additional Note:
I have updated the README file to include commands for easily updating to newer Leaflet versions in the future.