Problem/Motivation

I use ui_suite_dsfr 1.1.x with ui_patterns:2.x

In twig, i call the pattern tile without giving border props. According to component definition, border default value is TRUE, but this default value is never given to tile.twig.

My twig file :

{{ include('ui_suite_dsfr:tile', {
    variant: 'horizontal',
    image : content.field_icon ,
    title : content.title,
    icon: paragraph.field_tile_display_icon.value
}, with_context = false) }}

Steps to reproduce

Call a pattern from twig without passing a prop which is defined with default value.

Proposed resolution

Take care of props defaults values.

Remaining tasks

  • Fix
  • Create test

User interface changes

If some implementation of patterns in twig didn't declare props which have default value, display could be affected.
Example : in tile (for ui_suite_dsfr), border is default to true. After fixing this issue, implementation of the tile without defining border props will display borders, previously doesn't display borders.

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

goz created an issue. See original summary.

goz’s picture

Digging, i think the issues comes from src/Template/TwigExtension.php

  /**
   * Normalize props (and slots).
   *
   * This function must not be used by the templates authors. In a perfect
   * world, it would not be necessary to set such a function. We did that to be
   * compatible with SDC's ComponentNodeVisitor, in order to execute props
   * normalization before SDC's validate_component_props Twig function.
   *
   * See ModuleNodeVisitorBeforeSdc.
   *
   * @param array $context
   *   The context provided to the component.
   * @param string $component_id
   *   The component ID.
   *
   * @throws \Drupal\Core\Render\Component\Exception\InvalidComponentException
   */
  public function normalizeProps(array &$context, string $component_id): void {
    $component = $this->componentManager->find($component_id);
    $props = $component->metadata->schema['properties'] ?? [];
    foreach ($context as $variable => $value) {
      if (isset($component->metadata->slots[$variable])) {
        $context[$variable] = SlotPropType::normalize($value);
        continue;
      }
      if (!isset($props[$variable])) {
        continue;
      }
      $prop_type = $props[$variable]['ui_patterns']['type_definition'];
      $context[$variable] = $prop_type->normalize($value, $props[$variable]);
      if ($context[$variable] === NULL) {
        unset($context[$variable]);
      }
      if (isset($props[$variable]['ui_patterns']['prop_type_adapter'])) {
        $prop_type_adapter_id = $props[$variable]['ui_patterns']['prop_type_adapter'];
        /** @var \Drupal\ui_patterns\PropTypeAdapterInterface $prop_type_adapter */
        $prop_type_adapter = $this->adapterManager->createInstance($prop_type_adapter_id);
        $context[$variable] = $prop_type_adapter->transform($context[$variable]);
      }
    }
    // Attributes prop must never be empty, to avoid the processing of SDC's
    // ComponentsTwigExtension::mergeAdditionalRenderContext() which is adding
    // an Attribute PHP object before running the validator.
    // Attribute PHP object are casted as string by the validator and trigger
    // '[attributes] String value found, but an object is required' error.
    $context['attributes'] = $context['attributes'] ?? [];
    $context['attributes']['data-component-id'] = $component->getPluginId();
  }

  /**
   * Preprocess props.
   *
   * This function must not be used by the templates authors. In a perfect
   * world, it would not be necessary to set such a function. We did that to be
   * compatible with SDC's ComponentNodeVisitor, in order to execute props
   * preprocessing after SDC's validate_component_props Twig function.
   *
   * See ModuleNodeVisitorAfterSdc.
   *
   * @param array $context
   *   The context provided to the component.
   * @param string $component_id
   *   The component ID.
   *
   * @throws \Drupal\Core\Render\Component\Exception\InvalidComponentException
   */
  public function preprocessProps(array &$context, string $component_id): void {
    $component = $this->componentManager->find($component_id);
    $props = $component->metadata->schema['properties'] ?? [];
    foreach ($context as $variable => $value) {
      if (!isset($props[$variable])) {
        continue;
      }
      $prop_type = $props[$variable]['ui_patterns']['type_definition'];
      $context[$variable] = $prop_type->preprocess($value, $props[$variable]);
    }
  }

None of the 2 methods take default props values.

goz’s picture

Title: Default props values are not used » Default props values are not used including patterns in twig

goz’s picture

Status: Active » Needs review
pdureau’s picture

Status: Needs review » Postponed (maintainer needs more info)

Thanks you for your contribution, Fabien.

SDC is not processing JSON schema's default: https://www.drupal.org/docs/develop/theming-drupal/using-single-director...

    tertiary:
      type: string
      title: Tertiary
      # Provide a default value. It isn't used in Twig template.
      default: success

And we agree with that in UI Patterns 2: https://project.pages.drupalcode.org/ui_patterns/2-authors/0-authoring-a...

UI Patterns 2 uses default when building the component form, as #default_value.

default must not be used in the rendering process (sending default value if prop value is missing or empty) because:

  • sometimes we want a default value in forms while allowing the user to set empty or missing value
  • the |default() Twig filter is the expected tool for such an enforcement

what do you think about this?

goz’s picture

They document it by comment, but it doesn't make it good.

For my point of view, JSON schema of a component describe how the component is build and rules applying to it.
We should rely on it, and not report some stuff in twig for something like default values, especially when the implementation is not that hard.

If i make a documentation based on component definitions, UX will be very bad to say for each default value (be careful, this default value is not a default value for the component but only for form component). Form configuration is one of the way to use components. All the ways to implement components should follow the same rules.

I understand you are aligned with how SDC work, so it's more an issue with this core module at first.

I guess waiting fir this, theme maintainers will have to keep this in mind and report themselves the default logic into their twig files. One little missing piece of code, need to be care by many sub-projects.

goz’s picture

pdureau’s picture

2 things to take into account:

  • Users may be annoyed when data will start to be automatically injected into their components although this was not the case for 2 years
  • It is nice to keep all logic, including default values replacement, into a single unit of code: the Twig template. YAML definition are declarative.