- Issue created by @larowlan
- Assigned to effulgentsia
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
- I know for sure this was extracted out of https://www.drupal.org/project/jsx β .
- AFAIK @effulgentsia advocated for this to be built.
- I think that in principle what you propose could workβ¦
- β¦ but I don't know all considerations that went into building https://www.drupal.org/project/jsx β the way it was built.
So rather than me theorizing, I'd rather have somebody with more distance to the XB code base, but deep insight into the https://www.drupal.org/project/jsx β codebase, to respond here. That person would be @effulgentsia.
P.S.: the term "hijacking" has a very negative connotation. I don't think that is necessary.
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
I realise it's a strong word, I'm trying to convey that the current approach is not suitable for production sites
I've toned it down
- πΊπΈUnited States effulgentsia
I agree that
experience_builder_system_info_alter()
is too heavy handed in terms of overriding all themes to use the semi_coupled engine, and that this will need refactoring prior to XB being suitable for production sites.we already have a way to serialize things, and that's the serialization module
Do we have any precedent for using the serialization module to serialize render arrays? I think of the serialization module as there to serialize models, but render arrays are views (not in the Views module sense, but in the MVC sense). And serializing render arrays to HTML is exactly what the render and theme systems do.
Conceptually, the
*--xbxb.html.twig
files function exactly like theme hook suggestions. They override the base template and render different HTML than what the base template renders. In this case, that "different HTML" is an HTML custom element.I agree that it's weird that the
*--xbxb.html.twig
files contain JSON instead of Twig. I think there's ways that that can be refactored. For example, the JSON could be moved to a different filename, like*--xbxb.spec.json
, and the semi_coupled engine could be refactored to a Twig function, so that the*--xbxb.html.twig
file could become regular Twig files that just do something like:{{ renderSemiCoupledCustomElment(_variables, 'NAME.spec.json') }}
That would then also remove the need for
experience_builder_system_info_alter()
.Not sure if the above is the best DX though, and there might be better ideas for how to bridge XB's theme hook suggestions to rendering the desired custom element, but I don't see how or why the serialization module belongs in the mix for this. Maybe I'm wrong though and am just underestimating the scope or utility of the serialization module?
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Symfony setializer defines two interfaces, NormalizerInterface and SerializerInterface. The first one is for turning objects into arrays, the second one is for turning arrays into strings. Eg there's an XmlSerializer, a JsonSerializer. We'd just have a XB template setializer for turning render arrays into a string in our bespoke manner
- πΊπΈUnited States effulgentsia
But no other module that defines theme hook suggestions does that. E.g., `field_ui.module` adds `form_element__new_storage_type` to its `hook_theme()`; it doesn't define a new serializer. How much of the theme system would we need to duplicate into an XB template serializer?
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
I think with some refactoring of theme manager (eg adding methods as extension points) , we could subclass it and not need to duplicate much
But it would require a spike to identify those extension pointsI also think storing the link between a react component and a form element plugin should happen via form element plugin definitions. The setializer can read that to work out what should be formatted for hyperscriptify and what should not
This means in the future we can allow modules to opt in their custom elements. The hard coded mapping in the JS could be replaced with a mapping of lazy imports we could share via Drupal settings
This requires changes to vite config for the UI and the use of import maps, but we would need to do that anyway to support UI extension
I can work on a spike on this when I'm back
- π¬π§United Kingdom longwave UK
Just been digging into the semi-coupled theme engine a bit, and I think that if theme engines were tagged services, and could be selected by their supported file extension, instead of using hook-style function names, then you could hopefully just decorate the Twig engine in order to extend it?
- πΊπΈUnited States effulgentsia
By the way, if I understand the desired UX correctly, we're wanting to render the forms in the right-hand sidebar using a set theme, neither the site's front-end theme, nor the admin theme. If this is correct, then even if we did no refactoring to the semicoupled theme engine, we could just use it as it was initially designed for: as the theme engine of the theme that we use for rendering the forms in the right-hand sidebar, so we wouldn't need to hack-alter the engine of themes we don't control.
That said, I'm not opposed to refactorings that decouple it from being a theme engine, so that it could be more cleanly integrated with any theme.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
If this is correct, then even if we did no refactoring to the semicoupled theme engine, we could just use it as it was initially designed for: as the theme engine of the theme that we use for rendering the forms in the right-hand sidebar, so we wouldn't need to hack-alter the engine of themes we don't control.
It's not entirely correct.
@lauriii has indeed indicated that XB needs its own look and feel.
- In Drupal, "look and feel" is defined in Drupal Themes.
- But for XB, the intent is to do that in React, using the semi-coupled theme engine. IOW: just like a Drupal Theme would impose its decisions wrt markup/class names/β¦, in XB those decisions should happen in
*.tsx
files. - To make that possible, the starting point must be reliable β just like it must be for Drupal Themes. "stable starting point" implies no theme interacts with markup before XB's React code. Which is why π Changing default theme can break HTML forms generated by XB API routes: always negotiate the Stark theme Fixed made it so that we always negotiate the "Stark" Drupal Theme, which actually means "No Drupal Theme": it's only the raw render arrays/default templates that Drupal Modules provide.
- πΊπΈUnited States effulgentsia
we always negotiate the "Stark" Drupal Theme, which actually means "No Drupal Theme"
I don't get that. If we always negotiate the Stark theme, why can't we instead create an
xb_stark
theme that's just like Stark (whether a fork or subtheme) but that setssemicoupled
as its theme engine, and always negotiate that theme? That would allows us to removeexperience_builder_system_info_alter()
which is the part that's "taking over the twig theme engine for the whole site" that this issue's title is referring to. - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
π€
Ohhhhhhhh! π€π
TIL that's possible! π€― Quoting
core/modules/system/tests/themes/test_theme_nyan_cat_engine/test_theme_nyan_cat_engine.info.yml
:name: 'Test theme for Nyan Cat engine' type: theme description: 'Theme for testing the theme system with the Nyan Cat theme engine' version: VERSION engine: nyan_cat base theme: false
β¦ built by a certain "lauri" 9 years ago at #2574717: Remove PHPTemplate, and add test coverage for multiple theme engine support β β¦ during DrupalCon Barcelona β¦ 2015! π
I think this should be the next thing for @deepakkm to tackle now that π Lift all methods out of `SdcController` into separate invokable services-as-controllers Needs work is 99% done.
- πΊπΈUnited States effulgentsia
I agree with #14, but that wouldn't fully address @larowlan's original goals in this issue. He may still want to pursue the "custom serialization format" part, which I disagree with, but who knows, maybe he'll come up with an MR that convinces me. Therefore, I think we should open a child issue to do just the part that's in #13 and what's minimally required for that, and leave this issue as potentially still having @larowlan's more ambitious goals.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Fair! Done: π Introduce the `xb_stark` theme that uses the Semi-Coupled theme engine, removes the need for taking over the Twig theme engine for the whole site Active .
Let's wait for #3478287 to land, and then ask @larowlan to re-assess π
Restored original issue summary.
- Issue was unassigned.
- Status changed to Postponed
2 months ago 2:10pm 21 October 2024 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
π Introduce the `xb_stark` theme that uses the Semi-Coupled theme engine, removes the need for taking over the Twig theme engine for the whole site Active is in.
Unpostponing and assigning to @larowlan per @effulgentsia in #17.
- πΊπΈUnited States effulgentsia
With π Introduce the `xb_stark` theme that uses the Semi-Coupled theme engine, removes the need for taking over the Twig theme engine for the whole site Active in, we're no longer "taking over the Twig theme engine for the whole site", so retitling the issue to what I understand @larowlan's proposal to be. However, per #5, I still don't really see why we would want to circumvent the theme system when rendering to HTML, and I think trying to do so would lead to a lot of difficulties when the form has a mix of some elements that have XB suggestion overrides and some that don't interspersed through the tree (e.g., either can be a child of the other).
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Agreed with @effulgentsia. Perhaps this is the more appropriate issue status π