Add support for marker-color GeoJSON property#849
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request adds support for the GeoJSON Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/leaflet/FeatureBuilder.js`:
- Around line 153-166: The code assumes feature.properties exists when building
point markers (in the createMarker call and when reading
feature.properties['marker-color']), which throws if properties is
null/undefined; update the point rendering in FeatureBuilder.js to guard by
using a safe properties object (e.g., const props = feature.properties || {}) or
optional chaining before passing values to createMarker and to
maps.leaflet.GeoJson.popupContentFromProperties, and use props['marker-color']
when creating the colored icon (ensure title/text defaulting to '' when absent).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b429166-9823-4b99-b6f1-d32b3579d638
📒 Files selected for processing (6)
INSTALL.mdRELEASE-NOTES.mdextension.jsonresources/leaflet/FeatureBuilder.jsresources/leaflet/GeoJson.jsresources/leaflet/LeafletEditor.js
| let marker = createMarker( | ||
| { | ||
| lat: latlng.lat, | ||
| lon: latlng.lng, | ||
| title: feature.properties.title || '', | ||
| text: maps.leaflet.GeoJson.popupContentFromProperties(feature.properties), | ||
| icon: '' | ||
| }, | ||
| options | ||
| ); | ||
|
|
||
| let color = feature.properties && feature.properties['marker-color']; | ||
| if (color) { | ||
| let icon = maps.leaflet.GeoJson.createColoredIcon(L, color); |
There was a problem hiding this comment.
Guard against null/absent feature.properties in point rendering.
Line 157 and Line 158 can throw when a Point feature has properties: null (or missing), which can break marker rendering for the layer.
💡 Suggested fix
pointToLayer: function(feature, latlng) {
+ let properties = feature.properties || {};
let marker = createMarker(
{
lat: latlng.lat,
lon: latlng.lng,
- title: feature.properties.title || '',
- text: maps.leaflet.GeoJson.popupContentFromProperties(feature.properties),
+ title: properties.title || '',
+ text: maps.leaflet.GeoJson.popupContentFromProperties(properties),
icon: ''
},
options
);
- let color = feature.properties && feature.properties['marker-color'];
+ let color = properties['marker-color'];
if (color) {
let icon = maps.leaflet.GeoJson.createColoredIcon(L, color);
if (icon) {
marker.setIcon(icon);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let marker = createMarker( | |
| { | |
| lat: latlng.lat, | |
| lon: latlng.lng, | |
| title: feature.properties.title || '', | |
| text: maps.leaflet.GeoJson.popupContentFromProperties(feature.properties), | |
| icon: '' | |
| }, | |
| options | |
| ); | |
| let color = feature.properties && feature.properties['marker-color']; | |
| if (color) { | |
| let icon = maps.leaflet.GeoJson.createColoredIcon(L, color); | |
| pointToLayer: function(feature, latlng) { | |
| let properties = feature.properties || {}; | |
| let marker = createMarker( | |
| { | |
| lat: latlng.lat, | |
| lon: latlng.lng, | |
| title: properties.title || '', | |
| text: maps.leaflet.GeoJson.popupContentFromProperties(properties), | |
| icon: '' | |
| }, | |
| options | |
| ); | |
| let color = properties['marker-color']; | |
| if (color) { | |
| let icon = maps.leaflet.GeoJson.createColoredIcon(L, color); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/leaflet/FeatureBuilder.js` around lines 153 - 166, The code assumes
feature.properties exists when building point markers (in the createMarker call
and when reading feature.properties['marker-color']), which throws if properties
is null/undefined; update the point rendering in FeatureBuilder.js to guard by
using a safe properties object (e.g., const props = feature.properties || {}) or
optional chaining before passing values to createMarker and to
maps.leaflet.GeoJson.popupContentFromProperties, and use props['marker-color']
when creating the colored icon (ensure title/text defaulting to '' when absent).
Implements the marker-color property from the simplestyle spec for GeoJSON Point features. Markers with a marker-color property are rendered as colored SVG pin icons. The color value is validated as a hex color to prevent SVG injection. Closes #848
JeroenDeDauw
force-pushed
the
marker-color-support
branch
from
6987845 to
cad30c0
Compare
1 participant
marker-colorproperty from the simplestyle spec for GeoJSON Point featuresmarker-colorhex value are rendered as colored SVG pin icons#display_mapGeoJSON embeds, GeoJSON page view, and the visual editorCloses #848
also works in the editor:
Summary by CodeRabbit
New Features
marker-colorproperty in point features for customized map markers.Documentation