Overview
- It should be trivial to identify a component type based only on the component ID (i.e. the
Componentconfig entity ID). - It should be possible to have component type-specific settings.
Proposed resolution
- This issue should make this documentation change a reality:
diff --git a/docs/data-model.md b/docs/data-model.md index 77eca2bd..72f94183 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -243,22 +243,22 @@ Example: ```json { "ROOT_UUID": [ - {"uuid": "uuid-root-1", "component": "provider+two-col"}, - {"uuid": "uuid-root-2", "component": "provider+marquee"}, - {"uuid": "uuid-root-3", "component": "provider+marquee"} + {"uuid": "uuid-root-1", "component": "sdc+provider+two-col"}, + {"uuid": "uuid-root-2", "component": "sdc+provider+marquee"}, + {"uuid": "uuid-root-3", "component": "sdc+provider+marquee"} ], "uuid-root-1": { "firstColumn": [ - {"uuid": "uuid4-author1", "component": "provider+person-card"}, - {"uuid": "uuid2-submitted", "component": "provider+elegant-date"} + {"uuid": "uuid4-author1", "component": "sdc+provider+person-card"}, + {"uuid": "uuid2-submitted", "component": "sdc+provider+elegant-date"} ], "secondColumn": [ - {"uuid": "uuid5-author2", "component": "provider+person-card"} + {"uuid": "uuid5-author2", "component": "sdc+provider+person-card"} ] }, "uuid-root-2": { "content": [ - {"uuid": "uuid4-author3", "component": "provider+person-card"} + {"uuid": "uuid4-author3", "component": "sdc+provider+person-card"} ] } } - … and to do that, it will have to update
config/schema/experience_builder.schema.yml. It also means that the client-to-server data model transformation at@longwave opted to instead update the client where necessary, which was … virtually nowhere! 🚀 That means #3475584: Add support for Blocks as Components will be slightly easier!::clientLayoutToServerTree()needs to have its logic updated.
User interface changes
Zero changes.
Issue fork experience_builder-3469610
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wim leersComment #3
wim leersLet's hold off on this until #3462241-2: [PP-1] Decorate the SDC plugin manager and allow components defined in code is clarified (not implemented/complete, just direction finalized). Because the way that is going, this would just not be necessary.
Comment #4
wim leersOops … 🙈
I got this wrong, because I got #3469609 wrong — see #3469609-18: Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs.
Fixed now.
Comment #5
wim leers#3469609: Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs landed.
#3475584: Add support for Blocks as Components is moving forward, and @f.mazeikis confirms we will need this — see his screenshot to see for yourself: https://git.drupalcode.org/-/project/143385/uploads/6f6cca9105a427535996... — without that prefix, any module that provides a block plugin and an SDC with the same name would inevitably clash.
For now, this should hardcode
sdc+as the prefix, until #3475584: Add support for Blocks as Components introduces a constant that can be used instead :)Comment #6
wim leersComment #9
longwaveThere is some inconsistency going on, which is the source of the UI breaking.
In ComponentTreeHydrated::getValue() we do
but
$component_idis already a machine name by this point in preview, because that's how it comes from the React UI - so previously this was just a str_replace() and effectively did nothing in that case.So is this a bug, or a missing translation from machine name to ID somewhere? I'm confused about the shape sent by the client and how that gets mutated into the tree and then the render array, and at what points "component ID" means "config entity ID" versus "SDC ID" - overloading the term and swapping back and forth between the two is very difficult to follow.
Comment #10
wim leers#9: yes, the UI's data model is too simplistic right now. That's why we have these in
\Drupal\experience_builder\Controller\ApiPreviewController:class HardcodedPropsComponentTreeItem::clientLayoutAndModelToXbField()::clientLayoutToServerTree()Essentially, we have let the UI/client live in a simpler world than the reality, until such a time where we know in more detail what the client will need to know.
For ~2 months now, #3467954: META: Evolve XB UI's data model to allow non-SDC components' inputs, DynamicPropSource support, etc. has served as the "change the client-side data model to be less simplistic" critical bug. I've just rewritten that massively expanded the in that issue's summary.
This issue should not wait for that one. It likely needs to update the aforementioned methods.
Comment #11
wim leers#9: I think part of the confusion is coming from #3469609: Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs not having done everything correctly: I failed to spot in my review there that
::clientLayoutToServerTree()should've been updated too.Comment #12
wim leersComment #13
wim leersPer #3475584-11: Add support for Blocks as Components, this blocks #3475584.
Comment #14
longwaveTests should be green now.
Comment #15
longwaveStill not used to
+being the separator, it looks odd to me - is there a reason we can't just use.?Comment #16
longwaveOpened #3480193: Replace plus with dot in Component config entity IDs to discuss swapping the separator.
Comment #17
wim leersReviewing! Thanks for opening that issue :)
Comment #18
wim leersLooks great! Only trivial nits, half of which you identified :D
Comment #19
wim leersComment #20
wim leers