SDC prop of `type: string` with empty string listed in its `enum` results in broken input UX

By default XB adds a 'None' value to the SELECT OPTIONS, which is great.
But it doesn't seem possible to set this None value as the default value.

What I tried:
- Not to add examples at all
- examples: []
- examples: [NULL]
- examples: ['']

All of them return the same error
Twig\Error\RuntimeError: An exception has been thrown during the rendering of a template ("[linno_theme:select/select] Does not have a value in the enumeration ["ratio-32x9","ratio-21x9"]. The provided value is: ""."). in Twig\Template->yield() (line 1 of themes/custom/linno_theme/components/select/select.twig).

anjali rathod → made their first commit to this issue’s fork.

I'll look into this.

By default XB adds a 'None' value to the SELECT OPTIONS, which is great like this no need to add a empty string in enum.

FYI this is thanks to \Drupal\Core\Field\Plugin\Field\FieldWidget\OptionsSelectWidget::getEmptyLabel()'s:

      if (!$this->required) {
        return $this->t('- None -');
      }

Thanks, @tirupati_singh! Adding some related issues :)

Hi all, I've created a new component called "CTA Button" to replicate the issue and confirm that it still persists. Below is the component.yml file:

name: CTA Button
description: This is component for adding CTA.
status: stable
props:
  type: object
  properties:
    cta_url:
      type: string
      title: Link URL
      description: Add the url to be used.
      format: uri
      examples:
        - 'https://www.drupal.org'
    cta_title:
      type: string
      title: Link Title
      description: Add the title to be shown on hovering
      examples:
        - 'Learn more'
    cta_icon:
      type: string
      title: CTA icon
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - ''
        - PDF
        - Video
        - External link
        - Arrow icon

I agree with @wim leers and @heyyo's point that

By default XB adds a 'None' value to the SELECT OPTIONS, which is great like this no need to add a empty string in enum.

I also believe the empty string is unnecessary for the component props, as the "None" option provided by Drupal already covers this functionality.

To address the issue as per the comment #3 🐛 SDC prop of `type: string` with empty string listed in its `enum` results in broken input UX Active , I removed the empty string value from enum. However, I’d prefer that we accept the "None" option provided by Drupal, rather than adding an empty string in enum.

By removing the empty string, this should also resolve issues with other components. I've attached before and after screenshots of the fix for reference.

Please review the MR changes and let me know if any further adjustments are needed or if there's a better solution to address the issue.

Thanks!

@tirupati_singh: Thank you so much for articulating a much clearer path forward than I did back in January when I created this issue! 👏❤️🙏

I was very against this, but definitely last IS update in #10 changed my mind. Thanks Wim!
There are examples in core (at least in core tests) using this, so we might need a core issue for fixing those.

Yay, glad to read that! 😄

I don’t see why core SDCs would need to change though. They could, yes. But plenty of (core and contrib) SDCs won’t work in XB today. What is different about these? 🤔

That’s why we have the “Appearance -> Components -> Disabled Components” UI that provides the reason why a component is not compatible.

The reason is exactly the point that changed my mind:

> This is an misuse of JSON Schema/SDCs. The optionality of a type: string, enum: … should be indicated by NOT listing it as a required prop.

