Change base/default base Leaflet Map from 'OSM Mapnik' into 'OpenStreetMap' (openstreetmap)

Thanks @itamair for a fast response and fix, as always.

Do you think it's possible to make OpenStreetMap the default, under Leaflet settings for form and display? Because currently it's OSM Mapnik...

Also, I am not sure what the purpose of including "default" is, could something really simple like this not work?

  return [
    'OSM Mapnik' => ['label' => 'OSM Mapnik (deprecated)'] + $openstreetmap_default_map_settings,
    'openstreetmap' => ['label' => 'OpenStreetMap'] + $openstreetmap_default_map_settings,
  ]; 

PS. A detail: openstreetmap_dafault_map_settings should be openstreetmap_default_map_settings :)

Thanks @itamair, it looks great now, and the user interface works much better, without having the old OSM Mapnik there as well.

Freshly created, everything works really well, nice!

I only saw a single slightly "buggy" thing, which is not crucial. But if you start with OSM Mapnik, update to the latest version, and go to Form or Display pages:

• /admin/structure/types/manage/article/form-display
• /admin/structure/types/manage/article/display

... you get these warnings:

Warning: Undefined array key "openstreetmap" in Drupal\leaflet\Plugin\Field\FieldFormatter\LeafletDefaultFormatter::getDefaultSettings() (line 82 of modules/contrib/leaflet/src/LeafletSettingsElementsTrait.php).

It's not a big deal, you can just rebuild cache, and they are gone. I guess most users rebuild caches when they update modules with Composer?

Use label, not index on Form and Display settings page?
OSM Mapnik happened to use "OSM Mapnik" as index, so looked all right. But OpenStreetMap's index is "openstreetmap", so uses that. Is it possible to use label, and not index for the settings pages in the output of the "Format" column (last). See for example Image style, which uses human readable label "Wide (1090)" (/admin/structure/types/manage/article/display):

In the old version (with OSM Mapnik):

Leaflet Map: OSM Mapnik

In the new version:

Leaflet Map: openstreetmap

Two unrelated observations I just noticed:
Map "- Empty -" option for Form but not Display settings
I just noticed that there is an "- Empty -" option for Form but not Display settings, under Map Settings > Leaflet Map. Maybe it can be removed, unless it serves a purpose? Because it seems like it is required nonetheless, so selecting "- Empty -" and updating gets you an error.

Streamline Form and Display settings pages' widget/Format columns?
On the Form page, the "Widget" column (the last one) shows:

Geometry Validation: disabled

On the Display page "Format" column (the last one) shows:

Leaflet Map: openstreetmap
Map height: 400 px
Popup Infowindow: No

Is it worth considering showing map and resolution as well in the Form page "Widget" column, if possible? Like this:

Geometry Validation: disabled
Leaflet Map: openstreetmap
Map height: 400 px

Great, though there still is a single thing. If I open the Form display settings page and select "Leaflet Map (default)" (do we need " (default)" ?) the spinner starts but everything stalls and I get this error in the Console:

