Skip to content
This repository was archived by the owner on Dec 15, 2024. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,20 @@ Valid values: string: [ Point, LineString, Polygon ].<br>
: Specifies the geometry data for each feature. <br>
Valid format: array of coordinates for Point and LineString, array of arrays of coordinates for Polygon.

`#!css data-icon`
: Specifies an icon to use with Point geometries. See Leaflet documentation for [icon](https://leafletjs.com/reference.html#icon). <br>
```html
<span ... data-icon='{"iconUrl": "/url/icon.png", "iconSize": [38, 38]}'></span>
```
Valid format: JSON representing L.icon settings.

`#!css data-options`
: Specifies Polyline and Polygon options. See Leaflet documentation for [Polyline options](https://leafletjs.com/reference.html#polyline-option) and [Polygon options](https://leafletjs.com/reference.html#polygon). <br>
```html
<span ... data-options='{"color": "red", "weight": 1, "opacity": 0.8}'></span>
```
Valid format: JSON representing options compatible with L.polyline and L.polygon leaflet methods.
Comment on lines +178 to +183
Copy link

Choose a reason for hiding this comment

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

Ensure consistency in the presentation of JSON data in documentation.

The JSON data in the data-options attribute example is not formatted consistently with other similar examples in the document. Consider formatting it with proper indentation to improve readability and maintain consistency across the documentation.

- <span ... data-options='{"color": "red", "weight": 1, "opacity": 0.8}'></span>
+ <span ... data-options='{ "color": "red", "weight": 1, "opacity": 0.8 }'></span>

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.

Suggested change
`#!css data-options`
: Specifies Polyline and Polygon options. See Leaflet documentation for [Polyline options](https://leafletjs.com/reference.html#polyline-option) and [Polygon options](https://leafletjs.com/reference.html#polygon). <br>
```html
<span ... data-options='{"color": "red", "weight": 1, "opacity": 0.8}'></span>
```
Valid format: JSON representing options compatible with L.polyline and L.polygon leaflet methods.
`#!css data-options`
: Specifies Polyline and Polygon options. See Leaflet documentation for [Polyline options](https://leafletjs.com/reference.html#polyline-option) and [Polygon options](https://leafletjs.com/reference.html#polygon). <br>
```html
<span ... data-options='{ "color": "red", "weight": 1, "opacity": 0.8 }'></span>

Valid format: JSON representing options compatible with L.polyline and L.polygon leaflet methods.


</details>
<!-- suggestion_end -->

<!-- This is an auto-generated comment by CodeRabbit -->


`#!css data-popup`
: Specifies the [popup](https://leafletjs.com/reference.html#popup) content for each feature. Use in combination with other `data-*` attributes. <br>
```html
Expand Down
13 changes: 11 additions & 2 deletions src/Geometry/hyperleaflet-geometry-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function deleteNodeFromHyperleaflet(node, map) {
}

function changeNodeInHyperleaflet(change, map) {
const rowId = change.changes['data-id'];
const rowId = change['data-id'];
// eslint-disable-next-line no-underscore-dangle
const leafletLayers = Object.values(map._layers);
const leafletObject = leafletLayers.find((layer) => layer.hlID === rowId);
Expand All @@ -50,8 +50,17 @@ export default function hyperleafletGeometryHandler(map, { addCallback = () => {
}

function changeNodesInHyperleaflet(changes) {
// changes is an array of changes and a dataset
changes.forEach((change) => {
changeNodeInHyperleaflet(change, map);
// NOTE: Some changes have shape { node, changes }, but these seem to be related to add/remove
// lifecycle events.
const { dataset, ...partialChanges } = change;
if (typeof dataset !== 'undefined') {
// Handle { dataset, { i: change } } format
Object.values(partialChanges).forEach((partialChange) => {
changeNodeInHyperleaflet({ ...partialChange, dataset }, map);
});
}
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/Geometry/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function hyperleafletDataToMap(map) {
hyperChangeDetection.observe({
targetSelector: HYPERLEAFLET_DATA_SOURCE,
uniqueAttribute: 'data-id',
attributeFilter: ['data-geometry'],
attributeFilter: ['data-geometry', 'data-options'],
});

hyperChangeDetection.subscribe(HYPERLEAFLET_DATA_SOURCE, 'node_adds', (data) => {
Expand Down
40 changes: 31 additions & 9 deletions src/Geometry/leaflet-geometry.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import { GeoJSON, marker, polygon, polyline } from 'leaflet';
import { GeoJSON, icon, marker, polygon, polyline } from 'leaflet';
import { setPointEvents, setPolyGeometryEvents } from './events';
import hyperleafletConfig from '../config';

const createPointGeometry = (parsedGeometry, options) => {
const { reverseOrderAll, reverseOrder } = options;
const { reverseOrderAll, reverseOrder, icon: iconSettings } = options;
const isLonLat = reverseOrderAll || reverseOrder !== undefined;
const geometry = isLonLat ? [...parsedGeometry].reverse() : parsedGeometry;
const leafletGeometry = marker(geometry);
let leafletGeometry;
if (options.icon) {
leafletGeometry = marker(geometry, { icon: icon(JSON.parse(options.icon)) });
} else {
leafletGeometry = marker(geometry);
}
if (options.popup) {
leafletGeometry.bindPopup(options.popup);
}
Expand All @@ -26,10 +31,10 @@ function changePointGeometry(leafletObject, parsedGeometry, options) {
}

const createLineGeometry = (parsedGeometry, options) => {
const { reverseOrderAll, reverseOrder } = options;
const { reverseOrderAll, reverseOrder, options: polylineOptions } = options;
Copy link

Choose a reason for hiding this comment

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

The destructuring of options to extract polylineOptions is repeated in both createLineGeometry and createPolygonGeometry. This repetition could be reduced by refactoring the common logic into a utility function or by ensuring that the options object is structured appropriately before being passed to these functions.

Consider refactoring the common logic for handling options to improve code maintainability and reduce repetition.

const isLonLat = reverseOrderAll || reverseOrder !== undefined;
const geometry = isLonLat ? GeoJSON.coordsToLatLngs(parsedGeometry, 0) : parsedGeometry;
const leafletGeometry = polyline(geometry);
const leafletGeometry = polyline(geometry, polylineOptions);
Comment on lines +34 to +37
Copy link

Choose a reason for hiding this comment

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

Refactor the handling of polyline options to reduce code duplication between line and polygon geometries.

- const { reverseOrderAll, reverseOrder, options: polylineOptions } = options;
+ const { reverseOrderAll, reverseOrder, polylineOptions } = extractPolylineOptions(options);

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.

Suggested change
const { reverseOrderAll, reverseOrder, options: polylineOptions } = options;
const isLonLat = reverseOrderAll || reverseOrder !== undefined;
const geometry = isLonLat ? GeoJSON.coordsToLatLngs(parsedGeometry, 0) : parsedGeometry;
const leafletGeometry = polyline(geometry);
const leafletGeometry = polyline(geometry, polylineOptions);
const { reverseOrderAll, reverseOrder, polylineOptions } = extractPolylineOptions(options);
const isLonLat = reverseOrderAll || reverseOrder !== undefined;
const geometry = isLonLat ? GeoJSON.coordsToLatLngs(parsedGeometry, 0) : parsedGeometry;
const leafletGeometry = polyline(geometry, polylineOptions);

if (options.popup) {
leafletGeometry.bindPopup(options.popup);
}
Expand All @@ -49,10 +54,10 @@ function changeLineGeometry(leafletObject, parsedGeometry, options) {
}

const createPolygonGeometry = (parsedGeometry, options) => {
const { reverseOrderAll, reverseOrder } = options;
const { reverseOrderAll, reverseOrder, options: polylineOptions } = options;
const isLonLat = reverseOrderAll || reverseOrder !== undefined;
const geometry = isLonLat ? GeoJSON.coordsToLatLngs(parsedGeometry, 1) : parsedGeometry;
const leafletGeometry = polygon(geometry);
const leafletGeometry = polygon(geometry, polylineOptions);
if (options.popup) {
leafletGeometry.bindPopup(options.popup);
}
Expand Down Expand Up @@ -105,19 +110,36 @@ function changeGeometry(leafletObject, change) {
}
}

function changeOptions(leafletObject, change) {
const { to: options } = change;
return leafletObject.setStyle(JSON.parse(options));
}
Comment on lines +113 to +116
Copy link

Choose a reason for hiding this comment

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

Refine the implementation of changeOptions to include error handling for JSON parsing and ensure compatibility of options with setStyle.

function changeOptions(leafletObject, change) {
  const { to: options } = change;
+ try {
+   const parsedOptions = JSON.parse(options);
+   return leafletObject.setStyle(parsedOptions);
+ } catch (error) {
+   console.error('Failed to parse options:', error);
+   return leafletObject; // Or handle the error as appropriate
+ }
}

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.

Suggested change
function changeOptions(leafletObject, change) {
const { to: options } = change;
return leafletObject.setStyle(JSON.parse(options));
}
function changeOptions(leafletObject, change) {
const { to: options } = change;
try {
const parsedOptions = JSON.parse(options);
return leafletObject.setStyle(parsedOptions);
} catch (error) {
console.error('Failed to parse options:', error);
return leafletObject; // Or handle the error as appropriate
}
}


export function createLeafletObject(dataset) {
const { geometry, popup, tooltip, geometryType, id, reverseOrder } = dataset;
const { geometry, popup, tooltip, geometryType, id, reverseOrder, options = '{}', icon } = dataset;
const parsedGeometry = JSON.parse(geometry);
const parsedOptions = JSON.parse(options);
const { reverseOrderAll } = hyperleafletConfig;
const createGeometryFn = createGeometry(geometryType);
return createGeometryFn(parsedGeometry, { popup, tooltip, id, reverseOrderAll, reverseOrder });
return createGeometryFn(parsedGeometry, {
popup,
tooltip,
id,
reverseOrderAll,
reverseOrder,
options: parsedOptions,
icon,
});
}

export function changeLeafletObject(leafletObject, change) {
switch (change.attribute) {
case 'data-geometry': {
return changeGeometry(leafletObject, change);
}
case 'data-options': {
return changeOptions(leafletObject, change);
}
default: {
throw new Error('Parameter is not a number!');
}
Expand Down
6 changes: 5 additions & 1 deletion src/Geometry/test/geometry-object-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ describe('geometryObjectHandler', () => {
},
'2': {
type: 'LineString',
coordinates: [[1, 2], [3, 4], [5, 6]],
coordinates: [
[1, 2],
[3, 4],
[5, 6],
],
},
});
});
Expand Down
51 changes: 51 additions & 0 deletions src/Geometry/test/hyperleaflet-geometry-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,55 @@ describe('createLeafletObject', () => {
expect(polygon.getPopup().getContent()).toEqual('Hello, world!');
expect(polygon.getTooltip().getContent()).toEqual('I am a polygon');
});

it('should create a Leaflet polygon object with polyline options', () => {
const row = {
geometry: '[[[-122.414,37.776],[-122.413,37.775],[-122.413,37.776],[-122.414,37.776]]]',
geometryType: 'Polygon',
id: '123',
reverseOrder: '',
options: '{"color": "blue"}',
};
const polygon = createLeafletObject(row);
expect(polygon).toBeInstanceOf(L.Polygon);
expect(polygon.options).toEqual({ color: 'blue' });
expect(polygon.getLatLngs()).toEqual([
[L.latLng(37.776, -122.414), L.latLng(37.775, -122.413), L.latLng(37.776, -122.413)],
]);
});

it('should create a Leaflet polyline object with polyline options', () => {
const row = {
geometry: '[[-122.414,37.776],[-122.413,37.775]]',
geometryType: 'LineString',
id: '123',
options: '{"weight": 1}',
};
const polyline = createLeafletObject(row);
expect(polyline).toBeInstanceOf(L.Polyline);
expect(polyline.options).toEqual({ weight: 1 });
expect(polyline.getLatLngs()).toEqual([L.latLng(-122.414, 37.776), L.latLng(-122.413, 37.775)]);
});

it('should change Leaflet polyline object given new options', () => {
const row = {
geometry: '[[-122.414,37.776],[-122.413,37.775]]',
geometryType: 'LineString',
id: '123',
options: '{"weight": 1}',
};
const polyline = createLeafletObject(row);
expect(polyline).toBeInstanceOf(L.Polyline);
expect(polyline.options).toEqual({ weight: 1 });

changeLeafletObject(polyline, {
from: '{"weight": 1}',
to: '{"weight": 2}',
attribute: 'data-options',
'data-id': 123,
dataset: { geometry: '[-122.414,37.776]', geometryType: 'Point', id: '123' },
});
expect(polyline).toBeInstanceOf(L.Polyline);
expect(polyline.options).toEqual({ weight: 2 });
});
});