🇧🇪Belgium @wim leers

• @larowlan took my feedback from Monday and made it happen by Tuesday: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... — that's why this was legitimately RTBC! 👏 Awesome work here, @larowlan & @longwave 🙏🙏🙏
• Changes in /ui: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... points out that 📌 Maintain a per-component set of prop expressions/sources Active will rewrite most of those things; so changes in that area are basically fine to ignore — they're in service of achieving this issue's goal, they're not aiming to nail long-term code patterns.
• There is an end-to-end test 👍 … but this was still broken in actual usage, because XBTestSetup was pre-placing a Block component, which concealed the fact that the initial block plugin settings are missing. Details: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5.... Fixed in https://git.drupalcode.org/project/experience_builder/-/merge_requests/5....
• There were some API surface/code clarity concerns, as well as something that seemed innocent but revealed some gaps: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
• The #1 thing that's missing here: docs — they're absent from both the issue summary and this MR does not touch /docs/* at all. I didn't want to hold it up on that though, so I did a rough first pass that leaves us in a better place than if we'd merged this MR as it was, but we'll need to gradually tackle updating /docs/data-model.md, probably starting with 📌 Remove references to 'props' outside of SDC - use 'inputs' instead Active — after that is done, an overhaul becomes much simpler.

Please don't think I'm downplaying the quality of this MR. This MR is a huge leap forward! 🤩🥳 But in doing so, I wanted to make sure we kept our eyes aimed at a tighter API surface, a more precise definition of how XB works with components coming from various sources, etc. Because 🌱 [Meta] Plan for code components Active is being worked on right now, and will introduce a third component type very soon!

All subsequent refactoring for which this MR surfaces the necessity:

