Schema (or/and possibly the defaults in LeafletSettingsElementsTrait)

Created on 25 February 2023, over 1 year ago
Updated 24 March 2023, over 1 year ago

Problem/Motivation

Tests fail on schema for form field configuration using leaflet widget.

Steps to reproduce

This is an example of configuration exported
https://git.drupalcode.org/issue/geo_entity-3344453/-/blob/3344453-switc...

Running the test in that geo_entity_address submodule

1) Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud                                                                                                                                            
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for core.entity_form_display.geo_entity.address.inline with the following errors: core.entity_form_display.geo_entity.address.inline:content.loca
tion.settings.geometry_validation missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.multiple_map missing schema, core.entity_form_display.geo_entity.address.inline:conte
nt.location.settings.leaflet_map missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.height missing schema, core.entity_form_display.geo_entity.address.inline:content.loca
tion.settings.height_unit missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.hide_empty_map missing schema, core.entity_form_display.geo_entity.address.inline:content.loc
ation.settings.disable_wheel missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.gesture_handling missing schema, core.entity_form_display.geo_entity.address.inline:conten
t.location.settings.popup missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.popup_content missing schema, core.entity_form_display.geo_entity.address.inline:content.loca
tion.settings.leaflet_popup missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.leaflet_tooltip missing schema, core.entity_form_display.geo_entity.address.inline:content.
location.settings.map_position missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.weight missing schema, core.entity_form_display.geo_entity.address.inline:content.locati
on.settings.icon missing schema, core.entity_form_display.geo_entity.address.inline:content.location.settings.leaflet_markercluster missing schema, core.entity_form_display.geo_entity.address.inline:content.locat
ion.settings.feature_properties missing schema  

I'll post a branch with all the schema items added, it was almost a cut & paste from field.formatter.settings.leaflet_formatter_default into field.widget.settings.leaflet_widget_default.

But while I was cutting and pasting my way through the fields in that entity_form_display configuration I started to questioning why some of the fields were there. Once I got to leaflet_popup I was wondering if I'd actually seen this in anything I could configure for the widget. Then at weight: null I was truly intrigued https://git.drupalcode.org/issue/geo_entity-3344453/-/blob/3344453-switc...

My assumption, without digging further, what is going on here is configuration that is or isn't actually used from LeafletSettingsElementsTrait?

Proposed resolution

Either add all the configuration schema for what is exported by the configuration item; or possibly thin it out a bit for stuff that's not actually used (if that's the case)?

Remaining tasks

Decide which is the best course forwards...

🐛 Bug report
Status

Fixed

Version

10.0

Component

Code

Created by

🇳🇱Netherlands ekes

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @ekes
  • 🇳🇱Netherlands ekes

    So this would be the schema diff https://git.drupalcode.org/issue/leaflet-3344455/-/commit/5090465815a815... but like I say I'm not sure how many of these are actually used or available to field.widget.settings.leaflet_widget_default.

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy itamair

    Nice catch @ekes ...
    there is something to correct in exported configuration for the "leaflet_widget_default" field configuration,
    and basically limit it to the effectively uses properties/settings.

    The LeafletDefaultWidget is indeed including the LeafletSettingsElementsTrait because it is sharing (and re-using) some setting form components/blocks defined in it (such as the Geocoder Map Control for instance) but not all of them ...

    The problem (I guess) is that its defaultSettings() method is defining all its default options merging all the ones included in the LeafletSettingsElementsTrait::getDefaultSettings() ... that would hence be part of its configuration anyway, though not exposed to the UX and never used by the widget.

    So, rather than extending the LeafletDefaultWidget schema with al those (also un-used one) I would correct the LeafletDefaultWidget definition so that only the settings really used in the widget form would be part of its exported configuration ...

    • itamair committed 24e4c4da on 10.0.x
      Correct limitations of default Options (and Schema) of...
  • Status changed to Fixed over 1 year ago
  • 🇮🇹Italy itamair

    This is issue has been corrected into 10.0.x branch, so as in the incoming 10.0.11 Leaflet module release.

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

Production build 0.69.0 2024