Media Library integration (includes introducing a new main content renderer/`_wrapper_format`)

How would a SDC author declare that a component can accept a media image? (Backend)

It should not declare such a thing. Component author must declare a slot, that's all. A slot can accept any renderable. Sending a media image to the slot is the responsibility of the component user, on the CMS side.

How can we load the Media Library within the Experience Builder (which is a React SPA)? (Frontend)

It was started as a React SPA, but will it stay like that? Maybe, one day, we will consider this choice has more drawbacks than advantages.

How would a SDC author declare that a component can accept a media image? (Backend)

I agree with @pdureau, sort of.

A "media image" really means:

• a Media entity
• … of a bundle that is configured to use \Drupal\media\Plugin\media\Source\Image
• Which corresponds to … an image.

Step 1: SDC author defines the expected shape, and ideally using a predefined JSON schema definition

So, the SDC author should not define what the UX is for getting an image in there. They should declare that they want an image. But there's (at least) 2 kinds of images:

1. an URL to an image — which could be sufficient if it gets used as a background image
2. an URL to an image + associated metadata (alt + width + height), necessary to be used. in an <img> tag

We haven't gotten very far with making all of these concepts well-defined at the SDC JSON schema level, but we do have some, and we do have it for the aforementioned 2:

1. {$ref: 'json-schema-definitions://experience_builder.module/image-uri'}
2. {$ref: 'json-schema-definitions://experience_builder.module/image'}

(introduced in https://git.drupalcode.org/project/experience_builder/-/commit/f784af41, which is heavily inspired by @pdureau's https://git.drupalcode.org/project/ui_patterns/-/blob/28cf60dd776fb349d9...)

Step 2: when opting in an SDC component to XB, the default value for each prop determines the field type

#3444417: "Developer-created components": mark which SDCs should be exposed in XB landed. One of the next steps is #3452397: Allow specifying default props values when opting an SDC in for XB. An implied part of setting up a default value there is that a field type is selected at that time: whichever field type the default value for the StaticPropSource uses that the Site Builder configures, also determines what the field type is that Content Creators get to use (they do not get to choose: that'd make the UX unbearably complex).

In other words: if the selected default value is some "media image", then the Content Creator will be presented with the ability to use a media image.

How can we load the Media Library within the Experience Builder (which is a React SPA)? (Frontend)

This question is insufficiently specific IMHO 😇

We've discussed before that as a first step, we intend to load the existing field widgets in the right-hand sidebar (presumably with their Form API-based definitions automatically rendered using https://www.drupal.org/project/jsx, to bring real-time updates — see #3453690: [META] Real-time preview: supporting back-end infrastructure).

But … the way that the question is phrased seems to refer to in-place editing. Which is not something we should be doing from the start. See #3453690-10: [META] Real-time preview: supporting back-end infrastructure.

Ahhhh! So it's more focused on the "this ain't no simple single-document viewport, it's a viewport with many (i)frames" challenge. Now I get it 👍

We haven't gotten very far with making all of these concepts well-defined at the SDC JSON schema level, but we do have some, and we do have it for the aforementioned 2:

{$ref: 'json-schema-definitions://experience_builder.module/image-uri'}
{$ref: 'json-schema-definitions://experience_builder.module/image'}

Indeed, I was a bit "quick" pushing the opaque slot as the only solution. I still thing is by far the best, but a well-defined and structured object prop ("object" according to JSON, so an associative array, a Twig "mapping", not a PHP object) can also do the job, and the JSON schema reference system can help the author.

However, let's not look at the contents of the URI string ("experience_builder", "image") to guess if it is an image or not. Those URI, like every URI, are opaque too. An other reference URI (ex: {$ref: 'json-schema-definitions://whatever.module/amstragram'}) pulling the same JSON schema structure must be processed the same way.

2.How can we load the Media Library within the Experience Builder (which is a React SPA)? (Frontend)

I've managed to decouple(ish) Entity Browser in the RJSF module, it still depends on some drupalSettings values so it's not fully decoupled. Never explored doing it for Media Library because Entity Browser did a good enough job for my needs and could be used for any entity type. Linking in case it's helpful. https://git.drupalcode.org/project/rjsf/-/tree/1.0.x/modules/rjsf_entity...

However, let's not look at the contents of the URI string ("experience_builder", "image") to guess if it is an image or not.

🤔 Why? This would result in broken components wherever images are used. Can you elaborate? 🙏

Linking in case it's helpful.

Thanks!

🤔 Why? This would result in broken components wherever images are used. Can you elaborate? 🙏

If {$ref: 'json-schema-definitions://experience_builder.module/image'} resolve to:

{
  "type": "object",
  "properties": {
     "url": {"type": "string"; "format": "iri-reference"},
     "alt": {"type": "string"},
     "width": {"type": "integer"},
     "height": {"type": "integer"}
  }
}

The schema resolver will juste replace the reference to the resolved JSON schema snippet.

Then, the logic (is it an image?) will apply on the resolved JSON schema.

So, if a component author directly write (without using $ref):

{
  "type": "object",
  "properties": {
     "url": {"type": "string"; "format": "iri-reference"},
     "alt": {"type": "string"},
     "width": {"type": "integer"},
     "height": {"type": "integer"}
  }
}

It will also works.

Or if a component author just writes:

{
  "type": "object",
  "properties": {
     "url": {"type": "string"; "format": "iri-reference"},
     "alt": {"type": "string"}
  }
}

It will also works.

Or if a component author uses another reference resolving the same schema snippet.

{$ref: 'json-schema-definitions://whatever.module/amstragram'})