1. 📌 Maintain a per-component set of prop expressions/sources Active
2. 📌 Move SDC specific validation in ValidComponentTreeConstraintValidator::validate into the SDC source plugin Active — https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
3. 📌 Reconcile date format handling between FE and Evaluator Active
4. 📌 Introduce unit test coverage for both ComponentSource plugins (Block + SDC) Active
5. 📌 Remove references to 'props' outside of SDC - use 'inputs' instead Active — should also update all references to component prop in /docs/*, and possibly also mark ADR#2 as superseded and write a new one. This issue did the heavy lifting, but without that rename, all docs will remain confusing anyway.

wim leers → created an issue.

Thanks to @longwave, closed 📌 CI: also test against next minor: 11.2.x aka Drupal 11.x HEAD Active as a duplicate.

Well spotted!

… and also do generalize SDC's \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent::getClientSideInfo() to not return field_data but input.

This will allow BlockComponent::getClientSideInfo() to match that, and return the block plugin's default settings.

That will allow instantiating an SDC component or a Block component to happen in identical ways, and hence allow removing the work-around that 📌 Implement saving block settings forms Active added.

In #8, there's a solid reason for keeping PATCH support using the original semantics (the CLI tool): it'd trigger a full (config entity) save including validation. In contrast with an auto-save, which:

• does not validate (hence allowing invalid values)
• must leave the original (live) config entity untouched, and hence store these auto-saved values somewhere else

TBD: how we then do the auto-save: which HTTP method, URL, request body…

Discussed #6 through #9 with @balintbrews.

Our conclusion: take-away: keep everything for "code component" simple at first, ignore auto-saving support; do that in a subsequent sprint. Because getting this off the ground at all is more important than auto-saving. 👍

Also related to #6: @tedbow's write-up WRT auto-saving considerations over at #3500386-6: Code Components should render with their auto-saved state(if any) when rendered in the XB UI → .

Are we sure long term we will need this at all if rely on wse_config?

You're right: in principle not, because we'd just rely on wse_config's ConfigFactoryOverrideInterface. But there's still much uncertainty AFAICT on all things "config in workspaces", so consider it just for the worst-case scenario, where wse_config does not get us as far as we hope. (@traviscarden is working this quarter on writing the test coverage to help raise the confidence level on that front!)

We have requirement out auto-save js components but this seem like more a UX requirement than a specification for how this would handled as far as backend storage

Is this referring to the description in the issue summary?

Everything else you wrote is actually closely related to what I just surfaced in our call, and which I captured at #3499931-6: [PP-1] HTTP API for code component config entities → . I'll link to your excellent #6 from over there 👍

Turns out @effulgentsia essentially repeated in that meeting just now what he already wrote at #3500042-10: Auto-save code components → 😅 Sorry, Alex!

#10 touches upon the problem space I identified independently over at #3499931-6: [PP-1] HTTP API for code component config entities → — @effulgentsia essentially repeated what he wrote in #10 on a call just now.

(Sorry, Alex, I hadn't read this issue yet because it's not scheduled for this sprint.)

Thinking this through on a run, I realized there's 3 things we've missed:

1. major: sibling internal HTTP API issue for this new config entity, similar to ✨ HTTP API for code component config entities Active — see #3499931-6: [PP-1] HTTP API for code component config entities → .

   (EDIT: I see that this was already somewhat accounted for by Consider how this may reuse the work done in ✨ Config entity for storing code components Active and ✨ HTTP API for code component config entities Active . — but it would likely be simpler to first tackle just the config entity here and then tackle all internal HTTP API needs for both config entity types in ✨ Auto-save code components Active )

2. major: sibling issue for this new config entity, similar to ✨ Auto-save code components Active (see point 1)
3. minor: dependencies — asset libraries may need to depend on other asset libraries: not a problem for the scope of this issue; but we'll likely need that later

Complication: auto-saving

See if the work done in 📌 HTTP API to read+write PageTemplate and Pattern config entities Active can be leveraged.

The work that 📌 HTTP API to read+write PageTemplate and Pattern config entities Active did and was finalized in ✨ [PP-1] Integrate saving sections with the backend Postponed is not applicable/transferable to this issue, unfortunately, because of the auto-save requirement. Pattern config entities ("Sections" in the UI) are very different: they are NOT auto-saved, and hence they have a much simpler kind of communication between client and server.

Create an HTTP API for creating, reading, updating, deleting, listing code component config entities implemented in ✨ Config entity for storing code components Active .

Actually, it's not quite this simple, because we won't be updating this config entity directly: it'll be auto-saved (see ✨ Auto-save code components Active ) and then published from an auto-save state ( ✨ Publishing code components Active ).

Scope expansion: support this for "global CSS" too

We need the exact same saving functionality for the config entity being added at ✨ HTTP API for code component config entities Active as we do here.

Just discussed both of those 👆 with @effulgentsia, @lauriii and @tedbow in a call. @effulgentsia offered to clarify this.

@balintbrews' feedback about "compiled CSS/JS" has been incorporated.

(FYI: I've not lost track of this; @larowlan has just indicated that 📌 Implement saving block settings forms Active is more important to land first.)

I'd like to do the final pass on this; this is the most important/impactful MR of the month AFAICT!

I updated the issue summary + title based on my current understanding.

In doing so, I tried to unify hard requirements (e.g. 14. config management support) with future intentions and ambitions (45.1 page template variants, 26. Styleguide, ✨ Allow code components to import from npm packages Active …).

From that POV, I think @balintbrews' proposal at #18 for the concept of an in-browser code library makes sense. That'd be conceptually similar to https://git.drupalcode.org/project/component_library/-/blob/1.0.x/src/En....

Any final thoughts before I begin implementing this? :)

Related: ✨ Add an API for importmaps Active — being worked on by @larowlan :)

• Webkit merged it Jan 2, 2025: https://bugs.webkit.org/show_bug.cgi?id=279025
• Firefox only has an issue open: https://github.com/mdn/mdn/issues/622

#12++ … and that ties in elegantly with your proposal at #3499933-18: Storage for CSS shared across in-browser code components (and PageTemplate config entities in the distant future) → to introduce config-defined in-browser code libraries — that "list of quality, recommended packages" could be defined entirely as one or more in-browser asset libraries.

P.S.: literally yesterday:

The specification for this has landed in HTML and the implementation landed in Chromium and WebKit.

— https://github.com/mozilla/standards-positions/issues/1010#issuecomment-...

@balintbrews in #20: Thanks!

++

1. 👍
2. 👎 Because of 2 reasons:
   1. @lauriii states pretty definitively in #21 states again that it's theme-specific global CSS — so at minimum it'd require a dependency on a theme then. More about that below.
   2. That won't work for requirement 14. config management, because I could create code component A that assumes some global CSS on one site, and code component B in another environment that assumes other global CSS. When creating new config entities for XB, we must always do so in a way that requirement 14 is supported. That requires explicit dependencies. It almost makes me think that the global CSS should be stored as
3. 👍

@lauriii in #21:
Thanks for adding requirement 45.1 Page template variants 😊

The CSS should be tied to a specific theme.

If the "global CSS" is tied to a specific theme, then that means that the code component being built (using component-specific JS, component-specific CSS and theme-specific "global CSS") itself depends on that theme. This appears to be confirmed by:

If someone wants to reuse the same CSS in another theme, depending on their requirements, they could either set up a base theme with the common CSS or they could just copy the CSS to the other theme.

Great! This is an important clarification! 🥳 And it actually means that @longwave's original proposal from #5 ("just store global CSS in the PageTemplate config entity for a theme") is relevant again, and the lowest level of effort. But, it'd result in an auto-save entry that looks weird — at least until 📌 The pending changes API endpoint should list individual regions for global template changes Active allows seeing not "the (theme's) page template" as a unit of change to review/publish, but "a region in the theme's page template". From that POV, it'd then also make sense to expose "the theme's page template's global CSS" as its own line item in the "Review changes" UI.

However, the "theme-specific global CSS" should be prepared to support the new 45.1 Page template variants, and the global CSS is per theme, NOT per theme's page template variant. Given that, it follows that we should introduce a new config entity. Or … we just choose to accept that we'll have to lift it out of the PageTemplate config entity at the future point where we work on 45.1.

There's no explicit dependency to the global CSS in Drupal today. This assumption introduces many problems but I think we should keep it that way going forward because otherwise we'd be essentially introducing a 1:1 coupling of the theme and its components.

(Emphasis mine.)
Exactly, that coupling is a dependency which config entities must describe to not violate requirement 14. config management, but which you're stating we must somehow not do while still keeping things working reliably! 😬

How does this not conflict with your paragraph just before it? 🤔

💡 We could default to the JavascriptComponent config entity having an explicit dependency on the theme's global/shared CSS. That'd prevent problems for 14. config management. We could then still make it possible to sever those ties by providing an explicit or or button, which would literally just omit that one explicit dependency. That AFAICT gets you everything you want:

• maximum reliability (the default: dependency present)
• maximum reuse (an explicit action that omits the dependency, which it must be anyway, because transplanting components from one site to another requires a subset of site A's config to be imported into site B — and this is all very close to requirement 1.2 Design system (foundations)'s "shipped as a package" statement!)

they need to avoid hard dependency on the global CSS

Will the in-browser code component creation UX toggling the preview having the global CSS applied vs not, to make it possible to verify the fallback nature?

Exactly that is being tested? 🙈

    // Create a Pattern via the XB HTTP API, correctly: 201.
    array_pop($pattern_to_send['layout']);
    $request_options[RequestOptions::BODY] = self::encodeXBData($pattern_to_send);
    $body = $this->assertExpectedResponse('POST', $list_url, $request_options, 201, NULL, NULL, NULL, NULL, [
      'Location' => [
        "$base/xb/api/config/pattern/testpatternpleaseignore",
      ],
    ]);
    $expected_pattern_normalization = [
      'layoutModel' => [
…
      'id' => 'testpatternpleaseignore',
      'default_markup' => '<!-- xb-start-uuid-in-root --><div  data-component-id="xb_test_sdc:props-no-slots" style="font-family: Helvetica, Arial, sans-serif; width: 100%; height: 100vh; background-color: #f5f5f5; display: flex; justify-content: center; align-items: center; flex-direction: column; text-align: center; padding: 20px; box-sizing: border-box;">
  <h1 style="font-size: 3em; margin: 0.5em 0; color: #333;"><!-- xb-prop-start-uuid-in-root/heading -->Hello, world!<!-- xb-prop-end-uuid-in-root/heading --></h1>
</div>
<!-- xb-end-uuid-in-root --><!-- xb-start-uuid-in-root-another --><div  data-component-id="xb_test_sdc:props-no-slots" style="font-family: Helvetica, Arial, sans-serif; width: 100%; height: 100vh; background-color: #f5f5f5; display: flex; justify-content: center; align-items: center; flex-direction: column; text-align: center; padding: 20px; box-sizing: border-box;">
  <h1 style="font-size: 3em; margin: 0.5em 0; color: #333;"><!-- xb-prop-start-uuid-in-root-another/heading -->Hello, another world!<!-- xb-prop-end-uuid-in-root-another/heading --></h1>
</div>
<!-- xb-end-uuid-in-root-another -->',
…
    ];
    $this->assertSame($expected_pattern_normalization, $body);

… unless … you mean the case of default_markup intentionally being empty, because no preview is possible? That'd be the case if you'd create a Pattern consisting only of (found the ones with empty markup aka no preview by looking at /xb/api/config/component):

• block.shortcuts

But no, even then it works fine for me! 😬

So I'm afraid I still don't understand how to reproduce this/what's going on here 🙈 This is done using HEAD:

What am I missing? 🙈

I see. I read the #5 without re-reading the issue summary, where that was already spelled out. Makes sense! Sorry for the confusion.

Per https://git.drupalcode.org/project/experience_builder/-/merge_requests/5..., I'd like @longwave's thoughts here in particular because it's thanks to him we have docs/adr/0005-Keep-the-front-end-simple.md, and @larowlan's remark reveals that here we're deviating from that.

Introducing an explicit to allow identifying which things are known to be needed in the future, but explicitly omitted from the 0.3 scope.

This to me is definite polish, a nice-to-have, not a must-have for 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active .

Why?

Because:

• 99% of BlockPluginInterface implementations' forms do not have any validation at all — client nor server-side. The range of possible settings values allowed through the UI is constrained only by the HTML input element they picked (<select>, <input type="checkbox">, etc.)
• It's only since a few months (since 📌 Make Block config entities fully validatable Fixed ) that a few block plugins are fully validatable …
• … and it's only those few block plugins that have a precise config schema that would even allow for this! 😰
• I'm convinced that >50% of placed blocks contain invalid/nonsensical block settings, that only sort of work due to PHP's automatic typecasting — having worked on https://www.drupal.org/project/acquia_migrate → , and having seen the incredibly weird/convoluted/puzzling "settings" many placed blocks were accepting without complaining.
• Actually, even outside a migration context, the only reason block plugins' settings forms work at all is precisely because both the PHP typecasting magic in the form definition is matched with the Form API submission typecasting magic, and is matched with the block plugin's ::build() method's interpreting of the saved values

We'll need to update docs/components.md etc. as well 😊 Looking forward to making that all consistent and less confusing! 🤓😅

benjifisher → credited wim leers → .

#21: 📌 Rename component source plugin configuration keys for clarity Active fixed some of those "props" in the codebase :)

This MR is now looking way better than on Friday. I wanted to review+RTBC+merge, but @longwave's remarks alongside my own observations make me conclude that this is not yet ready to be merged. Two critical comments that I'm curious what y'all think about:

1. https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
2. https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

+1 for renaming ComponentPropsValues in a follow-up issue. Or … perhaps it actually would make help this issue simpler?

AFAICT the #1 thing slowing us down here is XB's SDC-centric naming of low-level infrastructure. If that were refactored away first, it'd allow us to land this MR with more clarity and confidence?

Unless I see comments articulating strongly why not to do that, that's where I'll start my day tomorrow: refactor away the pervasive "props" and "prop sources" in the XB field type. I think that will clear things up a lot.

#16 — but @lauriii and @longwave in #15 concluded , which implies theme-agnosticness?

#16 + #18: I want to keep 'style guide' out of this conversation. 🙏Nowhere near enough is known about it. We shouldn't risk missing 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active for something we don't know what it'll look like exactly yet, plus which we can easily refactor towards in the future.

#18:

1. What would "compiled CSS" be?
2. Can you provide an example of how two "code component" config entities depend on the same JS?
3. I see no plans in the GIFs/designs at 🌱 [Meta] Plan for code components Active for JS that is shared across code components?

Good point regarding: kernel vs functional. Realistic is important, so let's go for functional. AFAICT that means that XbConfigEntityHttpApiTest is now the example to follow?

RE: common API request: \Drupal\Tests\experience_builder\Kernel\Traits\RequestTrait::request() seems ideal? 😊

#3: 📌 Make AutoSaveManager::getAutoSaveKey static Active already landed Jan 17 :)

@longwave in #15:

editing global CSS will be done separately from editing the page templates, so it seems to make sense to keep the concepts separate on the back end as well.

This is the strongest argument to make it separate.

the global CSS will be applied whether or not a page template is currently in use.

That actually answers my question from #13: it should be theme-agnostic.

But it contradicts @effulgentsia's

@lauriii in #7 already said that global CSS is per-theme, which makes sense to me.

(I do not read that in #7, by the way. AFAICT "one stylesheet per theme" was merely in response to what @longwave was proposing in #5.

@effulgentsia in #14:

So the question in this issue is just whether the global CSS should be per-theme or not, and I think that's independent of whether code components are per-theme or not.

That's not independent though: a JavascriptComponent config entity needs to have all global CSS it relies upon to be loaded. That means it must depend on the config entity storing/providing that global CSS.

So if the global CSS is stored in the PageTemplate config entity, then necessarily the JavascriptComponent must depend on that page template's theme.

If the global CSS is not stored in the PageTemplate config entity, then it must live in another config entity, and then the JavaScriptComponent config entity must explicitly depend on that.

AFAICT that means this issue must introduce a new SharedCss config entity type (because "global" is a misnomer as @longwave points out in #15). But … how should it be identified? Perhaps we start with a "singleton" and default to default as the ID of the sole GlobalCss config entity?

Do we agree? If so, we should overhaul the issue summary to reflect the actually intended scope rather than the original scope. I already updated the title to match my current understanding — feel free to change it again if I misunderstood! 😅

Future questions this raises, which are all out-of-scope for this issue:

• How does a JavascriptComponent specify which SharedCss it relies upon, and how do we expose that in the UI?
• How do we guarantee that it works correctly across all themes? (AFAICT we don't/we can't/it's up to the JS component author?)
• If/when the PageTemplate needs some CSS in the future, it too could just start depending on a SharedCss config entity.

- The route must have a requirement for entity access to update pages, xb_page.update

This bit actually means that it's not just a list, it's a subset of the total list!

#6++ — I think all that is worth capturing in docs/ 😊🤓 Perhaps in docs/components.md?

#6++ — nice work, @larowlan!

I find both and fairly confusing. I tried to think out of the box. Thoughts on https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... ?

@effulgentsia at #10: great, I inferred the same ~8 hours ago at #3500761-7: [SPIKE] Allow code components to import from npm packages → (i.e. that this metadata/dependency information belongs in the config entity that ✨ Config entity for storing code components Active is introducing).

I wholeheartedly agree with @longwave in #11.

@lauriii in #12: there's not a single word in the 64 XB product requirements about variants for page templates. Only for content type templates, the concept of "variants" is stated. Route-specific choices should be achieved using requirement 41. Conditional display of components per the requirements. If your thinking has evolved, can you please add a new requirement there? 🙏 I don't see how any one of us could possibly have known your intent to support "page template variants".

(Not that I think that supporting PageTemplate variants would be complicated: it'd only require changing \Drupal\experience_builder\EventSubscriber\RenderEventsSubscriber::onSelectPageDisplayVariant(). But whether variants for some subset of routes should be to the "master variant" for a theme (completely different component trees for all regions and/or completely different global CSS), or whether it'd just be targeted overrides (completely different component trees for some regions and/or additional global CSS, on top of what the "master variant" contains) impacts everything here a lot. We need to deliver 🌱 [Meta] Plan for code components Active by the end of February, this issue is not the time nor place to start exploring potential future things.

There needs to be some place where it is possible to store design tokens that are true across the whole system.

Global CSS is an orthogonal concern to design tokens. Design tokens is not being worked on for 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active . Design tokens may be exposed through CSS, but that's an implementation detail for later. Very little is known/defined for how "design tokens" will appear in the UI and how they impact other XB functionality. Let's stay focused on the task at hand 🙏 The issue summary here is about the ability to store (not even load!) CSS rulesets that apply to >=1 in-browser code components.

AFAICT the question that truly needs answering here for this issue's scope is: If the answer is:

• "theme-agnostic": then we cannot store the global CSS for in-browser code components in the PageTemplate config entity, because a PageTemplate config entity is necessarily theme-specific.
• "theme-coupled": then we can store the global CSS for in-browser code components in the PageTemplate config entity.

Done.

Alright.

• As of https://git.drupalcode.org/project/experience_builder/-/merge_requests/5..., we're reusing a subset of the SDC JSON schema to validate propsand slots in this new config entity 👍
• … but as the expanded test coverage in https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... demonstrates: that is insufficient, because the SDC JSON schema does not validate everything!
• To validate everything exactly the same, we need to actually reuse ComponentValidator. I'll push the commit for that as soon as the one from point 2 has failed tests. That'll make it green.

Back to Felix & Ted. I've demonstrated how to reuse the SDC schema, and how to ensure that this new config entity type in XB can never go out of sync with SDC, to ensure long-term compatibility 👍

I hear that

🙏 This needs to be validated against the SDC json schema, because these are essentially config-defined SDCs. https://git.drupalcode.org/project/drupal/-/raw/HEAD/core/assets/schemas...

was not clear to neither @f.mazeikis nor @tedbow.

The discussion on the issue is crystal-clear: both @balintbrews and @effulgentsia clarified above (#17 + #19) that these “JS code components” must have the exact same syntax and semantics for props and slots. Duplicating that validation logic from the SDC subsystem A) risks deviating, B) is pointless duplication.

The technical implementation of how to achieve that is a bit tricky:

• Drupal (config) entities are validated by validation constraints, not by JSON schema (which SDC uses)
• Config entities' validation constraints are defined in config schema.
• A single validation constraint can apply to a single value, or an entire tree of values. So we must define a new validation constraint for the props key-value pair that this new config entity will contain, and "just" apply JSON schema validation to it.

I'll get that started.

📌 Implement adding component to empty layout without dnd Active has been merged.

But … #9: I found a remaining random failure, that is not related to drag-and-drop:

FYI: when this was merged, component-operations.cy.js failed during the 0.x test run 🙈 But it failed ~20 LoC earlier than the change here.

Failure:

      cy:command ✔  click	
        cy:fetch ➟  GET http://localhost/web/xb/api/config/pattern
                    Status: 200
      cy:command ✔  get	.primaryPanelContent
      cy:command ✔  within	
      cy:command ✔  findByText	Sections
      cy:command ✔  click	{force: true, scrollbehavior: false}
      cy:command ✘  findByText	The Entire Node 1
      cy:command ✘  assert	expected **findByText(`The Entire Node 1`)** to exist in the DOM
                    Actual: 	"findByText(`The Entire Node 1`)"
                    Expected: 	"findByText(`The Entire Node 1`)"

Relevant test code:

    cy.openLibraryPanel();
    cy.get('.primaryPanelContent').within(() => {
      cy.findByText('Sections').click(clickDefault);
      cy.findByText('The Entire Node 1').should('exist');
    });
    cy.get('.primaryPanelContent')
      .as('panel')
      .should('contain.text', 'The Entire Node 1');

— #3500542-9: Implement adding component to empty layout without dnd →

FYI: when this was merged, component-operations.cy.js failed during the 0.x test run 🙈 But it failed ~20 LoC earlier than the change here.

Failure:

      cy:command ✔  click	
        cy:fetch ➟  GET http://localhost/web/xb/api/config/pattern
                    Status: 200
      cy:command ✔  get	.primaryPanelContent
      cy:command ✔  within	
      cy:command ✔  findByText	Sections
      cy:command ✔  click	{force: true, scrollbehavior: false}
      cy:command ✘  findByText	The Entire Node 1
      cy:command ✘  assert	expected **findByText(`The Entire Node 1`)** to exist in the DOM
                    Actual: 	"findByText(`The Entire Node 1`)"
                    Expected: 	"findByText(`The Entire Node 1`)"

Relevant test code:

    cy.openLibraryPanel();
    cy.get('.primaryPanelContent').within(() => {
      cy.findByText('Sections').click(clickDefault);
      cy.findByText('The Entire Node 1').should('exist');
    });
    cy.get('.primaryPanelContent')
      .as('panel')
      .should('contain.text', 'The Entire Node 1');

FYI: this also blocks ✨ Create a ComponentSource plugin for JS components Active .

@balintbrews in #17: great, that answers #16.2, which is the most critical bit.

#19: Thank you for this wonderful comment! 🤩🙏 Crediting @pdureau 😊

Which tests are you thinking of, exactly? The ones written in PHP? Can you cite an example of needless repetition?

This restored all tests that 🐛 Temporarily skip intermittently failing e2e tests Active temporarily skipped.

This is well on its way, but not yet ready for review. The demo I saw @jessebaker do in private is 🤩, and a demo of the end result should definitely be made available here too! 😄

Removing the security tag because it makes this issue listed on the project page as as a publicly disclosed security issue which this isn't since the security implication has to do with what's being done in this issue.

🤯 Since when is that the case?! 🤪

That solution might stop working once 📌 [later phase] ApiPreviewController must use the previewed route's controller, and override canonical content entity routes' received entity object Postponed happens. But we'll cross that bridge when we arrive there.

More short-term concern: no ComponentSource plugin should be aware of the global/request context, which this would violate. But as long as it's documented with an explicit caveat and bound to change prior to 1.0, then I'm fine with it as an interim step.

Long-term, I think this ought to be implemented as \Drupal\Core\Config\ConfigFactoryOverrideInterface. That's the mechanism that Drupal core provides for context-dependent overrides of stored configuration. Would that even be more work to implement? Because that'd set us up better for the future AFAICT. 😇

Yep, very much aware of that! :D

That's what I was referring to in #12. And it's that "require human intervention" question at the end I wanted to double-check you're on the same page for? 🤞

So nice to see this moving forward! 🤩🥳 May it save us all a lot of time! 😄

I'm not convinced a utility class is worth it, because I doubt (m)any other code paths will end up calling this. But … that's really negligible subjective commentary, and could very well change.

I can't RTBC this because:

1. PHPStan is not passing
2. PHPUnit tests are not passing
3. I could fix those two, but the #1 thing this should be getting us, is the more helpful exception message appearing in the dialog shown in the issue summary. So I modified the return value for \Drupal\experience_builder\Controller\ApiLayoutController::__invoke() by renaming one of its 4 key-value pairs to be a wrong key. This triggers an OpenAPI error as expected, but that doesn't appear in the dialog:

Then again, I shouldn't have been surprised, because @balintbrews' issue summary clearly states:

On the front-end, that means translating the JSON into the error modal and console log.

But I'm not quite sure how an error message is supposed to show up, given that even

diff --git a/ui/src/components/error/ErrorBoundary.tsx b/ui/src/components/error/ErrorBoundary.tsx
index 5f649468..0e98257a 100644
--- a/ui/src/components/error/ErrorBoundary.tsx
+++ b/ui/src/components/error/ErrorBoundary.tsx
@@ -60,7 +60,7 @@ export default ErrorBoundary;
 
 const getRouteErrorMessage = (error: unknown): string => {
   if (isRouteErrorResponse(error)) {
-    return `${error.status} ${error.statusText}`;
+    return 'yo';
   } else if (error instanceof Error) {
     return error.message;
   } else if (typeof error === 'string') {

results in nothing appearing. So marking only for the back-end changes; once those are done, please assign to @balintbrews so he can tackle the front-end bits in a fraction of the time it'd take me 🙏😊

Ah, this is literally point 1 in the issue summary. But I'm surprised that #3499927 was created without taking this into account and this issue's title doesn't convey that it's a spike for future functionality and it doesn't link that issue. 😅

1. For now, we can hard-code a CDN (e.g., https://esm.sh/) to map them to. In the future, we can make that CDN configurable.

Blindly trusting a single external site is a serious risk: performance, reliability but certainly also security! Especially when they're maintained by a single person. I'd not want to do this unless we at minimum do https://developer.mozilla.org/en-US/docs/Web/Security/Practical_implemen...

But apparently supporting this for import maps is pretty much brand new: https://jspm.org/js-integrity-with-import-maps, published August 5, 2024, released in Chrome 127. But that's only got 78.7% global support right now; it's not supported at all in Firefox for example: https://caniuse.com/mdn-html_elements_script_type_importmap_integrity

🥳 for more reliable tests! (And running the entire test suite again.)

quietone → credited wim leers → .

IDK who/when/where this happened but there still is a “Publish” button next to thr “Review changes” dropdown 😅🙈

This issue exists to complete that work and make block settings changes in the UI propagate to the back end including data storage and preview.

(Emphasis mine.)

The preview part is working.

The storage part is not. It's only auto-saving (to ephemeral storage), not really saving (to an XB field or the PageTemplate). Which means it's not really stored, nor passing validation.

Besides that, I am concerned about:

1. the deviation from the previously established terms "explicit inputs" and "implicit inputs": https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... — block settings are explicit inputs for that the "block" component source. As captured by this bit in the issue summary:
2. the (undocumented) meaning of supportsProperties and supportsDynamicProperties: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

Perhaps it's worth meeting on Monday morning about this, @longwave, to figure out the naming and docs? Happy to write those together!

I see! But OpenAPI validation should be catching that 😬 And the issue summary is talking about exactly that? Even though the screenshot in the issue summary makes no sense to me because:

  /xb/api/config/pattern:
    description: TODO
    get:
…
    post:
      description: TODO
      requestBody:
        description: Create new Pattern/Section.
        content:
          application/json:
            schema:
              type: object
              properties:
                name:
                  type: string
                  description: Human-readable Pattern/Section label
                layout:
                  type: array
                  items:
                    $ref: '#/components/schemas/LayoutComponent'
                model:
                  $ref: '#/components/schemas/Model'
      responses:
…

👆 clearly, default_markup is not expected in the request body, only in the response body (and that is asserted in the test).

Actually, it should be forbidden, but it's not yet 🙈 It's missing:

diff --git a/openapi.yml b/openapi.yml
index 5e692f20..56af628a 100644
--- a/openapi.yml
+++ b/openapi.yml
@@ -514,6 +514,7 @@ paths:
                     $ref: '#/components/schemas/LayoutComponent'
                 model:
                   $ref: '#/components/schemas/Model'
+              additionalProperties: false
       responses:
         '201':
           description: Successful save of Section/Pattern

So … I'm still very confused 😅 (Pushing this OpenAPI change btw.)

Not sure how to test this, but it feels like if we could enable api validation in cypress tests we'd get it for free.

That should be the case ever since 📌 PHP asserts not running in CI/during tests Active — unless for Cypress tests some Composer packages are not being installed. That'd be a trivial fix :)

Looking great! for:

• Requesting to restore a DX improvement that 📌 [PP-1] Send page data to Drupal for storage in auto-save store Postponed: needs info landed yesterday that this MR is reverting.
• Deleting one now pointless file.

+1 for idea, but lots of tests not yet passing. Pushed one commit that I believe will fix many things, but not yet sure how many.

Aha! Let's get that fixed! 😄

Opened 📌 Automatically use radio buttons for "small enums" and dropdown for "big enums" Active with an MR that automatically uses input[type=radio] instead of select when there's >3 values in an SDC's enum. 🏓 That will allow (necessitate, even!) writing a test in prop-types.cy.js! 😊

Based on prior discussions, I always thought we'd be building in-browser defined SDCs. That's what we had @tedbow do the https://git.drupalcode.org/project/experience_builder/-/tree/research_sd... PoC for.

Now, this is different than that PoC, because we're assuming components to be entirely JS-defined, whereas SDC is pretty heavily assuming Twig.

Based on the issue summary, it seems like we're now saying we should be loosely matching the architecture of SDC, but not be aiming to have these exposed as SDCs? That raises many subsequent questions:

1. Where can I read the rationale behind that decision?
2. Which aspects are expected to be identical to SDC, and which ones are not?
3. Of the aspects that are expected to be identical, do we expect to chase upstream SDC changes (i.e. commit to maintaining compatibility)?

We need clear answers to these questions are crucial to ensure we architect this new config entity right from the start: wherever we're matching SDC, we SHOULD reuse SDC infrastructure; wherever we're not, we MUST explicitly document what/how/why.

#7 doesn't really answer my "different beast" remark. 😅 @lauriii told me in DM that we definitely need that eventually. Fine!

I don't know enough about JS import maps yet, nor why we need them (presumably for JS components, i.e. many JS components might all need the same import map?). And perhaps we don't know enough about it in general yet, given ?

Global CSS is comparatively simpler, so let's restrict this to just that. 🙏

wouldn't it be pretty straightforward to migrate to whatever format we decide in future?

Absolutely! :) Not even migrate, but update :D (Update path tests for config entities are a solved problem 😃)

wim leers → created an issue.

Thanks for doing that chore, @larowlan — I'd have done it myself but ran out of time (and steam) yesterday.

FYI @amangrover90: once you want me (or somebody else) to review this, please change the d.o issue status to — I check that multiple times per day! :)

Unpostponed ✨ Provide a way to create a new page Active 👍

🐛 Controllers performing data modification should make use of CSRF tokens via /session/token Active landed.

#5: hm … so then the entity would exist immediately, but need to be invalid, because the
required title cannot yet be specified by the author.
Plus, it'd need to be marked as unpublished (EntityPublishedInterface::setUnpublished()).

I'm sure we can make all that work at the technical level. I'm not sure what the UX would look like exactly?

Looks beautiful! Just a nit about the class being wrongly/confusingly named now. But that will likely evolve further in the future, so not worth doing a follow-up MR IMHO 👍

Thanks for those! 🙏

I quite liked/was impressed with that "hacky dance" 😅🙈 IDK what that says about me 🤣

Changing component per

have the FE not send values for unchecked checkboxes

✨ Leverage HTML comment annotations, remove wrapping div elements Active has also happened! Next steps:

• ✨ Refactor React Context (ComponentHtmlMapContext) to Redux to allow for better extensibility Active
• 📌 Only run XbTwigExtension inside XBPreviewRenderer Active

Don't worry yet about that — the link in the toolbar is a temporary measure anyway. This issue is really only about the generalized opt-in configurability, not about generalizing the full functionality :)

This is starting to shape up nicely! 😄 Thanks for pushing this forward! Key missing thing: a kernel test, which should confirm the bug I identified 😇 🤞

🤯🤩

#2 fixed that :)

https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... ← no need to start from zero for config schema!

This blocks an entire chain of work, all the way up to #3499988-10: [PP-3] Editing code components → !

This is blocked on ✨ Managing code components in library Active , which is blocked on ✨ HTTP API for code component config entities Active , which itself is blocked on ✨ Config entity for storing code components Active .

… but it looks like @hooroomoo is able to get at least UI changes in place, while awaiting an HTTP API to talk to? 😄

This is blocked on ✨ HTTP API for code component config entities Active , which itself is blocked on ✨ Config entity for storing code components Active .

This blocks ✨ HTTP API for code component config entities Active , which blocks many other things in 🌱 [Meta] Plan for code components Active .

This blocks ✨ Managing code components in library Active .

Make sure the data matches the shape of how ✨ Config entity for storing code components Active expects it. (Saving is out of the scope of this issue.)

… but per that issue, it'll be exactly like SDC props. So that means … that this can get started ahead of that issue being done? :)

#5++

That addresses this concern by @effulgentsia:

I think our initial implementation can be based on having just one, but I think conceptually having a config entity type for this that could in principle allow multiple config entities of that type would be good.

But

Also, would it make sense for the same entity to have any other property besides the CSS? For example, another global or semi-global thing might be a javascript import map (mapping bare package names of dependencies that the JS components might want to use, […]

is a very different beast. I'd rather keep the scope restricted to just CSS.

If we can do that, then I think @longwave's idea works.

This blocks ✨ Provide a way to create a new page Active .

Definitely is blocked on 🐛 Controllers performing data modification should make use of CSRF tokens via /session/token Active .