🇬🇧United Kingdom @f.mazeikis

Well, I've used the signature you've suggested as a suggestion and made some changes. I also renamed the method to testRenderComponentFailure().

I have added SDC and Block Plugin that fails by default by intentionally throwing exceptions when passed TRUE as value for "crash" prop/input. By default, the value is TRUE. These new component sources are currently part of xb_test_block and xb_test_sdc test modules.

At the moment this causes failures of Functional tests for a bunch of endpoints and the recently introduced testRenderComponentLive() in SDC and Block tests. By the way, what does "Live" part of testRenderComponentLive() refers to? Don't think I've seen this term used before in XB Components context.

I am not sure if that was what @wim-leers expected? I could move them into some sort of "xb_test_failures" submodules and somewhat "limit the damage". Or is the intent is to go the other way around and make even more breakage, so we can spot all the potential issues?

balintbrews → credited f.mazeikis → .

Addressed feedback.

f.mazeikis → changed the visibility of the branch 3502371-make-page-title to active.

f.mazeikis → changed the visibility of the branch 3502371-make-page-title to hidden.

f.mazeikis → made their first commit to this issue’s fork.

wim leers → credited f.mazeikis → .

This is still an issue. Drupal stores the following error message:
OutOfRangeException: No data provided to evaluate expression ℹ︎␜entity:media:image␝field_media_image␞␟alt in Drupal\experience_builder\PropExpressions\StructuredData\Evaluator::doEvaluate() (line 44 of /Users/felix.mazeikis/work/pbx/web/modules/contrib/experience_builder/src/PropExpressions/StructuredData/Evaluator.php).

wim leers → credited f.mazeikis → .

This works as intended and as per error message:

page_title_block block plugin does not support previews.

So it's not a bug.
We do have issue #3502371 🐛 Handle adding page title to content Active that will change this, even if the implementation is still going to be somewhat limited.

I actually can't reproduce the issue anymore. I wonder if other work in 0.x resolved this. Would you be able to re-test this, @lauriii → ?.

I was able to recreate this issue with simple Image Component that we include in Experience Builder.
Attaching video recording of my browser.

The problem seems to have something to do with inconsistency in XB API responses and FE HTTP call payloads.
When image is selected via media library browser, the selected image is represented by media entity ID. React application makes XB HTTP PATCH calls to `/xb/api/layout/node/{id}` and potentially to `/xb/api/form/component-instance/node/11` in order to refresh preview and sidebar form. For example, this is how `model` part of the HTTP PATCH payload to `/xb/api/layout/node/11` looks like on my local after selecting image via media library:

{
  "source": {
    "image": {
      "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}",
      "sourceType": "static:field_item:entity_reference",
      "sourceTypeSettings": {
        "storage": {
          "target_type": "media"
        },
        "instance": {
          "handler": "default:media",
          "handler_settings": {
            "target_bundles": {
              "image": "image"
            }
          }
        }
      }
    }
  },
  "resolved": {
    "image": "2"
  }
}

Note here "resolved": { "image": "2"}.

When I hit "Review & Publish" button, multiple HTTP calls are made by the FE part of the application. There's successful POST call to `/xb/api/autosaves/publish` and successful GET call to `/xb/api/layout/node/11`.
However, the successful GET response from XB API contains resolved image as an object, instead of media entity id:

"model": {
        "354a208b-33d2-4d07-80f7-5072e5fd066d": {
            "resolved": {
                "image": {
                    "src": "\/sites\/default\/files\/2025-03\/Screenshot%202025-03-03%20at%2009.21.56_0.png",
                    "alt": "kitch",
                    "width": 1840,
                    "height": 996
                }
            },
            "source": {
                "image": {
                    "sourceType": "static:field_item:entity_reference",
                    "expression": "\u2139\ufe0eentity_reference\u241f{src\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241fentity\u241c\u241centity:file\u241duri\u241e\u241furl,alt\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241falt,width\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241fwidth,height\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241fheight}",
                    "sourceTypeSettings": {
                        "storage": {
                            "target_type": "media"
                        },
                        "instance": {
                            "handler": "default:media",
                            "handler_settings": {
                                "target_bundles": {
                                    "image": "image"
                                }
                            }
                        }
                    }
                }
            }
        }
    },