It will also work.

It is solid architecture, using JSON schema reference as expected, without introducing "weird" behaviour by deducing information from the characters in that URL.

At some point do we need to talk about formatters? Yes we might store a media reference - but we might format it using e.g. a different view mode or a responsive image format

Integrate the Drupal Media directly with the Experience Builder, allowing users to upload, browse, and select images from the Media Library.

When talking about Experience Builder, it seems we are not talking a lot about the blocks, layout, formatters... plugins. Are they still considered as first class object? Or are they becoming an afterthought? Do we plan to reimplement in Experience builder features already existing in Drupal (like Media Library Browser) instead of leveraging them?

Experience Builder is currently looking like 2 different "products" bundled in one:

• the promised easy-to-use, powerful & universal display builder, which will replace Layout Builders and others
• SDC & design system / tokens integration / management / building. Ambitious, but still a bit blurry.

I am worrying this "coupling":

• will ask extra unexpected work (reimplementing features, but also mechanisms like cache or dependencies management), so we may not meet our ambitious projects targets (calendar, scope...)
• will cut experience builder from large parts of the Drupal ecosystem by:
  • not exposing the SDC integration to the other tools
  • not leveraging the plugins provided by Druapl core & contrib
• will create technical debt (when we will want to replace or revamp one of the 2 parts) and change management risk (if one of the 2 parts of Experience Builder is not welcomed by the community, it will take down the other part with it)

Instead, let's promote the Drupal plugins we all know and love at the heart of the discussions and architecture, as an universal interface between the display builder and the design system manager.

What do you think about that?

I think we might be able to bypass the #3456008: [later phase] Support matching enum SDC prop shapes against DynamicPropSources, not only generating StaticPropSources blocker by just going with manually defined first. Which is what #3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings would allow. I'll revisit this once that's done.

Following up on #19: stated that at #3456008-7: [later phase] Support matching enum SDC prop shapes against DynamicPropSources, not only generating StaticPropSources too 👍

To avoid being blocked on #3452512: Add component instance edit form to contextual panel, let's first do #3462709: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed.

As of #3462709-12: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed, this became actionable 😊

#3462709: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed is in! 🚀

@bnjmnm Assigning to you because when we paired on this yesterday, you said you'd do a few more things that I don't see in the MR yet. I don't want our commits to cross. 😅

I pushed things further.

But before I dive very deep into the AJAX system, I want to make sure you're seeing the same error after clicking the Add media button of MediaLibraryWidget. Because I'd swear it's different from what you showed me a few days ago 😅

This is what I see:

(note that url at the bottom, that's the one it opens)

And the response to that request is simply

{}

I also ran into a JS error due to Olivero JS getting loaded … which somehow happens because of attaching core/drupal.ajax 🤯 Did you see that too? Any ideas? Details at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1....

Back to you, @bnjmnm 🤓 Will continue this on Monday!

Closed #3467498: Component props edit form route should use a custom wrapper format as a duplicate.

Updating this issue title because that would've been pretty much impossible for @larowlan to find based on the current issue title. (It happened to be relevant for this issue's scope.)

Found out what is causing the pile of olivero assets being added

• core/drupal.ajax depends on core/drupal.message
• Olivero extends core/drupal.message with olivero/drupal.message
• olivero/drupal.message depends on olivero/messages
• olivero/messages lists olivero/global-styling as a dependency, adding ~45 CSS files and the olivero/navigation-base library. Seems like a bit much for a messages library?

The issue I'm running into when attempting to add media via the widget might be the same as the one in #31, but when I discussed it earlier I was showing where it fails server side - it is it looking for an 'image' field on the node entity vs what I'm guessing should instead be a property of a ComponentTreeItem?

If press play in the debugger and let the PHP take its course the end result is the same as#31.

It's not the slickest, but here's a few screenshots of the trace, starting with the most recent just-about-to-throw and a few steps back.

