fix(assets): serve leaflet marker icons from the leaflet package - #289
Open
janni1288 wants to merge 1 commit into
Open
fix(assets): serve leaflet marker icons from the leaflet package#289janni1288 wants to merge 1 commit into
janni1288 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #289 +/- ##
==============================================
+ Coverage 85.97% 100.00% +14.02%
Complexity 9766 9766
==============================================
Files 521 521
Lines 37782 37762 -20
==============================================
+ Hits 32483 37762 +5279
+ Misses 5299 0 -5299
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Several components (
leaflet-map-manager.js,map-adapter/leaflet.js,map-adapter/google.js,order-tracking-lookup.js,customer/create-order-form.js,customer/orders.js) referenceassets/images/marker-icon.png,marker-icon-2x.pngandmarker-shadow.pngfor Leaflet's default markers. These files were never added to the addon's public assets, so the browser gets 404s and shows broken image icons on the map.Why this approach
Rather than committing copies of these images into the repo (which would drift out of sync with the installed Leaflet version over time), this serves them directly from the already-installed
leafletpackage via a broccoli-funnel tree - the same pattern already used inindex.jsforleaflet-drawandleaflet-contextmenu's dist assets.Changes
index.js: newtreeForLeafletImages()funnelsmarker-icon.png,marker-icon-2x.png, andmarker-shadow.pngfromleaflet/dist/imagesintoassets/images, wired intomergeWithPublicTreepackage.json: addedleafletas a direct dependency - it was previously only resolved transitively vialeaflet-draw/leaflet-contextmenu, which causedresolve.syncto fail when referenced directlyTesting
/assets/images/marker-icon.png/assets/images/marker-icon-2x.png/assets/images/marker-shadow.pngVerification of scope
Searched the addon for all
/assets/images/*references to check whetherother assets were missing besides the three named in the issue — these
three were the only ones referenced in code but missing from the assets
folder, so this should be the complete fix.
Closes #211