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.

Comments

scott.whittaker created an issue. See original summary.

scott.whittaker’s picture

StatusFileSize
new1.02 KB
scott.whittaker’s picture

StatusFileSize
new4.27 KB

OK 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:

Only local images are allowed.
Default option

Only local images are allowed.
Default marker display

Only local images are allowed.
circleMarker option

Only local images are allowed.
Custom style with radius

Only local images are allowed.
circleMarker tool, circle points and custom styles

scott.whittaker’s picture

Status: Active » Needs review
scott.whittaker’s picture

StatusFileSize
new5.05 KB

Quick update to provide radius as a default property in LeafletSettingsElementTrait.

itamair’s picture

thanks @scott.whittaker. This looks pretty interesting.
Going to test and review this asap ...

itamair’s picture

Status: Needs review » Needs work

Actually #patch doesn't apply cleanly.

From the Leaflet module code base, the following git command:

git apply --check 3184639_enable_CircleMarker_0.patch

returns the following output:

error: leaflet/js/leaflet.widget.js: No such file or directory
error: leaflet/src/LeafletSettingsElementsTrait.php: No such file or directory
error: leaflet/src/Plugin/Field/FieldWidget/LeafletDefaultWidget.php: No such file or directory

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 ...

scott.whittaker’s picture

Thanks for the tips, this patch should be correct now.

itamair’s picture

Status: Needs work » Fixed

Thanks @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 ...

scott.whittaker’s picture

Great, 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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.