🇧🇪Belgium @wim leers

Finished everything in #22, except for the docs (only a stub was added).

Iterated on everything @larowlan did overnight, and between his work and mine, we surfaced a bunch of oversights in both ✨ Config entity for storing code components Active and the way that even the foundations of Component config entity type's support for ComponentSource-specific settings were not quite right yet. Which is understandable, because it's this issue that's introducing a 3rd ComponentSource plugin (and as the adage goes: an abstraction requires 3 concrete uses), and one that is quite different in some ways and almost the same in others!

I've spent most of today increasing test coverage, and in turn revealing weaknesses in our validation and hence the reliability of the Component config entity type in combination with the new ComponentSource this MR is introducing. Those weaknesses are important to eradicate to remain on track for requirement 14. Configuration management.

The one bit where I'm going quite strongly against what @larowlan did last night is https://git.drupalcode.org/project/experience_builder/-/merge_requests/5..., because by using ContribStrictConfigSchemaTestTrait, it became clear that all of those edge cases @larowlan was testing … actually are made impossible by the validation for the JavaScriptComponent config entity, and for good reason! (See the commit message.)

I believe that now, especially after the source-specific test coverage (which revealed missing validation) and its fixes (A — note how this causes the exact same fails + B) everything is in place 🤓

The only thing I have not reviewed at all is all rendering aspects: the astro island and asset generation. But for those, I'm happy to defer to @longwave & @larowlan. I want to get the config bits to be as robust as possible as soon as possible because they affect the data model/update path/overall stability — tweaks to the rendering can easily happen in subsequent MRs.

So: approved this MR! Hopefully @larowlan & @longwave think this is RTBC by tomorrow 🤞😊

Unblocked: #3502819-3: Connect "new page" with API to create new pages → , and reduced postponedness level on 2 others 👍

✨ Create a navigation modal for changing pages in the editor Active is in!

✨ Create a navigation modal for changing pages in the editor Active is in!

✨ Create a navigation modal for changing pages in the editor Active is in!

It looks like I was wrong → :

• @larowlan did implement the "auto-create upon save" functionality, just not in ::postSave(), but in the new JavascriptComponentStorage 👍 Only thing that needed changing: respect the status flag that ✨ Config entity for storing code components Active assigned a particular meaning to.
• He also added explicit test coverage for it, in JavascriptComponentStorageTest. Still, updating testComponentCreation() the way that I did provides value: it ensures that identical props for an SDC and a "code component" result in the exact same prop_field_definitions 👍

I did find some problems though, and I've fixed those.

I think I won’t be able to push this forward the next few hours. The only remaining thing I have partially (locally) is schema refinements, but there’s plenty more to do that won’t touch that.

#30: which of those belong in 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active and which are must/should/nice-to-have?

Finally: could you please upload a screencast showcasing this awesomeness so I can blog about it? 🤓🙏

+1 from me, but @bnjmnm is the long-time lead of Redux-integrated Field Widgets, so I'd like his explicit +1 :)

I don't feel strongly, I was just explaining the rationale!

If you got confused by it, and it sounds like #2 does not convince you.

So: you decide! 😄 What did you expect? What would've caused the least friction for you? Name it so! 👍

#2++

Only after that is fixed, should we tackle this one with an explicit tests. That other issue should fix this too :)

Thanks, @heyyo & @lauriii! 👍

SO great to see @larowlan continued where I left off 🤩

• Favorite changes are the ComponentSourceInterface API tightenings by @larowlan — best example: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... — done in a way that does not balloon scope and https://www.drupal.org/project/experience_builder/issues/3498889#:~:text... → remains true 👍
• Worried: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... and https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... implement the majority of 📌 Capture all reasons why particular SDC is incompatible Active — I'd definitely have considered that out-of-scope 😅 That's how we went from 14 files changed, 855 insertions(+), 742 deletions(-) to 30 files changed, 1632 insertions(+), 915 deletions(-) 😳 Postponed the other issue: #3473275-13: [PP-1] Support disabled Block Components + multiple reasons for incompatibility → .
  (And related: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5....)

But it looks like @larowlan tackled things I had not even identified in #22, so I can literally work on the list I created there 😄

