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.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

grimreaper created an issue. See original summary.

pdureau’s picture

Title: Props default depend on the source » [2.0.0-rc1] Props default depend on the source
pdureau’s picture

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" and send them to the textfield element
  • The class_attribute widget will transform the default value to foo bar and send them to the textfield element

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

just_like_good_vibes’s picture

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?

just_like_good_vibes’s picture

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 ?

pdureau’s picture

Assigned: Unassigned » just_like_good_vibes
Status: Active » Needs work

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?

just_like_good_vibes’s picture

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
pdureau’s picture

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

just_like_good_vibes’s picture

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

just_like_good_vibes’s picture

Assigned: just_like_good_vibes » pdureau
Status: Needs work » Needs review
pdureau’s picture

Title: [2.0.0-rc1] Props default depend on the source » [2.0.0-rc1] Merge attributes sources
Status: Needs review » Reviewed & tested by the community

I just add a little wording and i merge

pdureau’s picture

Assigned: pdureau » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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