Epic debugging trail there! 😬

I'll try to push this further today.

With the changes I just pushed up, the widget now works in the props form -> Short video if interested

It does not yet update the layout when the media is updated, though, so that's the next step.

#35: thanks for that video, that's awesome! 😄

FYI, I created a critical bug for a long-standing annoyance: #3469436: Fix the visually broken "image" component instance: use FileUriItem's computed `url` property, not the stored `value` property.

IMHO that belongs in the current sprint because it directly impacts the Barcelona demo. @lauriii confirmed.

(Because \Drupal\Tests\experience_builder\Kernel\MediaLibraryHookStoragePropAlterTest::getExpectedStorablePropShapes() also contains the …entity:file␝uri␞␟value… prop expression, so without #3469436, it’ll be impossible for @bnjmnm to get a selected image media to render correctly in the XB preview!)

IOW: don't bother fixing that in this issue — getting the image to render correctly is that issue's responsibility, not this issue's. It's this issue's responsibility to allow that public://path/to/homer.png file URI referenced by the selected image media to show up. 😊

@bnjmnm noticed that this also needed the fix that another front-end issue needed, so landed it in #3469729: Follow-up for #3467712: field instance settings lost in ComponentPropsForm, to help make this MR slightly simpler.

The MR is messy as heck right now, but the widget now updates the layout: https://youtu.be/DFiE4r5mEFE, though as discussed previously it is passing the file URI so we're not seeing a new image there, but the alt text change and the prior image going away is pretty good evidence that it works.

There's and extra step to get the existing image to show up in the media library widget when the props form opens, because it's not actually a media entity. Rather, it's an image field using the media library widget. If we want to do this, it will need to create a media entity for the image (perhaps on the fly) so it can be displayed within the widget. That should be easy enough, but I may ask around to be sure it's necessary.

The MR probably has some junk in there from the different approaches I tried to make it work but I assure you it'll be less chaotic soon (but probably still little chaotic)

#38: 🥳🚀 NICE! 👏 (For the visual brokenness, @tedbow is working on that over at #3469436: Fix the visually broken "image" component instance: use FileUriItem's computed `url` property, not the stored `value` property as I mentioned in #36.)

Do we already have an issue to load the media library using the admin theme? Or should we do that here?

Isn't that in direct contradiction of #3469686: Changing default theme can break HTML forms generated by XB API routes: always negotiate the Stark theme? 😅 Or are you reconsidering what you wrote at #3469686-4: Changing default theme can break HTML forms generated by XB API routes: always negotiate the Stark theme, and we should use Claro instead? Or perhaps you're distinguishing between the form in the right sidebar (stark) and the media library dialog (claro)?

Rather, it's an image field using the media library widget.

😮 That surprises me!

That's not what media_library_storage_prop_shape_alter() specifies. Can you point me to where in the MR that is happening? 🙏

#41: aha, great, so it's

Or perhaps you're distinguishing between the form in the right sidebar (stark) and the media library dialog (claro)?
👍

So the widget works in the prop form and can very much update a component, but getting this process to work well in E2E proven a bit tricky.
@Wim Leers said he'd have a look at this so assigning to him. Do a grep for @yo-wim and you'll find explanations next to their relevant lines of (sometimes commented) code.

I was able to reproduce the same failure locally. 👍

Root cause found, it's a combination of:

1. #3450308: Create an Open API spec for the current mock HTTP responses having added an OpenAPI spec + a KernelEvents::RESPONSE subscriber to validate responses.
2. We already knew that, and I updated this MR 20 days ago to make /openapi.yml aware of this API route.
3. … but making the Media Library stuff actually work causes an AJAX request to be sent by Drupal's JS, which causes a POST request to that same route. 🚨 That's where things go wrong: the openapi.yml file only had an entry for GET …

Fix: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... — E2E test is now passing ✅👍

💀 But why was this so excruciatingly hard to find? Well, because:

1. \Drupal\experience_builder\EventSubscriber\ApiResponseValidator::validateResponse() throws an exception …
2. … which then is not handled by anything else, resulting in the default AjaxResponse to be sent rather than the fully built one (which is done by \Drupal\Core\EventSubscriber\AjaxResponseSubscriber::onResponse()).
3. Consequence: only the debug log message that appears at /admin/reports/dblog revealed the nature of the problem:
   

   OpenAPI spec contains no such operation [/xb-field-form/node/1]
   

Creating critical follow-up issue for that …

And now the drupal_flush_all_caches() is gone, too!

I think @bnjmnm can push this forward to finish the test coverage now 😊 — I'll happily continue reviewing/pushing this forward tomorrow!

I didn't push it forward today because other issues required my attention. To be continued on Tuesday 😊

As far as I'm concerned, this is now ready. 12 open threads remain. I'd like @bnjmnm to sign off on those, and then this can/should IMHO land.

Epic work here, @bnjmnm! 👏

(Next up after this: styling the Media Library … but to match what? Claro's Media Library/modal dialog styles?)

I just tried testing this and it didn't work:

1. switched to mr branch
2. cd ui
3. npm i
4. npm run build
5. delete xb component config
6. drush cr
7. drag image component to desktop area
8. click "choose file"
9. error popup
10. tried again with the existing image in the desktop area and got popup there too

Moving back to needs work.

Just tested again and see 2 errors in the logs:

TypeError: _semi_coupled_get_variable(): Return value must be of type object|array|string|bool|null, int returned in _semi_coupled_get_variable() (line 286 of /var/www/html/web/modules/contrib/experience_builder/themes/engines/semi_coupled/semi_coupled.engine).

Drupal\Core\Render\Component\Exception\InvalidComponentException: [image] String value found, but an object is required in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of /var/www/html/web/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).

#50:
Did you see the Warning: This site has JS aggregation enabled. The media library widget will not work unless it is disabled. This will, of course, be addressed soon. message that @bnjmnm added? (Follow-up issue created to get rid of it: #3471974: Remove obsolete message about Media Library widget not working when JS aggregation is enabled.)

It works fine here:

(And zero warnings or exceptions being logged. If you find the precise STR for them, please create a new issue.)

⚠️ Note that the uploaded image does not show up because the public:// URI is being used rather than the corresponding computed http(s):// URL. @tedbow is working to fix that over at #3469436: Fix the visually broken "image" component instance: use FileUriItem's computed `url` property, not the stored `value` property.

Created #3471978: Make the Media Library dialog look like the admin theme without materially affecting anything outside of the dialog (possibly use an iframe for CSS isolation) for #48.

Unfortunately, I'm going to probably have to bypass review/approval from:

• @traviscarden for the changes to /openapi.yml (and tiny refinement to ApiMessageValidator that removes the OpenAPI logged complaint about the /experience_builder.experience_builder route)
• @hooroomoo or @effulgentsia for the changes to the semi-coupled theme engine — while @bnjmnm has built most of that, sign-off from them would have been nice
• @hooroomoo or @balintbrews or @jessebaker for the changes to /ui

… because this issue is literally the #1 most important issue at #3454094: Milestone 0.1.0: Experience Builder Demo 😅

I want to land this this AM. So I asked @jessebaker to review this in the next hour. Otherwise, I'll go ahead and merge this. It's too important progress to let get stale.

(@traviscarden told me he was going to review last night but apparently he did not. @hooroomoo is out.)

Jesse's on it :)

@kristen pol

You'll get those errors if you don't have Media and Media Library modules installed.

Here are the steps that I followed with a fresh Drupal installation.

• Install Media and Media Library Modules.
• Install XB.
• Go to /admin/config/development/performance and uncheck Aggregate JavaScript files & save.
• Add Article content.
• Go to /xb/node/1 and test with image component.

One more thing,

If we upload an image with a space in its name(ex - image xyz.png) the following error is thrown.

Drupal\Core\Render\Component\Exception\InvalidComponentException: [image.src] Invalid URL format in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of /var/www/html/web/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).

#58 That was fixed in #3469421: `{type: string, format: uri}` disallows image URLs containing spaces, `uri` data type stores + returns invalid URIs! … 🤔 Can you create a new issue for that problem and include the exact filename that triggers it? 🙏

Done @wim leers.

Image src containing spaces throws error.

Unblocked @jessebaker on an environment issue, and that's how I realized that my GIF in #54 actually shows a problem: the "Add media" functionality in the right sidebar has Olivero styles … despite [#3469686} 😅

Fortunately, it works fine with just Stark too (pushed a one-line commit to fix it):

👆that now defers all styling considerations to #3471978: Make the Media Library dialog look like the admin theme without materially affecting anything outside of the dialog (possibly use an iframe for CSS isolation), as intended 👍

I've just approved this from the /ui code perspective. There is obviously more to be done here, but I see no reason not to go ahead and merge this as a significant step forward.

Great!

Merged in upstream not once, but twice: first Felix' #3470550: Component config entities are incomplete: missing entries for optional props, causing errors in ComponentPropsForm, then Harumi's #3458503: Improve the page hierarchy display!

Still green CI, and per #55, this shouldn't wait any longer — if the code owners disagree with the changes in the areas they did not approve, they are totally free to create follow-up issues 👍😊

See y'all in #3471978: Make the Media Library dialog look like the admin theme without materially affecting anything outside of the dialog (possibly use an iframe for CSS isolation) (which @lauriii prioritized for 0.1.0 — see #3454094-36: Milestone 0.1.0: Experience Builder Demo).