@larowlan has actually independently implemented overnight an alternative to what this MR was aiming to do WRT storing reasons. But this MR so far contains zero LoC for the storing of reasons, only for making the Block component source auto-creation support better.

So, postponing this on ✨ Create a ComponentSource plugin for JS components Active , because it'll tackle a big chunk of the work this would need to do!

A new challenge apepared: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... for ✨ Create a ComponentSource plugin for JS components Active .

Remember that the way XB's UI is able to determine which parts of the (server-side rendered!) preview markup correspond to which components and slots is powered by HTML comments. Quoting /ui/tests/unit/function-utils.cy.js, it looks like this:

 <!-- xb-start-ad3eff8e-2180-4be1-a60f-df3f2c5ac393 --><div data-component-id="experience_builder:two_column" data-xb-uuid="ad3eff8e-2180-4be1-a60f-df3f2c5ac393">
          <div class="column-one width-25" data-xb-slot-id="ad3eff8e-2180-4be1-a60f-df3f2c5ac393/column_one">
            <!-- xb-slot-start-ad3eff8e-2180-4be1-a60f-df3f2c5ac393/column_one --><!-- xb-start-9bee944d-a92d-42b9-a0ae-abae0080cdfa --><h1 data-component-id="experience_builder:heading" class="primary" data-xb-uuid="9bee944d-a92d-42b9-a0ae-abae0080cdfa">A heading element</h1>
<!-- xb-end-9bee944d-a92d-42b9-a0ae-abae0080cdfa --><!-- xb-slot-end-ad3eff8e-2180-4be1-a60f-df3f2c5ac393/column_one -->
        </div>

          <div class="column-two width-75" data-xb-slot-id="ad3eff8e-2180-4be1-a60f-df3f2c5ac393/column_two">
            <!-- xb-slot-start-ad3eff8e-2180-4be1-a60f-df3f2c5ac393/column_two --><p>This is column 2 content</p><!-- xb-slot-end-ad3eff8e-2180-4be1-a60f-df3f2c5ac393/column_two -->
        </div>
    </div>
<!-- xb-end-ad3eff8e-2180-4be1-a60f-df3f2c5ac393 -->

(Note the xb-start- and xb-slot-start-.)

And like this:

                            <!-- xb-start-fce5e0e3-175f-48b5-a62c-176dbc5f3e91 -->
                            <div data-component-id="experience_builder:my-hero"
                                 class="my-hero__container">
                                <h1 class="my-hero__heading">
                                    <!-- xb-prop-start-fce5e0e3-175f-48b5-a62c-176dbc5f3e91/heading -->
                                    There goes my hero
                                    <!-- xb-prop-end-fce5e0e3-175f-48b5-a62c-176dbc5f3e91/heading --></h1>

(Note the xb-start- and xb-prop-start-.)

While editing components and placing them in slots requires only the annotations from the first kind (component + slot), SDC-sourced XB components actually already have their props annotated too, which will enable us to provide (visual and otherwise) affordances connecting inputs in the ComponentInputsForm form to very specific bits of markup in the preview. We're just not doing that yet. That work landed in preparation for 📌 Implement endpoint for realtime preview Active .
Because it's not yet actively used in XB, I explicitly +1'd not holding up #3498889 for that.

@larowlan pointed out in https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... that we can keep generating xb-start- and xb-slot-start- HTML comments, but that xb-prop-start- is not actually possible. How did the folks working on the PoC envision supporting that?

AFAICT the closest issue to that 👆 is ✨ Editing components sourced as code components Active . But I figured I'd post on the meta/plan issue to avoid potentially derailing/blurring the scopes of #3500081. 😊

wim leers → changed the visibility of the branch 0.x to hidden.

FYI: \Drupal\experience_builder\Entity\JavaScriptComponent::$machineName was specifically chosen to match SDC's core/assets/schemas/v1/metadata-full.schema.json#properties.machineName

As documented in config schema:

    # See core/assets/schemas/v1/metadata-full.schema.json#properties.machineName
    machineName:
      type: machine_name
      label: 'Machine Name'
      constraints:
        Regex:
          pattern: '/^[a-z]([a-zA-Z0-9_-]*[a-zA-Z0-9])*$/'
          message: 'The %value machine name is not valid.'

Also merged in upstream and resolved conflicts, because this conflicted heavily with 📌 Remove references to 'props' outside of SDC - use 'inputs' instead Active .

