Overview
⚠️ The client-side/UI data model was introduced very early on and is very simple: LayoutNode and friends were introduced on May 9, 2024, during DrupalCon Portland, before we even were using the d.o issue queue! It's simple and good enough for #3454094: Milestone 0.1.0: Experience Builder Demo. But it's too simple for #3455753: Milestone 0.2.0: Early preview.
It currently looks like this:
export interface LayoutNode {
name?: string;
uuid: UUID;
nodeType: 'slot' | 'component' | 'root';
type?: string;
children: LayoutNode[];
props?: {} | undefined;
}
The client-side/UI data model must evolve because:
When trying to place theshoe_tab_panelSDC-as-an-XB component that #3446722: Introduce an example set of representative SDC components; transition from "component list" to "component tree" introduced, you'll notice that thenameSDC prop cannot be used as normal. This was documented long ago and left for the future:
// @todo the current quick-and-dirty UI PoC unfortunately prevents any prop from being named `name`, because it expects that to convey the component name — but it's not actually one of the props consumed by the SDC.→ handled in #3487773: Get component name from components list not from the component's model
- Designs for
7.1 Tokensupport are coming, and implementing them will bring the ability to the UI to useDynamicPropSources in addition toStaticPropSources, to use data stored in base/bundle fields of the host entity ⇒ more client data model changes needed- The work on #3475672: Research: Possible backend implementations of auto-save, drafts, and publishing might result in yet more client data model changes
- The docs for the Redux integration should help inform a more complete client data model #3446434: Document "Semi-Coupled Theme Engine" and "Redux-integrated field widgets" components
- Finally, different component types may have different kinds of inputs, for example blocks-as-components will likely need block settings, not
StaticPropSources: #3484666: Show configuration forms for Block components- If product requirement
13. Undo and Redomust also apply to bundle/base fields on the host entity (i.e. Field Widgets in the "regular entity form"), that would also impact the UI data model.Product lead @lauriii should clarify that 😄🙏Confirmed by @lauriii in #14.- #3467870: Support `{type: array, …}` prop shapes might also impact this.
- Designs for
Proposed resolution
- #3499540: Remove dynamic prop sources from test data
- #3499554: Move client-side assumptions about prop form data shape into a series of prop-specific transforms
- #3494684: Add ::clientModelToInput() to ComponentSourceInterface, the inverse of ::inputToClientModel()
- #3493941: Maintain a per-component set of prop expressions/sources
- #3493943: SDC+JS Component Sources: split default values into `resolved` and `source`
- #3499550: Support server-side massaging and validating of ComponentInputsForm values
- #3504421: Rename `PropSourceComponent.field_data` to `.propSources`
- #3500152: Remove InputBehaviorsBlockSettingsForm and consolidate with input components form
- #3487284: [META] Add explicit end-to-end test coverage for using all core field widgets including multiple cardinalities in the "Page Data" (content entity) form
User interface changes
None.
Issue fork experience_builder-3467954
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
gauravvvv commentedComment #3
gauravvvv commentedComment #4
kristen polAdded link to code in summary.
Comment #5
kristen polGood to know this... we have components that use "name" for props so tracking this:
#3470573: Track issue that XB doesn't allow props to be called "name"
Comment #7
wim leers@fazilitehreem just reported the same bug: #3473525: Adding shoe icon and shoe tab panel crashes the site, and I had to re-investigate this: #3473525-3: Adding shoe icon and shoe tab panel crashes the site 😬
Crediting him.
Comment #8
wim leersComment #9
wim leersComment #10
wim leers@lauriii, point 6 in the issue summary's needs a clarification/response from you. 🙏
Comment #11
wim leersComment #12
wim leers#3469610: Prepare for multiple component types: prefix Component config entity IDs with `sdc` just landed. Component config entity IDs are now present in the client-side data model, instead of SDC plugin IDs. Next up: #3475584: Add support for Blocks as Components. So one small step in the right direction has already been made! :)
Comment #13
wim leersComment #14
lauriiiThe undo/redo should apply consistently to any content and design changes in XB. This means that for consistency, the undo and redo should also apply to changes in the values of base fields and bundle fields. 👍
Comment #15
wim leersThanks for confirming. That massively complicates things obviously. That means the undo/redo history must track not only the XB component tree (its
treestructure +propsvalues), but also the host entity's values — both in server-side representations (to be able to updatepropsvalues usingDynamicPropSources) and client-side representations (to be able to update the corresponding bundle/base field widgets — presumably without triggering AJAX updates to ensure instantaneous reactivity of the UI when undoing/redoing).That's quite a big challenge. Because field widgets today do not support that kind of undo/redo either… 🙈
Comment #16
wim leersComment #17
wim leersOops, fix bad copy/paste 😅
Comment #18
wim leersComment #25
wim leers@tedbow, @effulgentsia, @longwave, @bnjmnm, @jessebaker, @traviscarden and I just met and discussed this issue.
Notes: https://docs.google.com/document/d/1sMpbWxnOZM-yq4IbrabUBh7-RGYj_2-aYkBR...
We have a rough outline for what we think could/should work.
Next steps:
Comment #26
wim leers@jessebaker started #3487773: Get component name from components list not from the component's model, which addresses the first point in the issue summary 👍
It's been RTBC since Thursday, but needs final sign-off from @jessebaker. He's back from PTO tomorrow. Postponing on that.
Comment #27
wim leers#3487773: Get component name from components list not from the component's model landed.
I think the most important next step is #3484666: Show configuration forms for Block components. I'll try to push this issue forward tomorrow, but ideally it would not be me 😇
Comment #28
wim leersComment #29
larowlanWe'll also probably need a version number or hash to prevent clobbering of other user's edits in multi-user env.
Comment #30
larowlanPlanning to pick this up next week
Comment #31
wim leersWhen you do pick this up next week: see #3489899: Add support for global regions, where @longwave, @lauriii and @jessebaker articulated already one significant change to support
PageTemplate: we'll get rid of the special "root" in the client-side data model, in favor of multiple named component trees, with thecontentregion's component tree representing the component tree of the currently edited content entity's XB component tree, if the XB UI is currently looking at a route pointing to a content entity.Comment #32
larowlan.
Comment #33
larowlan.
Comment #34
larowlan.
Comment #35
wim leersHow could they ever? Content type templates would be independent of
PageTemplateconfig entities? 🤔The challenge I see with content type templates is that they're a single component tree (i.e. comparable to a single region's component tree in a
PageTemplate), but with the need to have arbitrary subtrees locked. It sounds like you're thinking of naming the editable subtrees? 🤔 But that depends on what the product/UX decisions would be for #3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model.Comment #36
larowlanI added
based on comms in 2 in https://www.drupal.org/project/experience_builder/issues/3489899#mr429-n... but it sounds like I've represented that wrong
Comment #37
larowlanmy current thinking on how to solve this involves the following:
I think this gets us:
I'll start building this out tomorrow
Comment #38
bnjmnmRe #37 👏 I'd been hoping we could get to a point where the form could just POST what it has vs a bunch of client side manipulation and this seems like a feasible way to do it.
Some things that come to mind that may not need to be considered for this initial implementation (or at all 🤷) but just to get them out there:
onQueryStarted+updateQueryDate, but having not used them yet I'm not sure what specifically is being addressed through their use instead of a plain old store update. I have my hunches, but if you can share details I don't have to make a wrong assumption in publicComment #39
larowlanGreat idea, we can also return any invalid entries, but it would require moving the form state context into the global redux slice. I don't think that's an issue and is probably desirable in the long run
Yes I think this will likely end up being wiring on the backend to the source plugins with any luck
I'm not afraid of being wrong in public - I do it all the time. I think its important for newcomers to see experienced campaigners like us get it wrong. It helps with imposter syndrome. To answer your question I _think_ we can avoid an extra request to the backend when we update the model by making use of this feature. We will need to keep the postPreview call in useEffect for other operations that modify the tree but don't need to post to Drupal (new component insert, re-ordering, duplication, delete). This means when the proposed POST to do validation/massaging returns a new model and a new preview, we don't want that update to the model to trigger another postPreview call. I think there' should be a combination of those APIs we can call to prevent that needing an extra round trip if we already have the data. But I could also be wrong, once I get into it I'll know.
One thing this does do is move us further from real time updates because now we're waiting for a round-trip to Drupal before we update the model but really, we weren't updating the preview without the round trip anyway. I think to get to that we'd need to implement the transformers mentioned in the google doc and try to convert as many of the massageFormValues in the BE to transformers. We could retain the round trip when we don't have info on transformers (e.g. for widget classes we don't know about in contrib). But all of that is a long way off because we don't have real time preview updates yet anyway
Comment #40
larowlanPushed a WIP to branch
3467954-server-side-model-massagewill continue tomorrowComment #41
larowlanMade some good progress here, server side model validation and massaging is happening in a single request that also updates the preview
Work to date https://git.drupalcode.org/issue/experience_builder-3467954/-/compare/0....
Working on removing the media workarounds at present
Comment #42
larowlanComment #44
larowlan#3493940: [PP-1] Set form state values in bulk rather than in inputBehaviours and #3492511: Move form state into the global store are blocking this
Comment #45
larowlanAdded child #3493941: Maintain a per-component set of prop expressions/sources which is a discrete next step with manageable scope
Comment #46
larowlanAdded #3493943: SDC+JS Component Sources: split default values into `resolved` and `source` as another discrete step
Comment #47
larowlan#3492511: Move form state into the global store is in
Comment #48
wim leers#3489899: Add support for global regions is in!
Comment #49
effulgentsia commentedPer #44, is this still blocked on #3493940: [PP-1] Set form state values in bulk rather than in inputBehaviours?
Comment #50
effulgentsia commentedHas the work in this issue turned up specific cases of #3487284: [META] Add explicit end-to-end test coverage for using all core field widgets including multiple cardinalities in the "Page Data" (content entity) form that could be documented on that issue? Also, I'm hoping some of the work here helps elucidate how to then implement those cases as they're discovered.
Comment #51
effulgentsia commented#3491978-7: Implement saving block settings forms raises an interesting question about block settings forms having some characteristics of props forms and some characteristics of page data forms, and I'm wondering if the work done in this issue helps make that clean.
Comment #52
larowlanUpdated issue summary with the 5 issues I think we need to resolve here to mark this as done.
I've put them in order I think they should be tackled
Comment #53
larowlanClosing MR 448 as that has moved to https://git.drupalcode.org/project/experience_builder/-/merge_requests/536 and #3499550: Support server-side massaging and validating of ComponentInputsForm values
Comment #55
wim leers#3499540: Remove dynamic prop sources from test data is in, as is #3491978: Implement saving block settings forms.
AFAICT #3493941: Maintain a per-component set of prop expressions/sources is the next big one, but #3500997: Move SDC-specific validation in ValidComponentTreeConstraintValidator and ComponentTreeMeetsRequirementsConstraintValidator into the SDC source plugin and #3500994: Remove references to 'props' outside of SDC - use 'inputs' instead would also help get us to a better place.
Comment #56
wim leersAll in:
@longwave has already started pushing #3493941: Maintain a per-component set of prop expressions/sources forward again.
Let's get this done 🤝
Comment #57
wim leersIMHO #3494684: Add ::clientModelToInput() to ComponentSourceInterface, the inverse of ::inputToClientModel() should happen before #3493941: Maintain a per-component set of prop expressions/sources. See https://git.drupalcode.org/project/experience_builder/-/merge_requests/5... for why.
Comment #58
wim leers#3493941: Maintain a per-component set of prop expressions/sources just landed.
It feels like #3500152: Remove InputBehaviorsBlockSettingsForm and consolidate with input components form should be considered part of this meta by the way.
Comment #59
wim leersAdded (the trivial) #3504421: Rename `PropSourceComponent.field_data` to `.propSources`.
Comment #60
larowlanAgree re #3500152: Remove InputBehaviorsBlockSettingsForm and consolidate with input components form - added it
Comment #61
wim leers#3500152: Remove InputBehaviorsBlockSettingsForm and consolidate with input components form landed yesterday!
#3499550: Support server-side massaging and validating of ComponentInputsForm values is in review, with currently one known FE and one known BE regression. I'm confident we'll be able to fix those.
Comment #62
wim leersComment #63
larowlanComment #64
larowlanAdded child meta #3487284: [META] Add explicit end-to-end test coverage for using all core field widgets including multiple cardinalities in the "Page Data" (content entity) form
Comment #65
wim leers#3493943: SDC+JS Component Sources: split default values into `resolved` and `source` is in!
Unless you oppose, I'd like to mark this meta as fixed — #3504421: Rename `PropSourceComponent.field_data` to `.propSources` is a clean-up and #3487284: [META] Add explicit end-to-end test coverage for using all core field widgets including multiple cardinalities in the "Page Data" (content entity) form is a meta of its own, with a different focus.
Comment #66
larowlanWorks for me
Comment #68
wim leersThanks!