Uncaught 
	Object 
{
  message: "
  An AJAX HTTP error occurred.
  HTTP Result Code: 200
  Debugging information follows.
  Path: /admin/structure/types/manage/article/form-display?ajax_form=1
  StatusText: parsererror
  ResponseText: TypeError: Drupal\\Component\\Utility\\Html::escape(): Argument #1 ($text) must be of type string, null given, called in /var/www/html/web/core/lib/Drupal/Component/Render/FormattableMarkup.php on line 238 in Drupal\\Component\\Utility\\Html::escape() (line 433 of /var/www/html/web/core/lib/Drupal/Component/Utility/Html.php).",
  name: "AjaxError",
  stack: "@https://drupal.ddev.site/sites/default/files/js/js_LFmCYD36YaSxPMd2Vtt6HfODS-rKhhEFdZCPi_qSSbs.js?scope=footer&delta=1&language=en&theme=claro_compact&include=eJxtUG0OgyAMvRDCGXYSUqAqE4GVYubth5lkn7_avq_kFdzqo-aUggFS55RMiHJOG5KPjJEFvMt0QSA7q3-gXHA3CcjpMidiW1kYP-nsM6q-CJta6p0rBOWoZgjyhQxnXhMRdhos-w2H4OPygTOYgI5g-kUJS06xNNuTO4pQbOz1VpF2OSZaxegxOF19d_ZbfD8Ei4WMl6PwA11Ld9s:257:2314
  @https://drupal.ddev.site/sites/default/files/js/js_LFmCYD36YaSxPMd2Vtt6HfODS-rKhhEFdZCPi_qSSbs.js?scope=footer&delta=1&language=en&theme=claro_compact&include=eJxtUG0OgyAMvRDCGXYSUqAqE4GVYubth5lkn7_avq_kFdzqo-aUggFS55RMiHJOG5KPjJEFvMt0QSA7q3-gXHA3CcjpMidiW1kYP-nsM6q-CJta6p0rBOWoZgjyhQxnXhMRdhos-w2H4OPygTOYgI5g-kUJS06xNNuTO4pQbOz1VpF2OSZaxegxOF19d_ZbfD8Ei4WMl6PwA11Ld9s:257:19388
  "
}
js_LFmCYD36YaSxPMd2Vtt6HfODS-rKhhEFdZCPi_qSSbs.js:257:2314

By the way, if you get tired of manually creating release notes at some point, many use Matt Glaman's tool for this. It also automatically adds links to the relevant issues: https://www.drupal.org/docs/develop/git/git-for-drupal-project-maintaine...

Thanks @itamairo, it looks good now. It could be considered to add a tip in the 10.4.7 release note, that the user may need to rebuild caches, if they are using OSM Mapnik and updating from a previous version ... because otherwise it is shown in the Form and Display pages.

Like I mentioned, the Form settings page widget includes "default" (Leaflet Map (default)). It seems too much to open a separate issue for, unless you prefer that, then let me know.

PS. About release notes, it would be a great improvement for the users, if real links were used, so you can just click them and go straight to the issue. On drupal.org [#3586903] is automatically rendered as a link, including the title, like here: #3586903: Change base/default base Leaflet Map from 'OSM Mapnik' into 'OpenStreetMap' (openstreetmap).

You probably know this, and maybe have good reasons to hand craft the text, and not use this automatic link feature -- but I thought I'd just mention it.

Perfect @itamairo, thanks for clarifying about the "default" on the Form display page, and why it actually serves a purpose, so I agree that it should stay.

I appreciate your openness to checking out the Drupal Make Release Notes tool, I think it will be a gain for both you and the users, getting streamlined release notes, and automating boring tasks, like formatting, making sure all changes are included, etc.

I very much appreciate your great work on maintaining and expanding the Geofield stack (including Leaflet!) and will continue to try to help out where I can. Have a nice day :)

PS. I tried generating release notes (10.4.5 > 10.4.7) and it looks like a pretty good start, which of course needs refining:

Add a summary here

Contributors (2)

itamair, ressa

Changelog

Issues: 1 issues resolved.

Changes since 10.4.5 (compare):

Misc

• Fixed tests compatibility with 11.3:
• leaflet/js/leaflet.drupal.js: code adjustment to set the marker.title tooltip only in case of Marker simple title,
• Change OSM Mapnik label to OpenStreetMap
• Merge branch '10.4.x' of git.drupal.org:project/leaflet into 10.4.x
• 'OSM Mapnik' bundled/default base map into 'openstreetmap' ('label' => 'OpenStreetMap')
• LeafletDefaultFormatter: render the Leaflet Map Label in the settings summary.
• LeafletMap.php: minor code improvement
• LeafletDefaultWidget: added Leaflet Map info and Map Height to Summary.
• Further Leaflet code refactoring and fixes.
• Merge branch '10.4.x' of git.drupal.org:project/leaflet into 10.4.x

Task

• Revert " feat: #3586903 Change OSM Mapnik label to OpenStreetMap"