Should make for a smooth start for the next person 😊

Delivered on much of #21, but not everthing yet. EOD, so passing context + next steps on to the next person:

1. Managed to lift 600 LoC out of \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent into GeneratedFieldExplicitInputUxComponentSourceBase. I kept it as simple as possible, and resisted doing significant refactoring 🤓
2. This made SingleDirectoryComponent merely ~275 LoC, and there's likely more still to be done.
3. Similarly, extracted the prop_field_definitions setting for the SingleDirectoryComponent ComponentSource plugin out of its config schema type into a new config schema base type: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
4. Which then allowed me to quite simply/elegantly introduce the JsComponent ComponentSource: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

Next steps:

1. auto-creating Component config entities based on JavaScriptComponent config entities. @longwave suggested automatically doing that whenever a JavaScriptComponent config entity is saved. I agree that makes sense. This would be a ::postSave() override in \Drupal\experience_builder\Entity\JavaScriptComponent.
2. The easiest way to test is to expand \Drupal\Tests\experience_builder\Kernel\Config\ComponentTest — which was previously expanded in 📌 Add support for Blocks as Components Active to test Block-sourced Component config entities.
3. 📌 Introduce unit test coverage for both ComponentSource plugins (Block + SDC) Active would make writing tests for this a hell of a lot simpler. ⚠️ But I think we explicitly should keep that out of scope to ensure we land this in the next few days rather than growing it. Let's add unit tests in #3475584.
4. Docs: updating docs/components.md!
5. I'm sure there's more, but those are the first steps :)

P.S.: I did not test \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::renderComponent() at all, I just matched what @larowlan did in \Drupal\Tests\experience_builder\Element\IslandTest. It likely needs updating 😅

I strongly prefer option 1, because it keeps internal consistency.

#14 needs an answer from @lauriii.

Everything in this MR so far has focused on the astro island/rendering/assets aspects.

But there's more to this issue:

1. The issue summary states clearly that there must be a new ComponentSource plugin.
2. I asked @balintbrews this in #7 and he responded affirmatively:
   

   my code components appear in the left-hand Library in the XB UI

Working on those two aspects.

Unblocked @f.mazeikis: made the test pass (https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...) that he was stuck on — this was an oversight in #3499927.

Also reviewed this MR while on it anyway — this looks well on track 👍 Let's get this wrapped up today? 😊

Related: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... — in ✨ HTTP API for code component config entities Active .

This is a new requirement; hence cannot be a bug. This relates to the whole "content region should never be made editable in XB and should always be assumed to contain only the main content block" thing that you wrote in the issue queue, but which was absent from both requirements and designs.

IMHO this should be merged with 📌 Consider refactoring page_template into page_region(s) Active , see explanation in #3501600-7: Consider splitting 1 PageTemplate config entity into N PageRegion config entities → and subsequent comment.

Given @lauriii specifically opened #3502372, requesting him to approve #7 + #8.

To add to #7: that'd also mean that the content region would either:

• never get a PageRegion content entity, because that region is hardcoded (in code) to contain the "main content" block
• always get it, but we'd never expose it and it'd be a de facto hardcoded config entity

Related: 🐛 Hide main page content block from XB Active

✨ Provide a way to create a new page Active is in!

Marked because:

• Some of the same problems present in #3500046's MR are present here too.
• This MR doesn't yet do what this issue's title and summary say WRT "updatable entities"
• I'm confused by #3500046 had the ambition to be as xb_page-agnostic as possible but not this one? 🤔
• Concerns about the way pagination is currently implemented.

FYI, personally I don't think pagination must be implemented in this issue — it could be implemented in a follow-up. This issue is meant to get things off the ground, and IMHO it's fine to crash hard when you have e.g. 100 xb_page entities during these early stages. We do the same thing for \Drupal\experience_builder\Controller\ApiConfigControllers::list()!

IOW: pagination is important in production, not in early development.

Next up: ✨ Provide an API for listing available pages Active .

Found a number of critical problems that would've caused problems in future issues that'll build upon this one:

1. OpenAPI schema was incorrect
2. no test coverage for authorization (which is necessary here, but not for XB's config entity HTTP API tests, because those do not yet have per-config entity type permissions)
3. incorrect catching and recasting of exceptions

I fixed all of these (plus more minor things). Please look at the MR and search for to check out the most important remarks and the fixes, to avoid repeating them in the future 🙏

@longwave: could you push this one across the finish line? 🙏 ✨ Provide a way to create a new page Active mistakenly was extending the ApiControllerBase class, and this MR is removing it, which would've prevented that understandable confusion!

Ultimately there will probably have to be different instructions for people wanting to contribute/commit to the project and for those that are simply trying to casually use a pre-release to see what's coming.

Yep, this!

For now, the pre-releases you can easily try are those at https://www.drupal.org/project/experience_builder/releases → — so currently only 0.1.0-alpha1. Subsequent 0.x.0-alphas will appear in the future, the first one at the end of February.

most code components won't have source CSS.

I know that, but the end result is still CSS, as you say. I was merely making a high-level comparison between "code component" and "generic asset library" in #35.

Wow, the merged MR looks nothing like what I reviewed a month ago: no more server-side changes!

After some fighting with GitLab to get this to merge, I won! 😜

Thanks, @amangrover90 & @mglaman!

wim leers → changed the visibility of the branch 3497000-seo-settings-for to hidden.

Per 📌 Remove references to 'props' outside of SDC - use 'inputs' instead Active 😊

Updated issue summary to make it clear that the scope was narrowed.

Retitling per https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... — because I previously pointed out that everything in this issue suggests it should be saveable. But apparently that's for a follow-up.

Ah, @bnjmnm chimed in and said as much: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

I don't think the title here is accurate though.

Did you confirm that it is in fact caused by our React rendering, and not a general bug? See https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... for how to rule that out.

No more need for review; it's been approved by both @lauriii in #6 and @mherchel in #9.

It now needs to be implemented.

I fixed the incorrect use of hook_preprocess_html(). But multiple bugs remain.

Plus, as @lauriii requested >2 months ago: there should be a minimal E2E test.

I manually ran through these steps locally twice, in a fresh environment, starting from /tmp.

I believe @luke.leber's original request:

Just more copy/pastable instructions!

is now finally met :)

(The only assumption left, that is not stated explicitly: run a web server. I think that's reasonable.)

FYI: 📌 CONTRIBUTING.MD is nearly impossible to follow without insider drupal knowledge Active was also done by @luke.leber, who opened this issue 😊 So most of the concerns reported here have already been addressed there.

@longwave in #4: DDEV instructions were added in 🐛 Clarify commands in CONTRIBUTING.md, and provide XB DDEV add-on alternate commands Fixed

Will implement @shrop's suggestion!

Fixed :)

we will probably need to think about dependencies between components when one component makes use of another, but we can equally punt that to a followup

+1, because that'd also require the config entity to know about those dependencies. That was not in scope for ✨ Config entity for storing code components Active (nor mentioned as future work, actually), and this is in the critical path for many other things. So let's stick to just individual code components in this issue 🙏

Something similar to core's AssetControllerBase that serves CSS/JS from the assets:// stream wrapper to serve the compiled css and js as stored in the config entity. […]

I think it could be much simpler:

• Generate those CSS and JS assets on disk, regardless of whether config is saved through user interaction or through config deployment — similar to https://git.drupalcode.org/project/component_library/-/commit/a78f3af960...

— ✨ Storage for global CSS Active

[…] We can then append ?preview=1 to the URL for the preview version […]

Why do we even need to load these assets while previewing? That'd require going from client to server and back to the client? The client-side already has literally all the information it needs (source+compiled CSS, source+compiled JS), I don't see why the server needs to be involved at all? 🤔

Un-assigning @larowlan because it's a holiday in Australia on Monday. He'll likely be the one to push this forward further, but that shouldn't prevent somebody from doing so until he returns :)

This issue summary does not explicitly state which JSON schema types should be available to pick from while defining props for code components.

Are you aware that type: array and type: object are both impossible to support at this time? Which means that this issue would be restricted to "primitive/scalar" JSON schema types.

