Overview

Quoting @lauriii from #3501902-65: Adding the Image component results in a state considered invalid:

I'm getting this error when I try to place "Container" element from demo design system. It has an optional image without an example.
AssertionError: assert(\is_array($this->value)) in assert() (line 78 of /var/www/html/web/modules/contrib/experience_builder/src/PropSource/FallbackPropSource.php).

Quoting @longwave from #3501902-66: Adding the Image component results in a state considered invalid:

the problem with the demo Container is that it specifies an image without an example:

    # Don't put examples as this is used with and without images.
    image:
      $ref: json-schema-definitions://experience_builder.module/image
      type: object
      title: Image

but Experience Builder's image schema says the src property is required:

    "image": {
      "title": "image",
      "type":  "object",
      "required": ["src"],
      "properties": {
        "src": {
          "title": "Image URL",
          "$ref": "json-schema-definitions://experience_builder.module/image-uri"
        },

This causes a validation error because when you first add the component the image doesn't have a src. If an example was specified (as in the XB image component) then we use that as the fallback, but there is nothing to fall back to here either.

Proposed resolution

  1. Allow GeneratedFieldExplicitInputUxComponentSourceBase::clientModelToInput() to skip resolving a value when a prop is not set.
  2. Prevent computation of the default resolved value from crashing if no example is present.

User interface changes

None.

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

wim leers created an issue. See original summary.

lauriii’s picture

Category: Task » Bug report
Priority: Major » Critical

It must be possible to provide components with optional image fields without a default value, hence empty examples must be considered a valid schema for optional images.

\Drupal\experience_builder\ComponentMetadataRequirementsChecker::check() only checks for the presence of an examples[0], but it does not verify that it actually complies with the JSON Schema!

But in this case examples[0] doesn't exist: https://git.drupalcode.org/project/demo_design_system/-/blob/1.0.x/stars... 🤔

lauriii’s picture

Issue tags: +sprint-candidate

longwave made their first commit to this issue’s fork.

wim leers’s picture

Crediting @mherchel for #3507543, which is a duplicate. See my response at #3507543-3: Single broken component can break XB list of all components.

wim leers’s picture

Issue tags: -sprint-candidate +sprint

wim leers’s picture

wim leers’s picture

🏓

longwave’s picture

Status: Needs work » Needs review
Issue tags: +Needs issue summary update

This actually solves #2 and not the original problem described, so tagging for issue summary update.

This works locally with the image-no-example and SDDS components no longer crash for me, but some of the other SDDS components are acting strangely for me locally so this needs some more testing before it's ready to go in.

longwave’s picture

Assigned: longwave » Unassigned

Unassigning for now, will pick up again next week.

wim leers’s picture

Assigned: Unassigned » wim leers

Will review Monday.

wim leers’s picture

Assigned: wim leers » longwave

Other things took priority.

longwave’s picture

Title: ComponentMetadataRequirementsChecker::check() should validate that the example(s) actually match the JSON schema » Allow components containing non-required images and no examples to be placed
Assigned: longwave » Unassigned
Issue summary: View changes
Issue tags: -Needs issue summary update

Since I moved the check to earlier then it seems the filtering is no longer needed, only the explicit continue, plus the fix to allow missing examples.

Updated the IS and migrated the original issue to #3508725: ComponentMetadataRequirementsChecker::check() should validate that the example(s) actually match the JSON schema

longwave’s picture

Assigned: Unassigned » wim leers

Assigning to Wim for review.

wim leers’s picture

Status: Needs review » Needs work

This looks GREAT!

But I'd like to see slightly more test coverage. So that we never have to revisit #3501902: Adding the Image component results in a state considered invalid nor anything "image prop shape"-related. 🤞

wim leers changed the visibility of the branch 3502902-simpler to hidden.

wim leers’s picture

Assigned: wim leers » Unassigned
Status: Needs work » Reviewed & tested by the community

FYI: added the (trivial) comprehensive test coverage I wanted: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6... — good to go now!

Why? As I wrote in #16: I don't ever want to revisit this again! 😅

  • wim leers committed 6bccf220 on 0.x authored by longwave
    Issue #3507641 by longwave, wim leers, lauriii: Allow components...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed
nagwani’s picture

Issue tags: -sprint
wim leers’s picture

Component: Config management » Component sources

Status: Fixed » Closed (fixed)

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