Component "default" functionality isn't implemented

Created on 18 October 2023, 8 months ago
Updated 18 June 2024, 8 days ago

Problem/Motivation

According the the documentation we should be able to do the following:

    tertiary:
        type: string
        title: Tertiary

        # Limit the available options by using enums.
        enum:
            - success
            - warning
            - danger

        # Provide a default value
        default: success

However this doesn't seem to be implemented. In the tests this value is missing from the my-button component and is being passed in explicitly through the template instead.

Steps to reproduce

Failing test attached, see MR !5032 with the failing test report.

Proposed resolution

Implement the missing default functionality for SDC, see MR !5353, including both the test change, and the actual fix.

Remaining tasks

Maintainer review.

User interface changes

No.

API changes

No.
Documentation always suggested `default` should work for properties, so it is instead allowing that behavior.

Data model changes

No.

Release notes snippet

Maybe, hinting that default can be used now, as originally intended is a good idea, but may not be needed.
Also, let us change the related documentation to remove the "It isn't used in Twig template." around default, which would be implemented after this change gets in.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
single-directory components 

Last updated about 15 hours ago

Created by

🇬🇧United Kingdom justafish London, UK

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

Merge Requests

Comments & Activities

  • Issue created by @justafish
  • 🇬🇧United Kingdom justafish London, UK
  • last update 8 months ago
    30,414 pass, 1 fail
  • First commit to issue fork.
  • Merge request !5353Resolve #3394815 "Sdc default props values" → (Open) created by wotnak
  • Pipeline finished with Success
    8 months ago
    Total: 1498s
    #48099
  • Status changed to Needs review 8 months ago
  • 🇵🇱Poland wotnak

    Added props default value functionality implementation in https://git.drupalcode.org/project/drupal/-/merge_requests/5353. It passes test modified by justafish.
    I implemented it in `sdc_additional_context` twig function that is executed in ComponentNodeVisitor and already used to add things to component context. The implementation loops through component props and if a given prop has a default value set and doesn't already have a value in the current context, the default value is added to the context (which makes it available in the component template).

  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    Can the issue summary be updated please, left some TBD if they need to be filled in, can put NA if it doesn't apply to this issue.

  • 🇺🇸United States FrankieD3

    Any updates on this functionality? If this has still yet to be implemented, the documentation should be altered to reflect the current state of the module so not to mislead developers.

  • Status changed to RTBC 2 months ago
  • 🇵🇪Peru marvil07

    @justafish, thanks for opening this issue, and providing a clear test change to reproduce it 👍

    @wotnak, thanks for implementing a fix! 👍

    Apart from the test fix, I also verified with another use-case on a custom theme that the default value is being used.

    @smustgrave, I have updated the issue summary with a few more details.

    Marking as RTBC.

  • 🇮🇳India Pravesh_Poonia

    @wotnak, changes are looking fine to me
    @justafish, you can check and close the issue

  • First commit to issue fork.
  • Pipeline finished with Success
    2 months ago
    Total: 992s
    #147856
  • Status changed to Needs work 2 months ago
  • e0ipso Can Picafort

    This has come up in the past in Slack. See this thread. Screenshooting for convenience:

    In that thread @pdureau mentions:

    Prop default values are not automatically injected into the template and that's a good thing IMHO. They can be used by the SDC ecosystem to build admin pages forms, where they will be the form element default value. You can set a default value in the template with the default() filter.

    I tend to agree. The JSON-Schema specification allows the default keyword, and we support JSON-Schema. However, I don't think the intention for it should be to inject data into the template. I think the correct use for it is to inform the application of a default value when building forms, or generating synthetic example renderings of a component, etc.

    While we could do what the IS seeks, we would face significant challenges down the road. Imagine when we need to translate the default value, or if the default value is a site URL and the site is installed in a subdirectory, or the default date should follow the site configured time zone, ...

    In general, imagine that we need to use dynamically processed data as the default (which I think is a common use case). For that we need a place to declare the default with access to a runtime. The current recommendation is to use Twig to do so. This makes sense for me because:

    1. It is the place where the data is provided to the component. That be via mapping fields from a content type, providing a hard-coded value, ... or doing any of those but with a defauls. Ex: content.field_foo|default('my default'|t).
    2. It is a place that can access the necessary APIs for those dynamic behaviors mentioned above.
    3. It is a common established practice in Twig that front-end developers already know.

    However, I acknowledge that there is confusion around this. The fact that it has come up more than once is an indicator. I wonder if adding a section in the official FAQs is the way to fix this.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    We need to update https://www.drupal.org/docs/develop/theming-drupal/using-single-director... for sure as that's what as caused some of the confusion.

  • 🇺🇸United States agentrickard Georgia (US)

    I tend to agree with the assessment that this is a documentation/schema feature and not a means to inject data into Twig, which would make this a documentation issue.

    I wonder if this part: generating synthetic example renderings of a component is something that might be implemented by modules like Storybook. If so, we could open a similar issue there.

  • 🇫🇷France pdureau Paris
    The JSON-Schema specification allows the default keyword, and we support JSON-Schema. However, I don't think the intention for it should be to inject data into the template. I think the correct use for it is to inform the application of a default value when building forms, or generating synthetic example renderings of a component, etc.

    🙏

    The current recommendation is to use Twig to do so.

    As a side note, using default() filter is a better practice than the collapsed (?:) or null (??) ternary operator:

  • 🇺🇸United States FrankieD3

    I feel as though the updated text "Provide a default value. It isn't used in Twig template." does not include enough context. If I were looking at this with fresh eyes, I would be thinking to myself, "then where is it being used?"

    An example of a Twig template containing the use of the default() filter would be useful somewhere within the documentation. Something to that effect of {% set text = text|default('Placeholder'|t) %}.

  • 🇺🇸United States xjm
Production build 0.69.0 2024