Core will be used as implementation reference for most devs. This is not only about XB support (even this issue should be focused on that and I'm offtopic), but about defining best practices.

👍 Makes sense!

Moving back to "needs work" and assigning myself to look into the failing tests.

You took a different approach than what's in the issue summary, @thoward216, and you had me first thinking it was genius, simple and elegant, but … I think I spotted some negative consequences: do you agree?

Either way, this definitely still needs explicit test coverage. I think:

• one test SDC that has type: string, enum: [""] — aka only a single enumerated value, that now won't be valid anymore — this is also the case that definitely doesn't work in your current approach
• another test SDC that has type: string, enum: ["funny", "", "strings", "here"]
• perhaps another? That seem to be the edge cases?

Oh, and this should add a message in \Drupal\experience_builder\ComponentMetadataRequirementsChecker::check(), to ensure there's a friendly message 👍

Combined with the test SDCs I suggested above, that'd force you to update the test expectations in \Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponentTest::testDiscovery(), which will then prove the friendly message appears as needed/expected :)

thoward216 → changed the visibility of the branch 3503087-sdc-prop-of to hidden.

thoward216 → changed the visibility of the branch 3503087-sdc-prop-of to active.

I've updated the approach here to match the proposed solution, added a new test SDC and updated tests as needed.

After updating the approach, a number of tests are now failing due to them using the SDC sdc_test:my-cta as this is now not compatible with XB. (as it contains an enum with an empty.) It appears it is used in a lot of other tests which now error, due to the tests trying to use it. I'm not clear on the path forward for this yet.

#21:

XB started early on with only SDC support. For pragmatic reasons, and to avoid painting ourselves into a corner decorated with our own assumptions, we relied on test SDCs in Drupal core. That includes core/modules/system/tests/modules/sdc_test/components/my-cta/my-cta.component.yml, which now turns out to be a bit of a weird one.

I see that there are 37 failing tests relating to sdc_test:my-cta after the (necessary!) change this MR makes.

The clearest possible failure is

ComponentValidationTest::testEntityIsValid
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for experience_builder.component.sdc.sdc_test.my-cta with the following errors: 0 [status] The component '<em class="placeholder">sdc.sdc_test.my-cta</em>' cannot be enabled because it does not meet the requirements of Experience Builder., 1 [status] Prop "target" has an empty enum value.

… which is exactly right: this is saying that the Component config entity has status: true but it can't/shouldn't be because of the empty string enum value! 👍

I suggest the following path forward:

1. in places where the use of sdc_test:my_cta is meaningless (i.e. "just some SDC"), simply pick another SDC. For example, experience_builder:heading or experience_builder:my-hero (which both also have a few props) or even experience_builder:druplicon (which has zero props).
2. in places where some aspects of the props of sdc_test:my_cta are tested/used for tests: replace it with another SDC that has similar props. For example ContentTemplateDependencyTest used "my CTA" but could easily switch to "heading" and retain the same test coverage, even despite testing specific props (explicit inputs). Because the input that the test is centered around is a type: string, which is a prop shape that "heading" also contains 👍
3. in other places, switch to a different SDC.
4. … if and only if you run into a place where there truly is a need for semantically exactly what my-cta offered (essentially, a link: URL + text + target attribute), then just add a "cta" or "link" SDC to XB itself.

@wim leers, thank you for the detailed feedback and insights — much appreciated!

You took a different approach than what's in the issue summary, @thoward216, and you had me first thinking it was genius, simple and elegant, but … I think I spotted some negative consequences: do you agree?

Yes, I do agree with your assessment. You're absolutely right — I took a different approach by omitting "" from the enum instead of explicitly disallowing it. At first glance, it seemed like a clean and unobtrusive way to sidestep the HTML limitation, but I can now see the downside — particularly in cases where "" is explicitly included in a required field's enum. This results in an unusable form element, which defeats the purpose of validating the SDC in the first place.

After re-reviewing the changes in my merge request, I realized that the current fix does not fully resolve the issue, especially for props like cta_icon1 and cta_icon2.

While the intention was to handle enums with empty strings more gracefully, the approach I took (silently omitting "") doesn't correctly address all problematic cases. For instance:

• cta_icon1 includes "" in the middle of the enum array.
• cta_icon2 includes "" at the end.

Both of these cases still result in invalid for below component props

cta_icon:
      type: string
      title: CTA icon
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - ''
        - PDF
        - Video
        - External link
        - Arrow icon
    cta_icon1:
      type: string
      title: CTA icon 1
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - ''
        - External link
        - Arrow icon
    cta_icon2:
      type: string
      title: CTA icon 2
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - External link
        - Arrow icon
        - ''

Thanks again for the clear explanation — it’s really helped clarify the situation. I'll adjust the implementation to explicitly disallow "" in enums and ensure proper test coverage for these cases.

Thanks @thoward216 for the quick implementation of the requested changes. I’ve been reviewing the updated merge request with the feedback incorporated, but I’m encountering an issue while testing it with the same component props mentioned in comment #23.

When I use the component props as follows (from the earlier example):

name: CTA Button
description: This is component for adding CTA.
status: stable
props:
  type: object
  properties:
    cta_url:
      type: string
      title: Link URL
      description: Add the url to be used.
      format: uri
      examples:
        - 'https://www.drupal.org'
    cta_title:
      type: string
      title: Link Title
      description: Add the title to be shown on hovering
      examples:
        - 'Learn more'
    cta_icon:
      type: string
      title: CTA icon
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - ''
        - PDF
        - Video
        - External link
        - Arrow icon
    cta_icon1:
      type: string
      title: CTA icon 1
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - ''
        - External link
        - Arrow icon
    cta_icon2:
      type: string
      title: CTA icon 2
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - External link
        - Arrow icon
        - ''

I did not encounter the specific error message ('Prop "%s" has an empty enum value.', $prop_name) in the ComponentMetadataRequirementsChecker class, after the feedback was incorporated and the changes were made in the MR. However, I am now encountering a different issue in the Experience Builder UI. The error message displayed is: "An unexpected error has occurred while rendering the component's form." I’ve attached a screenshot for reference.
Could you please suggest any specific steps or checks I should follow when creating or testing the component? If there’s anything I might have missed or overlooked in the process, I’d really appreciate it.

Thanks!

@wimleers - thanks for the context and suggested path forward in #22 🐛 SDC prop of `type: string` with empty string listed in its `enum` results in broken input UX Active will take a look into this.

@tirupati_singh - I've pulled in the latest commit locally and tested your above component and it appears to work as expected, it does not show in the component library in experience builder and appears within the "disabled" section under "/admin/appearance/component/status" with the reasons - see screenshot.

wim leers → changed the visibility of the branch 0.x to hidden.

Was the component already placed within experience builder and saved?

My bet too :)

@thoward216 Just confirming … you are unblocked, right? 🤞

@wimleers thanks, yes I'm unblocked on this.

Tests are now all passing with the update approach. Moving to needs review.

Sooooooooooo very close! 😄

🕺