After this response, I get at least a single HTTP PATCH call to `/xb/api/layout/node/11` resulting in HTTP 500 error and potentially second HTTP PATCH call resulting in HTTP 500 to `/xb/api/form/component-instance/node/11`, depending on whether I have selected the component in the preview or not. Both API calls have the model part of their payloads for image looking like so:

{
  "resolved": {
    "image": {
      "src": "/sites/default/files/2025-03/Screenshot%202025-03-03%20at%2009.21.56_0.png",
      "alt": "kitch",
      "width": 1840,
      "height": 996
    }
  },
  "source": {
    "image": {
      "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}",
      "sourceType": "static:field_item:entity_reference",
      "sourceTypeSettings": {
        "storage": {
          "target_type": "media"
        },
        "instance": {
          "handler": "default:media",
          "handler_settings": {
            "target_bundles": {
              "image": "image"
            }
          }
        }
      }
    }
  }
}

Note "resolved": {"image" part is no longer just media entity ID, but object containing `src`, `alt`, `width` and `height`. If I understand this correctly, that's what causes failed preview (losing the image part) and HTTP 500 responses.

The actual publishing of the Node still seems to happen as intended and publishing transaction stores the correct media entity ID in Node XB field inputs.
I had separate calls with @jessebaker and @longwave + @bnjmnm and me are all unsure what the data flow should actually be - IDs or fully resolved images and what is actually supposed to be happening here. We do think however that reporting this as a bug is probably incorrect - the ability to select media entity for image, saving and publishing the node with such component is not a finished and tested functionality. Can't be a bug if we haven't finished building it :sweat_smile:

We also suspect that there are multiple Media Library adjacent issues that are all related - @longwave is already looking into issue to do with default values and thinks it's stemming from the same "unfinished feature" problem. We suspect that a meta issue is required to round up the issues around Media Library and images and outlining what exactly is missing and focusing on that.

f.mazeikis → created an issue.

Updated Cypress tests, no PHP tested were affected.

f.mazeikis → made their first commit to this issue’s fork.

Made some changes in efforts to address https://www.drupal.org/project/experience_builder/issues/3504819#comment-15992301

Attaching some screenshots validating the following:

• disabling block component works
• manually disabled block components show up in the reasons list
• multiple incompatibility reasons for SDC components are listed
• enabling block component works

Addressed feedback.

Merged in 0.x, resolved conflicts, implemented Wim's suggestion.

f.mazeikis → made their first commit to this issue’s fork.

f.mazeikis → changed the visibility of the branch 3473275-support-disabled-block to active.

Implemented the last suggestion, addressed failing CI on ESlint and merged.

Addressed nits, updated one failing test, waiting on CI.

f.mazeikis → made their first commit to this issue’s fork.

Actually, found an issue, posted comment on MR.
TL;DR: Until Code Component is not saved with _something_ in it's CSS field, preview doesn't include draft CSS changes.
Attaching screen recording for clarity.

Resolved merge conflict, tests passing and the code makes sense. However, trying this out on local I see no draft CSS changes being applied to the actual preview. This seems to be regression, as few days ago at least saved CSS of the component would show in preview, but now neither saved nor draft CSS is being applied during preview. Looking into this.

f.mazeikis → made their first commit to this issue’s fork.

f.mazeikis → made their first commit to this issue’s fork.

f.mazeikis → changed the visibility of the branch 3473275-support-disabled-block to hidden.

wim leers → credited f.mazeikis → .

Addressed the feedback, there's still a couple of questions in MR, but it's ready for another review pass.

f.mazeikis → made their first commit to this issue’s fork.

Merged in 0.x - a lot of changes since. Will try to spend some time working on this issue.

f.mazeikis → created an issue.

wim leers → credited f.mazeikis → .

Reviewed and tested - great work!

I’ve added ednpoints to openapi.yml, but I’ve intentionally stopped myself from adding all the methods and examples, as I am not even sure if that’s reasonable thing to do, if FE doesn’t use it yet. Happy to expand this, if that will be the in the review feedback.
The other thing is !505 was merged in and resulted in conflict that forced me to make some changes to ApiConfigControllers and XbConfigEntityHttpApiTest , so changes in MR are slightly beyond just addressing the previous review feedback.

wim leers → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

wim leers → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

wim leers → credited f.mazeikis → .

Reviewed and approved. It seems MR still needs approval from @tedbow or @effulgentsia.

tedbow → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

balintbrews → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

Looks good, approved

Re-ran pipeline and it's now passing. Code looks good too - certainly an improvement, even if we still need to come back to this once we have solutions for datetime and image (and entity references in general) props handling.

I've created #3475584 📌 Add support for Blocks as Components Active to start working on "Blocks as Components" requirement. It's based on @larowan work started in MR68 (closed). It's limited scope, focusing only on blocks for now and using the "Component Source Plugin" approach, as per #12 🌱 [META] Support component types other than SDC Needs work .

I wasn't marking it as blocking as it looked as if the issue would be merged anyway and we could have started this issue while the work continued on #3469609 📌 Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs Fixed . And it is merged now.
I've moved most of the code from MR68 (closed) by @larowlan and reached out to him on slack as I couldn't retain his commit signature.
This is not a working commit and will be used as starting point.

f.mazeikis → created an issue.

All the feedback from me and Dave has been addressed, moving to RTBC

Additional docs, ADRs and context is really neat!
Posted some nitpicks and minor feedback about sections that I personally found difficult to follow.

Your self-review questions on MR and contextualisation of commits was really helpful.
There was a lot of context and information to go through, so it took some time.
But at the end I feel like I know much better what is happening in that MR and in general with Props/Shapes/JsonInterpreter related part of backend.
Hence the lack of nitpicks - it is a significant improvement to clarity when it comes to namespacing, docs and inline docs.

Well, the MR is mostly clarifying, removing dead code and updating docs. I don't have any nitpicks 😅

My only comment is that perhaps we should file an issue for #note_374141, just so we don't lose track of that it still needs to be addressed.

wim leers → credited f.mazeikis → .

Applied suggestions, converted controller into a service, added comments.

f.mazeikis → created an issue.

f.mazeikis → created an issue.

Addressed additional feedback, waiting on more feedback on 2 unresolved threads in MR.

Bonus idea: a "Refresh" button that calls ::clearCachedDefinitions() + ::getDefinitions(), to force a refresh :)

