Default image for SDC with optional image is lost after changing value for another SDC prop

Please share the SDC with which you tested this. Otherwise we have to waste time figuring out how to reproduce this.

Will investigate Monday.

Picking this up as I've been solving other image issues in #3507641: Allow components containing non-required images and no examples to be placed

Chatted with @longwave. I am going to take a look at this since he is on another issue

Lovely, thanks! 🙏

I am wondering if this is front-end issue. I am not super familiar with how the front-end sends back the form inputs but this is what I am seeing using the optional-props-image‎ component I added in the MR

1. Add the component to the canvas
2. open the edit form
3. start to type in the heading
4. the image goes away in the canvas

Between step 3 and 4, I debugged the call to \Drupal\experience_builder\Controller\ApiLayoutController::patch()

The body of the request has model->source->heading and model->resolved->heading but 'image' is not under either shouldn't it be sent back under 'source' even though it is empty?

Looking at the value in the client form of edit-form-xb-props I see

{
  "name": "XB test SDC with optional text and image",
  "resolved": {
    "heading": {
      "value": "Heading the right direction?"
    }
  },
  "source": {
    "heading": {
      "expression": "ℹ︎string␟value",
      "sourceType": "static:field_item:string",
      "value": {
        "value": "Heading the right direction?"
      }
    },
    "image": {
      "required": false,
      "jsonSchema": {
        "title": "image",
        "type": "object",
        "required": ["src"],
        "properties": {
          "src": {
            "title": "Image URL",
            "type": "string",
            "format": "uri-reference",
            "pattern": "^(/|https?://)?.*\\.(png|gif|jpg|jpeg|webp)(\\?.*)?(#.*)?$"
          },
          "alt": {
            "title": "Alternative text",
            "type": "string"
          },
          "width": {
            "title": "Image width",
            "type": "integer"
          },
          "height": {
            "title": "Image height",
            "type": "integer"
          }
        }
      },
      "sourceType": "static:field_item:entity_reference",
      "expression": "ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}",
      "sourceTypeSettings": {
        "storage": {
          "target_type": "media"
        },
        "instance": {
          "handler": "default:media",
          "handler_settings": {
            "target_bundles": {
              "image": "image"
            }
          }
        }
      }
    }
  }
}

So it does have the info for image. should it be sending back what it has?

Manually testing the MR now I can get to not lose the default image when changing the heading property.

My solution was to set the props that were missing from the client in \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::clientModelToInput

Not knowing this part of the code it seems reasonable to allow the client the skip props that don't have a value, since the server can figure out this info.

Will see if this breaks other tests

@tedbow: when it's green, can you please mark Needs review and assign to @longwave? 🙏 He knows this bit best.

This seems related to Wim's analysis in #3507929-25: Allow adding "image" props in the code component editor.. not sure the fix is quite right but it's in the right location, will try to look later on today.

#13: I had a similar hunch!

Added a test for this and fixed it to only apply to object shapes; this works but it feels wrong? Also I don't like the way the fallbacks are checked both before and then after the inputs are set up, but too tired now to figure out how to refactor it nicely. The test passes locally though!

Wondering if this it the right way at all, maybe we should be altering the SDC component info much earlier to remap the example/default path instead of trying to map it in via a prop source.

I've been testing https://github.com/phenaproxima/xb-demo and ran into this issue and using the MR on my local fixed this issue.

Also tested with this and it worked :)

#15: AFAICT this is hugely overlapping with #3509608-9: Unable to save an optional image with its default value (because it does not correspond to a Media entity): allow saving DefaultRelativeUrlPropSource?! I bet you write in #9 it feels wrong for lots of reasons, and I articulated those reasons in #3509608-9?! 😅

👆 Rebased onto 0.x.

I bet #3493943: SDC+JS Component Sources: split default values into `resolved` and `source` will help with this.

Unfortunately, #3493943: SDC+JS Component Sources: split default values into `resolved` and `source` hasn't been completed yet, even though it would have removed the need for this work-around. @longwave confirmed at #3493943-20: SDC+JS Component Sources: split default values into `resolved` and `source` that that is the proper fix.

So, to ease the pain during DrupalCon (next week!), we intend to land this work-around, knowing full well that #3493943 will again remove it.

Going ahead and merging this to fix this ahead of DrupalCon next week.

But https://git.drupalcode.org/project/experience_builder/-/merge_requests/7... identified that this is actually being fixed in the wrong place: it's not broken on the server side, but on the client side: https://git.drupalcode.org/project/experience_builder/-/merge_requests/7....

I'm assuming #3493943: SDC+JS Component Sources: split default values into `resolved` and `source` will fix that, too.

Thanks!!!