- Issue created by @wim leers
- Assigned to gauravvvv
- Issue was unassigned.
- ๐บ๐ธUnited States Kristen Pol Santa Cruz, CA, USA
Added link to code in summary.
- ๐บ๐ธUnited States Kristen Pol Santa Cruz, CA, USA
Good to know this... we have components that use "name" for props so tracking this:
๐ Track issue that XB doesn't allow props to be called "name" Active
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
@fazilitehreem just reported the same bug: ๐ UI reserves the "name" key in its model, preventing any SDC prop from being named "name" โ causes crash for `shoe_icon` and `shoe_tab_panel` SDCs Active , and I had to re-investigate this: #3473525-3: Adding shoe icon and shoe tab panel crashes the site โ ๐ฌ
Crediting him.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
@lauriii, point 6 in the issue summary's needs a clarification/response from you. ๐
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ [later phase] [PP-2] Prepare for multiple component types: prefix Component config entity IDs with `sdc` Postponed just landed. Component config entity IDs are now present in the client-side data model, instead of SDC plugin IDs. Next up: ๐ Add support for Blocks as Components Active . So one small step in the right direction has already been made! :)
- ๐ซ๐ฎFinland lauriii Finland
If product requirement 13. Undo and Redo must 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 ๐๐
The 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. ๐
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Thanks for confirming. That massively complicates things obviously. That means the undo/redo history must track not only the XB component tree (its
tree
structure +props
values), but also the host entity's values โ both in server-side representations (to be able to updateprops
values usingDynamicPropSource
s) 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โฆ ๐
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Oops, fix bad copy/paste ๐
- ๐บ๐ธUnited States bnjmnm Ann Arbor, MI
wim leers โ credited bnjmnm โ .
- ๐บ๐ธUnited States effulgentsia
wim leers โ credited effulgentsia โ .
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
wim leers โ credited tedbow โ .
- ๐บ๐ธUnited States traviscarden
wim leers โ credited traviscarden โ .
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
@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:
- first land ๐ Experiments in rendering Twig as React Active
- then provide concrete examples for each combination in the table that that issue is adding to the docs
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
@jessebaker started Assigned to: jessebaker โจ Get component name from components list not from the component's model Active , 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.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
โจ Get component name from components list not from the component's model Active landed.
I think the most important next step is ๐ Define `props` in the context of Block components Active . I'll try to push this issue forward tomorrow, but ideally it would not be me ๐
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
We'll also probably need a version number or hash to prevent clobbering of other user's edits in multi-user env.
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Planning to pick this up next week
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
When you do pick this up next week: see ๐ Add support for global regions Active , 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 thecontent
region'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. - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
* locked regions so that content-type templates don't impact page templates
How could they ever? Content type templates would be independent of
PageTemplate
config 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 ๐ฑ [later phase] [META] 7. Content type templates โ aka "default layouts" โ affects the tree+props data model Active . - ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
I added
locked regions so that content-type templates don't impact page templates
based on comms in 2 in https://www.drupal.org/project/experience_builder/issues/3489899#mr429-n... ๐ Add support for global regions Active but it sounds like I've represented that wrong
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
my current thinking on how to solve this involves the following:
- stop calling postPreview from a useEffect hook in sendPreviewRequest
- put the previewHtml in the redux store as its own prop, select that in Preview and pass that along as the iframe src
- add a getPreviewHtml to the preview API and add a controller for that, use that for the initial load
- change formStateToStore to instead POST to the preview endpoint, send the current model AND the component ID and the new prop values for that component
- in the preview endpoint do the widget massaging stuff
- return the built previewHTML like preview currently does (top level html key) but also return editedComponentId and newValues (After having run Drupal widget massage etc)
- client side unwrap the response and set the previewHTML in the store using onQueryStarted and updateQueryDate in the POST preview mutation https://redux-toolkit.js.org/rtk-query/api/created-api/api-slice-utils#u.... Also use onQueryStarted in this mutation to dispatch the model change
I think this gets us:
- no additional http request because we're already doing a round trip for the preview when we change values
- drupal/server side is the authority on how to take the form values and manipulate them
- I think we can simplify a fair bit of the logic in things like getPropValues because all that is done Drupal side - in fact I'm pretty sure we can delete that with this in place
I'll start building this out tomorrow
- ๐บ๐ธUnited States bnjmnm Ann Arbor, MI
Re #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:
- Are there scenarios where validation status is based on something more complex than what can be easily enforced clientside (html5/AJV)? Particularly if the criteria can't be determined until the value massaging takes place? If this is the case, perhaps this can be factored into what the Preview Endpoint returns - if the FE is aware of a validation issue it can then output a message etc and it will be clear why the preview is not updating.
- This approach seems useful for other component edit forms such as block settings, which would also presumably POST to the preview endpoint. Although I think everyone participating in this issue is aware, I want to explicitly mention that the props form will work differently from forms such as Block Settings or Entity Forms because although SDC Prop Forms use field widgets, they (unlike those other forms) are not working with fields and have different mappings, validation etc. I believe the data needed to account for those differences is already present in the proposed approach, but JIC.
- I've been wanting an excuse to work with parts of the Redux API like
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 public
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
-
Are there scenarios where validation status is based on something more complex than what can be easily enforced clientside (html5/AJV)? Particularly if the criteria can't be determined until the value massaging takes place? If this is the case, perhaps this can be factored into what the Preview Endpoint returns - if the FE is aware of a validation issue it can then output a message etc and it will be clear why the preview is not updating.
Great 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
-
This approach seems useful for other component edit forms such as block settings, which would also presumably POST to the preview endpoint. Although I think everyone participating in this issue is aware, I want to explicitly mention that the props form will work differently from forms such as Block Settings or Entity Forms because although SDC Prop Forms use field widgets, they (unlike those other forms) are not working with fields and have different mappings, validation etc. I believe the data needed to account for those differences is already present in the proposed approach, but JIC.
Yes I think this will likely end up being wiring on the backend to the source plugins with any luck
-
I've been wanting an excuse to work with parts of the Redux API likeonQueryStarted + 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 public
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
-
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Pushed a WIP to branch
3467954-server-side-model-massage
will continue tomorrow - ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Made 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
- Merge request !448Draft: Resolve #3467954 "Server side model massage" โ (Open) created by larowlan
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
๐ [PP-1] Set form state values in bulk rather than in inputBehaviours Postponed and ๐ Move form state into the global store Active are blocking this
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Added child ๐ Maintain a per-component set of prop expressions/sources Active which is a discrete next step with manageable scope
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Added ๐ Split model values into resolved and raw Active as another discrete step
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
๐ Move form state into the global store Active is in
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ Add support for global regions Active is in!
- ๐บ๐ธUnited States effulgentsia
Per #44, is this still blocked on ๐ [PP-1] Set form state values in bulk rather than in inputBehaviours Postponed ?