Problem/Motivation
Leaflet supports circles as markers and the drawing toolbar provided by the Leaflet Geoman drawing tools used in the leaflet form widget provide a toolbar option for this. The configuration options for the widget show it as a listed option along with a warning that it is not supported by the GeoJSON data format, and the checkbox is disabled preventing it from being ever shown.
I appreciate the label warning that circle and circle marker are not supported by GeoJSON, but I'm not sure disabling the checkbox is the right call. Circle markers are saved as POINT data in GeoJSON, the same as marker pins. For my application I prefer the circle marker to dropping a pin, so I'd prefer to keep that as an option.
I can alter the contents of leaflet maps with post-load javascript to display markers as circles, but with only markers available on the toolbar itself, such markers will be displayed as markers instead of circles while being drawn, which is not really going to work for me.
There is no real support for circles with variable radii, so I think leaving the Circle tool permanently disabled makes sense for now, at least until a better solution is found to make it usable.
Proposed resolution
Since circles technically *are* supported by GeoJSON (as POINT data) I see no reason to enforce this option being permanently disabled. I think having the checkbox unchecked by default and displaying the current warning message is sufficient and allowing them to be used with the understanding that they will display as markers on the default map unless overridden with custom javascript.
In the future we could probably provide styling options for circle markers in the style setting options and use those when displaying the maps. We could possibly add a toggle option to use either markers or circles as a toggle and provide the appropriate toolbar button and display options as per preference.
User interface changes
Enable the CircleMarker toolbar widget.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | leaflet-enable-circlemarker-3184639-08.patch | 4.96 KB | scott.whittaker |
| #5 | 3184639_enable_CircleMarker.patch | 5.05 KB | scott.whittaker |
| #3 | 3184639_enable_CircleMarker.patch | 4.27 KB | scott.whittaker |
| #2 | 3184639_enable_CircleMarker.patch | 1.02 KB | scott.whittaker |
Comments
Comment #2
scott.whittaker commentedComment #3
scott.whittaker commentedOK I've done quite a lot of work on this one and implemented the marker choice as a radiobutton selection and provide the appropriate toolbar item and map layer styling automatically based on the selection. I've also implemented the custom styles in the drawing tool layer itself.
I'm not 100% happy with the code. I originally intended to catch the settings form submission and process the submitted settings so that the marker and circleMarker tool statuses are set in the original format on save so that that processing doesn't need to be done in the javascript, but I couldn't figure out how to catch settingsForm submissions within the class. I also tried processing it earlier on in the JS class building process but the changes didn't stick. It works fine as it is, but perhaps someone has a better approach for it?
Anyway here's how it looks in action:
Default option
Default marker display
circleMarker option
Custom style with radius
circleMarker tool, circle points and custom styles
Comment #4
scott.whittaker commentedComment #5
scott.whittaker commentedQuick update to provide radius as a default property in LeafletSettingsElementTrait.
Comment #6
itamair commentedthanks @scott.whittaker. This looks pretty interesting.
Going to test and review this asap ...
Comment #7
itamair commentedActually #patch doesn't apply cleanly.
From the Leaflet module code base, the following git command:
git apply --check 3184639_enable_CircleMarker_0.patchreturns the following output:
It seems there are issues in the way you created your patch, @scott.whittaker.
Also I recommend you to comply to:
- Drupal Patches naming conventions (https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa...). It seems you are not doing that: incremental patches should be named differently, usually embedding the issue comment #
- Drupal & Php Coding standards (https://www.drupal.org/docs/develop/standards/coding-standards). I didn't check yet, as the patch doesn't apply, but (better know in advance, and sorry if this looks superfluous to you) the patch won't pass the review if it doesn't comply with those ...
Comment #8
scott.whittaker commentedThanks for the tips, this patch should be correct now.
Comment #10
itamair commentedThanks @scott.whittaker ... Personally I don't like this special use case (and feature too much) but I liked your commitment and wanted to credit you for this.
I made some slight adjustment, but this commit basically adds your last patch to the 2.1.x-dev branch. Will be part of the next Leaflet module release ...
Comment #11
scott.whittaker commentedGreat, thank you. The more useful options we provide the better I think. At some point it's likely I'll need to add the ability to set custom marker icons as well, but not for the project I'm working on now.