Default image is not loaded after removing media

We'll need to check if clicking the button triggers the appropriate Redux state changes.

There's some trickiness here (surprise!) because all of the current redux-updating is based on changes to input values, but when an item is removed via media library, this is reflected in the form/ui by removing the correspnding hidden <input /> that represents the selection.

I have some ideas on how to deal with this...

Current MR gets us closer to fixing it. Currently works best when the UI is loaded and the image prop already has media.

It is still buggy when new media is added and then removed. I assume it's because the component is now removed-element aware, but there isn't yet a full equivalent for additions

wim leers → credited f.mazeikis → .

@f.mazeikis re-discovered this same bug. @longwave is going to update the media-library.cy.js e2e test.

During pair (triple?) debugging we discovered that the client-side model never got updated. Our going theory is that the mediaSelection client-side transform added in 📌 Move clientside assumptions about prop form data shape into a series of prop specific transforms Active is not firing when removing an image.

Rebased and added assertions to media-library.cy.js to demonstrate the issue. The canvas starts with a sample image inserted, which we assert correctly, then the Remove button is clicked, which (I believe) should fall back to the example image for the component - but this doesn't happen at the moment so the test fails.

Removed the SDC changes, as per Wim we can use image-optional-with-example-and-additional-prop for testing here.

There is something wrong with the changes to the frontend in this MR, as this now has the same result as 🐛 SDCs with optional images without examples cannot be placed Active when testing manually with image-optional-with-example-and-additional-prop - as soon as I select a valid media entity then the form errors out with a 400 error because the prop is missing. I've tried making some changes in the frontend code but I'm not sure what I'm doing or where it's going wrong so assigning to @bnjmnm for more investigation.

https://git.drupalcode.org/project/experience_builder/-/blame/0.x/ui/src... is the likely cause (disclaimer didn't check the MR)
When we click remove there's no new elements in the ajax response and hence nothing to trigger the model update via this event

Tried to test this and it doesn't seem to be doing what's expected. The image persists but I'm also unable to make changes to other props after removing the image 🤔

@lauriii Id like to try and reproduce your findings to narrow down what is going on, but it looks like the component you are testing with is called "Image with text", which is not one available in the experience builder codebase.

The MR was tested with the various image-including components provided by the xb_test_sdc module, as well as the standard image component. I'd like to see what is different about the one in #18 and figure out which side things need to be addressed.

The component used for the review in #18 is a custom code component. I think it's still worth reviewing with the included-with-experience-builder components the MR was tested with as it would be good to get feedback on the solution approach sooner than later.

Once I have access to & can successfully import the custom component from #18 I can see why that is running into problems, but that probably should preclude getting eyes on the solution that has solved the reported symptom for the components that are available to anyone with the Experience Builder codebase.

Also, with the current solution in the MR any removed image reverts to the example when one is available, otherwise it is removed from the preview. The IS suggests there might be interest in different logic depending on whether or not the image prop is required. That should be a feasible expansion of the current logic if needed.

Tested this with the built in components and couldn't reproduce #18 🐛 Default image is not loaded after removing media Active with them. I attached config for the component that allows you to reproduce this.

When remove is clicked on an optional image, it should not fallback to the example image.

Keeping this needs review in case that the approach needs review.

Only test fails are a common random failer. Will click retry and it may be green by the time anyone sees it.

But I found a bug when testing with a JS component that has both the image prop and a text prop. Here in this GIF, I removed the media image, but then the text prop content also disappears from the preview, but it still exists in the form. And then when you type in the text prop, it does return to the preview.

I expanded the two e2e tests at the end of media-library.cy.js to include this scenario and they are passing. Perhaps the issue was resolved by a recent commit - many bugfixes recently.

I manually tested this and it works great but I'm a bit uneasy about the isRemovedProp() method as well - I think we should just be able to drop the values from resolved and source entirely and let the server side figure out that we need to go back to the default?

Good news
It looks like changes in 0.x that happened since I came up with the solution ~3 months ago means there's way less backend hacky stuff needed.

Bad news
Getting the e2e tests to pass reliably - despite the IRL version working fine - is not going smoothly at all. I'll just set this to needs work until I get that slop more agreeable.

Marked 🐛 Cannot remove media Active as a duplicate of this.

Thanks for all the reviews etc on this lil' dragger.

It looks like in the case of a required image for a SDC, after removing the image, the component fails to render due to an error. Is this expected?

For #35.

Few more observations:

1. Upon image removal, the error message is shown: "Component failed to render, check logs for more detail."
2. The image container appears to break layout and overflow the content container.
3. Validation for required image fields fails before removal and works after removal:
   1. When an required image component is added via drag-and-drop and published without adding an image, it doesn't works.
   2. However, if the image is removed afterward, the component throws the mentioned error and it prevents publishing with: "The property image is required."

Argh - this must have been due to changes that occured during the longer-than-typical review cycle. Filed a new bug for what was found 🐛 SDC Required image can't be removed without error Active . If there's interest in keeping this open until that is fixed NP, but it will be easier for me to get it addressed with a fresh issue scoped to those specific symptoms.

Thanks for opening that! Asked a Q there to hopefully be able to help out with it.

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