This already happens on every page load for the "Disabled components" page. Would you prefer that not to happen and introduce a separate "refresh" button to trigger this behaviour manually?
Other than that, I've addressed (or at least replied to) the feedback on the MR.

I think I've addressed all of the feedback.

Pushed the changes so far, don't want to polish this too much before we agree it's going in the right direction.

wim leers → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

+1 to using enable()/disable() for now
Implementing a new requirement "must not be obsolete" for auto-adding is simple enough
Auto-delete needs in-use tracking, as per #3 📌 New component requirement: obsolete SDCs must not be auto-added, but must be auto-updated Active .

f.mazeikis → created an issue.

We’ve got this pretty close to done, but it is still failing Cypress test. Specifically, due to Shoe Button (components/simple/shoe_button/shoe_button.component.yml) component missing. It is not being auto-generated as there’s no StorablePropShape for it’s icon prop.
The way I see us resolving this:

• Removing/updating cypress test (I originally was thought it was testing a feature we need, but @wim-leers tells me otherwise)
• Prioritising this might resolve #3467890 📌 [later phase] Support `{type: object, …}` prop shapes that require *multiple* field types — also: nested components/component reuse Postponed , but that issue was deprioritised and would take quite a while
• We could consider adding another hardcoded match for icon shape prop in Drupal\experience_builder\SdcPropJsonSchemaType::computeStorablePropShape as we are already doing for image (L243), but it also needs #3467890 📌 [later phase] Support `{type: object, …}` prop shapes that require *multiple* field types — also: nested components/component reuse Postponed

I'll proceed with removing or modifying the failing Cypress test.

Wim Leers → credited f.mazeikis → .

Wim Leers → credited f.mazeikis → .

f.mazeikis → created an issue.

Wim Leers → credited f.mazeikis → .

Reviewed, re-read and approved. Nice amount of details in there, the Terminology section alone is worth it's weight in gold. I see @wim-leers is already responding to @longwave threads, so we just need @tedbow to have a look.

Postponed until blocker 📌 Add fieldInstanceSettings to (Candidate)StorablePropShape Active is resolved.

f.mazeikis → created an issue.

Approved.