Citing the relevant bit of config schema that landed in ✨ Config entity for storing code components Active :

          type:
            type: string
            label: 'Type'
            constraints:
              Choice:
                # Not all JSON Schema basic types make sense (for example: `null`).
                # @see https://json-schema.org/understanding-json-schema/reference/type
                - 'string'
                - 'number'
                - 'integer'
                - 'boolean'
                # @todo Consider adding `array` after https://www.drupal.org/i/3467870 is fixed.
                # @todo Consider adding `object`, but defining _arbitrary_ object shapes is not yet supported.
                # @see \Drupal\experience_builder\JsonSchemaInterpreter\SdcPropJsonSchemaType::computeStorablePropShape()

I did do s/original/source/ just now, to more closely match the final terminology used in ✨ Config entity for storing code components Active . 👍

#34: the answer is in the schema:

    # If not `null`:
    # - css will be stored on the filesystem at public://xb/asset_libraries/global/global.css
    # - will be taken into account for generating an opaque version identifier, to ensure aggregated CSS is never stale

Rationale:

• It's totally reasonable to have an asset library without CSS or without JS. Plenty of examples of that in Drupal core.
• It's impossible to have a "code component" without JS.
• It's not reasonable to have a "code component" without CSS — there'll always be something to style. In the super rare case that there wouldn't be anything, storing the empty string in the config entity is fine.

I'm very sorry, but 📌 Remove references to 'props' outside of SDC - use 'inputs' instead Active seemed both more important and like it could help make this more clear, because it'd ensure this MR would then make this change from a clearer starting point.

So I'm afraid this needs a reroll. But I'm very eager to get this in early next week :)

P.S.: please first check #3500994-15: Remove references to 'props' outside of SDC - use 'inputs' instead → + #3500994-18: Remove references to 'props' outside of SDC - use 'inputs' instead → — I think this issue is the perfect one to identify what could/should be clearer in docs/shape-matching-into-field-types.md.

I figured rather than delaying the landing of this major clarification by awaiting review from @longwave, @larowlan and/or others, I'd just merge this. This is a major improvement compared to the state in HEAD!

But please feel free to reopen this issue if you feel the docs are still not clear enough! 🙏 I'm confident that what I did is an improvement, but I certainly do not think that my docs are perfect 🙈

IOW: I merged for the sake of enabling the majority of us working on XB to move on from this :)

Docs

I was working on the docs parts — I think it can be a lot clearer: by lifting all of the “shape matching” and “prop shape” and “prop source” stuff OUT of data-model.md AND components.md, and into a new shape-matching-into-field-types.md.

That now means that those 2 pre-existing docs talk only about the general case, and leave the additional complexity for generating an input UX and storing those inputs for that new doc.

It also means that redux-integrated-field-widgets.md can now refer to that more tightly scoped new doc instead of to the waaay bigger data-model.md.

Stragglers

I fixed a bunch of "component prop"stragglers (including renaming some of those to "SDC props") — if you search for it new, you'll find only 3 occurrences, and all 3 are for React component props, which is indeed an accurate term there.

(This also helps make the Redux-integrated Field Widgets doc clearer, because I remember going back-and-forth with @bnjmnm about how confusing all the different "props" meanings were! 🤪)

Adding 📌 Implement saving block settings forms Active .

AHHHH! I thought I was going mad while testing 📌 Implement saving block settings forms Active because I did run into this but failed to reach this logical conclusion!

#12: Can you do those in an additional MR on the same issue (and do it on top of a squashed version of the current MR)? I'd like to keep the current MR focused on what it currently does, to keep it easy to review.

I'm surprised 📌 The pending changes API endpoint should list individual regions for global template changes Active isn't on here (for 6. Save (draft) content).

But AFAICT 📌 Consider refactoring page_template into page_region(s) Active is now likely to be the successor to that, but it'd fall under 19. Modify the page template (i.e. global regions). Tentatively adding it to "should-have", but I have a hunch that even though unplanned, @lauriii would now in hindsight mark this as "must-have" — so added it there.

Per #3501600-7: Consider splitting 1 PageTemplate config entity into N PageRegion config entities → , I now think it's quite likely we should not do this, and do that instead. Postponing until #3501600 lands and we're 100% confident we won't need (something like) this.

@longwave: well predicted! I see at least one nice bonus benefit — see bottom of that comment 😄

Remember: the future GlobalPageTemplate config entity 👻

📌 Introduce an XB `PageTemplate` config entity Active introduced the PageTemplate config entity (config schema type: type: experience_builder.page_template.*:). It comes with this comment:

# Otherwise, an Experience Builder Page Template can be created per installed theme, and stores component trees for each
# region in a theme's `page.html.twig`.

That "otherwise" refers to the preceding config schema type: a placeholder for a future piece of functionality that is very closely related: the global page template config entity.

That looks like so:

# At most a single entity of this type is allowed to exist. If it exists, it *replaces* the theme's `page.html.twig`.
experience_builder.global_page_template.*:
  type: config_entity
  constraints:
    FullyValidatable: ~
  mapping:
    # A single component tree.
    component_tree:
      # @todo This MUST contain a single XB Component that points to the title block, main content block and messages block
      # @see \Drupal\Core\Block\MainContentBlockPluginInterface
      # @see \Drupal\Core\Block\TitleBlockPluginInterface
      # @see \Drupal\Core\Block\MessagesBlockPluginInterface
      # @todo Blocked on Blocks-as-XB-Components: https://www.drupal.org/project/experience_builder/issues/3475584
      type: experience_builder.component_tree
      label: 'The component tree that represents the default page'

How would you imagine that to work in the new world proposed by this issue? 🤔 AFAICT, that'd remain unchanged, because it completely ignores the "default/active theme"'s regions.

(Sadly, I can't find any "global page template" details: it's based on discussing this with @laurii IIRC, and is about the "global template" vs the "page template" as captured by https://docs.google.com/spreadsheets/d/1OpETAzprh6DWjpTsZG55LWgldWV_D8jNe9AM73jNaZo/edit?gid=1721130122#gid=1721130122&range=I29">requirements 18 vs 19.)

+1 if we're on the same page

If y'all agree that splitting PageTemplate into N PageRegion config entities does still leave the GlobalPageTemplate config entity unchanged … then I'm +1 to this proposal, and I do think that it'll make things simpler 😊

Bonus! 🚀

In fact, a nice consequence will be that if you try to uninstall a module that provides some SDC or Block, you'd get a more precise overview in the "oops, can't do that, because these config entities depend on it" UI: it'd tell you exactly which regions it's being used in, and in the future even which variants! 🥳

Back to for a new MR that'll tackle the client-side bits!

While it'd be great to surface this in the UI, the updated expectations in XbConfigEntityHttpApiTest will be super valuable while working on ✨ HTTP API for code component config entities Active !

So: let's first land the back-end parts of this issue. 😄 (We can totally do >1 MR/issue.)

Forgot to say: I ended up merging this without having updated docs/config-management.md, because:

• I wanted to unblock those 2 issues listed in #44 ASAP
• there's 📌 [PP-5] Documentation for code components Postponed to ensure we don't forget 😊

Increasing the priority for following issues:

You didn't actually — you only added those new issues. Made it so!

✨ Config entity for storing code components Active is in! 🥳

✨ Config entity for storing code components Active is in! 🥳

Note: as described in the issue summary of #3499927 only if JavaScriptComponent::status() === TRUE should it get a Component config entity generated.

This unblocked:

1. ✨ Create a ComponentSource plugin for JS components Active
2. ✨ HTTP API for code component config entities Active

🚀

Unlike at #3500997-13: Move SDC-specific validation in ValidComponentTreeConstraintValidator and ComponentTreeMeetsRequirementsConstraintValidator into the SDC source plugin → , I was … not pleasantly surprised what I found here 😅

I found a few severe oversights that would most certainly have slowed down the front-end team when they interact with this:

• using integer, number or boolean types would've caused a hard failure: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...
• "required props" don't work: https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

Fixed all that :)

Doing a thorough review; will try to self-address as much as possible to unblock next steps ASAP.

You're right, I misread & misunderstood!

Per @longwave: I misread & misunderstood! Undoing #6.

Per #3501600-4: Consider refactoring page_template into page_region(s) → , if we go that route, AFAICT this'd become obsolete.

🤯 I didn't see this coming for sure. #2 is accurate — that's how @lauriii always described it.

I definitely agree with the cons, especially the variants one. Although @lauriii makes a strong observation in #3.

I'll think about this some more. I urgently need to review some things, leaving not enough time to think through all consequences.

AFAICT this would make 📌 [PP-2] Don't store page template model data in auto-save for an entity Postponed obsolete too, and hence we could add it to the "avoid" list?

