Props default depend on the source

Created on 4 December 2024, about 2 months ago

Problem/Motivation

For example when defining an attributes prop type, if in the component a default value is set, it is ok with the "HTML classes" source, but it is lost when switching to another source.

I think there are 2 points to discuss:

- first, I guess the format of the default value is different depending on the source plugins or is it the source plugin responsibility to transform this default value to fit in its code?
- second, if point one is not doable and default value format depends on the source plugin, when defining a component, it is not possible to guess which source plugin will be selected by default so this could lead to empty config form by default.

✨ Feature request
Status

Active

Version

2.0

Component

Code

Created by

🇫🇷France Grimreaper France 🇫🇷

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

Merge Requests

Comments & Activities

  • Issue created by @Grimreaper
  • 🇫🇷France pdureau Paris
  • 🇫🇷France pdureau Paris

    first, I guess the format of the default value is different depending on the source plugins or is it the source plugin responsibility to transform this default value to fit in its code?

    Today situation: if NULL, the prop is unset. So, it may be good.

    second, if point one is not doable and default value format depends on the source plugin, when defining a component, it is not possible to guess which source plugin will be selected by default so this could lead to empty config form by default.

    The default value in a prop must be valid with the prop type schema. It the job of the source plugins to alter the default value to fit with the form element(s) if necessary.

    For example, in theory, an attribute prop will have this default value:

    class: ["foo", "bar]
    role: "info"
    
    • The attributes widget will transform the default value to class="foo bar" role="info"
    • The classes widget will transform the default value to foo bar

    Do we agree on this mechanism? Is it the current implementation?

  • 🇫🇷France just_like_good_vibes PARIS

    I am not sure about the mechanism of default value we need/want, and the exact one we have today.
    To my knowledge, we were not relying at all in sources, on the default value in the prop being edited.
    but i may be wrong.
    Right now, each source is independent and it would be each source responsibility to read from the prop definition, and fill the defaultSettings of the source, in order to make that value appear in the form.

    so right now, default value in the definition seems not used in sources.

    but what do we want to support ?

    Question 1) : should the default value in prop definition appear on the source form that is editing that prop ?

    Question 2) : If it appears, should we have "revert to default?" mechanism, maybe a "default value" checkbox, a defaultValue source or whatever?

    Question 3) : When should we really apply default value? in other words, when is a source value really "null" and be replaced to the default. What if we want to really put null as a value?

  • 🇫🇷France just_like_good_vibes PARIS

    To add to this "brainstorm", imagine a slot with a default value which would be an insane render array or any random markup.
    it is not realistic to have any slot source to be able to support that value as being "casted" into a source form.

    Honestly, i would rather let sources be free to implement or not the default value handling, and we need maybe to add some little things to the current code :

    > do we show/indicate the default value in prop/slot description ?
    > do we add a "DefaultValue" source which job is to empty the source settings and let the default value be injected for sure ?

  • 🇫🇷France pdureau Paris

    Hi Mikael,

    so right now, default value in the definition seems not used in sources.

    Can you have a look on https://git.drupalcode.org/project/ui_patterns/-/blob/2.0.x/src/SourcePl... .

      public function getSetting(string $key): mixed {
        $value = parent::getSetting($key);
        if (("value" === $key) && (NULL === $value)) {
          return $this->getDefaultFromPropDefinition();
        }
        return $value;
      }
    
      protected function getDefaultFromPropDefinition(): mixed {
        if (is_array($this->propDefinition) &&
          array_key_exists("default", $this->propDefinition)) {
          return $this->propDefinition["default"];
        }
        return NULL;
      }
    
      protected function mergeDefaults() : void {
        $defaultSettings = $this->defaultSettings();
        // -> we prefer the prop definition default value.
        if (array_key_exists("value", $defaultSettings)) {
          $defaultValueProp = $this->getDefaultFromPropDefinition();
          if (NULL !== $defaultValueProp) {
            $defaultSettings["value"] = $defaultValueProp;
          }
        }
        $this->settings += $defaultSettings;
        $this->defaultSettingsMerged = TRUE;
      }
    

    Contributed by you 6 month ago: https://git.drupalcode.org/project/ui_patterns/-/commit/af53b6af42d7f622...

    Is it still OK? Do we need to restore/replace this mechanism?

  • 🇫🇷France just_like_good_vibes PARIS

    hmm yes, i see sorry. i forgot that one.
    so yes, we have this little mechanism but it covers only some sources, the ones deriving from SourcePluginPropValue.
    Those sources are a bit specific : They have only a 'value' setting key, and they may use it in their form.
    in those particular cases, we process the default value indicated in the prop json schema.
    We can keep this behavior for those sources, but what about other sources ?

    list of sources deriving from SourcePluginPropValue :

    • Drupal\ui_patterns\Plugin\UiPatterns\Source\CheckboxesWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\CheckboxWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\ClassAttributeWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\NumberWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\SelectWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\SelectsWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\TextfieldWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\UrlWidget


    other noticeable sources (i also used the current ones from ui_icons and i filtered out the ones with slot in the prop_types)

    • Drupal\ui_icons_patterns\Plugin\UiPatterns\Source\IconSource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\AttributesWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\BreadcrumbSource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\EntityFieldSource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\EntityLinksSource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\EntityReferencedSource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\FieldPropertySource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\ListTextareaWidget
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\MenuSource
    • Drupal\ui_patterns\Plugin\UiPatterns\Source\PathSource
    • Drupal\ui_patterns_field_formatters\Plugin\UiPatterns\Source\FieldLabelSource
    • Drupal\ui_patterns_views\Plugin\UiPatterns\Source\ViewTitleSource
  • 🇫🇷France pdureau Paris

    They have only a 'value' setting key, and they may use it in their form. in those particular cases, we process the default value indicated in the prop json schema.

    So we are good, aren't we? :)

    So, to fix this issue, maybe we need to make Drupal\ui_patterns\Plugin\UiPatterns\Source\AttributesWidget extending SourcePluginPropValue

  • 🇫🇷France just_like_good_vibes PARIS

    ok, seen during the weekly

    we will deprecate the "HTML class source", still usable the same way.

    but, the recommended source is Attributes, and the processing of the value is more elaborate.
    we check if "=" sign is inside the value,
    - if yes : in the attributes string format
    - otherwide : a list of classes

    so validation and getPropValue are changing for the better :)

    let's update also the default source

  • 🇫🇷France pdureau Paris

    I just add a little wording and i merge

  • 🇫🇷France pdureau Paris
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024