99% of docs updates have now been done. Down to renaming, mostly :)

#59.2 — pass 3 of 3.

Still outstanding:

1. Reconciling the should-haves for 6. Save (draft) content with those of 5. Place blocks as components — both are impacted by/need changes to the data model. I still need to dig deeper to be able to articulate which falls in which bucket.
2. 23. Component creation via code editor: actual implementation work has only truly begun last week. See the plan at 🌱 [Meta] Plan for code components Active , which @balintbrews crafted. Pinged him to ask about must-have/should-have/nice-to-have breakdown, because that meta doesn't currently specify.
3. 2.1. Content editing of meta fields needs clarification from @effulgentsia on whether it's essentially done by virtue of having the Page content entity type plus a working 6. Save (draft) content. (I was on paternity leave and working limited capacity, I was in none of the meetings.)

At this time, there's nothing more I can do. Hopefully updates on the remaining pieces in the next 24 hours.

Yay! :D

And the remaining failures should be largely trivial to resolve, thanks to the detailed validation + test coverage we have:

--- Expected
+++ Actual
@@ @@
 Array &0 [
-    'component_tree' => 'The 'Drupal\Core\Block\TitleBlockPluginInterface' component interface must be absent.',
-    'component_tree.props.block-invalid.' => Array &1 [
-        0 => ''label' is a required key.',
-        1 => ''label_display' is a required key.',
+    'component_tree' => Array &1 [
+        0 => 'The 'Drupal\Core\Block\TitleBlockPluginInterface' component interface must be absent.',
+        1 => 'The array must contain an "inputs" key.',
     ],
 ]

👆 Failures like that make it easy :)

No commits/comments for a week because @f.mazeikis has been working on something more urgent. Unassigning to reflect current status.

#59.2 — pass 2 of N.

A risk with removing this from the XB-wide storage of explicit inputs (currently ComponentPropsValues in the XB field property named 'props', but 📌 Remove references to 'props' outside of SDC - use 'inputs' instead Active will change that), is that we may lose the ability to apply update paths across all stored XB component trees.

It is the ability to consistently detect what field type the stored input for an SDC prop uses that enables us to e.g. automatically upgrade all SDCs that use an image field+widget to instead use an entity_reference restricted to Media + the Media Library Widget. (See media_library_storage_prop_shape_alter() — which now runs always, but we do not want to prevent the ability for different site owners to make different choices for field type + widget. Nor the ability to change their decision later, which requires the ability to write such an update path.)

Idea:

• Add interface ComponentSourceWithCustomizableExplicitInputStorageInterface extends ComponentSourceInterface, similar to how we already have interface ComponentSourceWithSlotsInterface extends ComponentSourceInterface. Only the SDC component source (and the upcoming "code component" component source) would implement this.
• Add ComponentSourceWithCustomizableExplicitInputStorageInterface:: getPropSourceTypePrefixList().

#59.2 — pass 1 of N.

One more follow-up: 📌 [SPIKE] Prove that it's possible to apply block settings update paths to stored XB component trees Active .

wim leers → created an issue. See original summary → .

📌 Move SDC specific validation in ValidComponentTreeConstraintValidator::validate into the SDC source plugin Active is in.

@longwave surfaced this over at 📌 Move SDC specific validation in ValidComponentTreeConstraintValidator::validate into the SDC source plugin Active .

@longwave posted 2 remarks minutes after merging — thankfully he opened 📌 Decouple component UUID from clientModelToInput and validateComponentInput Active to address those excellent observations! 😊

Nice catch!

Discussing this with @effulgentsia and @lauriii. To enable @lauriii to specify which polish he considers necessary for meeting the product requirements, I need to:

1. clearly distinguish between must-have (all the "foundations" bits), should-have (the "polish for long-term API stability" aka avoid technical debt) and nice-to-have ("polish for later phase")
2. I should first order the should-have lists by what I perceive to be priority order; this enables @lauriii to either confirm or ask for changes

This update to the issue summary only does the first bit.

📌 Move SDC specific validation in ValidComponentTreeConstraintValidator::validate into the SDC source plugin Active is in, this is the logical next step now.

(Merged given that @larowlan independently pushed this forward in the exact same direction I was going. No need to delay this crucial data integrity/validation/clean-up issue!)