- Issue created by @effulgentsia
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
I did think about this when reviewing/providing direction. That's why there's this:
// To avoid a race condition for auto-saved code components, always load // the data that it might start using at any point. $libraries[$library_name . '.draft']['dependencies'][] = 'experience_builder/xbData.v0';The draft asset library is already what SHOULD be getting attached, because that's what would guarantee loading up-to-date dependencies (i.e. drafts of first-party imports!).
I definitely *did* forget to double-check that this actually was the current reality in the existing code component editor — massively multi-tasking as is the required norm lately will do that.
Making that so would hopefully be trivial, otherwise @effulgentsia's alternative is exactly the escape hatch I had in mind 😊
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Will verify whether #2 (and point 2 in the issue summary) is the current reality or not.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Ahhhh — the code editor preview is rendered using a client-side generated iframe src:
<iframe className={styles.iframe} title="XB Code Editor Preview" ref={iframeRef} height="100%" width="100%" srcDoc={iframeSrcDoc} data-xb-iframe="xb-code-editor-preview" // @todo: Remove 'allow-same-origin' in https://www.drupal.org/i/3527515. sandbox="allow-scripts allow-same-origin" />—
ui/src/features/code-editor/Preview.tsxwith the
iframeSrcDoccontaining among others:✁✁✁ <head> <script type="importmap"> {"imports":{"preact":"/modules/contrib/experience_builder/ui/lib/astro-hydration/dist/preact.module.js", ✂✁✁ </style> <link rel="stylesheet" href="/xb/api/v0/auto-saves/css/js_component/live_debug"> <link rel="stylesheet" href="/xb/api/v0/auto-saves/css/js_component/xb_test_code_components_using_drupalsettings"> <style></style> <script id="xb-code-editor-preview-data" type="application/json"> {"compiledJsUrl":"blob:http://core.test/fad28408-15f0-4119-b6ce-e60bb16b9cf4","compiledJsForSlotsUrl":"blob:http://core.test/6fc4d6c9-4fb9-4c30-9380-78035a2ff726","propValues":{},"slotNames":[],"drupalSettings":{"path":{"baseUrl":"/","pathPrefix":"","currentPath":"xb/api/v0/form/content-entity/node/3","currentPathIsAdmin":false,"isFront":false,"currentLanguage":"en","currentQuery":{"_wrapper_format":"xb_template","ajaxPageState":"{\"libraries\":\"eJyFzzEOwyAMRuELVfQqVdK9MvAjWQET2a5Eb5_sHTy_b3lYJ5QhBZ_85V6hTyyHGE-xB_7rysmVxNrUkQqZh6iS480DIWys5hvK1BraznKEaBDLS-eN_BdjVKYdHcXv9wvuwG7N\",\"theme\":\"olivero\",\"theme_token\":null}"}},"pluralDelimiter":"\u0003","suppressDeprecationErrors":true,"ajaxPageState":{"libraries":"ckeditor5/internal.drupal.ckeditor5,✁✁✁ </script> <script type="module" src="/modules/contrib/experience_builder/ui/dist/assets/code-editor-preview.js"></script> </head> ✁✁✁Both of those
/xb/api/v0/auto-saves/css/js_component/URLs are generated client-side.IOW: the code editor does not rely on Drupal's asset library system.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
So the crucial finding of the checking I promised to do in #4 and which I described in #5 is:
IOW: the code editor does not rely on Drupal's asset library system.
Which kinda makes sense given that the pretty recent 🐛 JavaScriptComponent CSS libraries should depend on AssetLibrary libraries Active (June 20) updated it all to properly use the asset library system. I was thinking about this problem from that perspective: ensuring that outside of an XB UI context, the right information is guaranteed to load, using existing Drupal infrastructure. Because before #3529677, we were playing whack-a-mole precisely because we weren't using that infra yet!
But in the case of the code editor, it's reasonable that it wants to remain in full control. Although it does mean that contrib/themes altering asset libraries won't see their effects in the code component editor preview … but then again, that could be considered intentional.
Now working to make @effulgentsia's suggestion happen, hopefully without the permission checking.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Done.
- Test: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- Naïve implementation: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- Better implementation: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- Fix for logic introduced in
📌
Populate data to drupalSettings to enable Dynamic Code Components
Active
to make better implementation work (also fixes loading of
xbData.v0for auto-saved code components): https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
But … I think it probably makes more sense to (mostly) NOT use the "real logic", because this is for editing purposes, which means that
breadcrumbsandpageTitlewould be based on the XB UI route 😅 So just pushed that, too.I'd like @isholgueras to do manual testing and then RTBC, and then @balintbrews to do the final sign off: he should be the one to merge an RTBC MR here.
- 🇪🇸Spain isholgueras
After testing it, it now works well. In `HEAD`
In this branch, it works
The code looks good too.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Changing from to . If you want to know why, read what's below the
<hr>.Actually, this was not something we missed in 📌 Populate data to drupalSettings to enable Dynamic Code Components Active , because that stated:
The data needs to be added everywhere where the astro_island render element (\Drupal\experience_builder\Element\AstroIsland) is rendered, so not only on the XB UI.
That was implemented. But the code component editor preview does not use that infrastructure: it renders it entirely client-side as I discovered in #5.
So: in #3531249, I mistakenly assumed that the FE team would handle the loading of the right settings in the same issue where
getPageData()andgetSiteData()would be implemented (is there an issue for that? I gleaned this from #3531249-18: Populate data in `drupalSettings` to enable simple use cases in dynamic code components → .) - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Great, thanks!
P.S.: I think all the metadata changes were accidental? This is a beta blocker from a feature POV, which is why 📌 Populate data to drupalSettings to enable Dynamic Code Components Active was a beta blocker too. These issues, together with yet-to-be-created front-end issues will allow landing 🌱 Plan how to evolve code component overrides Active 's
block_overrideremoval MR 👍 Restoring. - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Given how much @balintbrews has on his plate, plus @isholgueras' testing, plus @larowlan's RTBC, plus the fact that this is at minimum a big improvement on the status quo, going ahead and merging. Hopefully this means @balintbrews will be pleasantly surprised :)
-
wim leers →
committed e49bbdc3 on 0.x
Issue #3533535 by wim leers, isholgueras, effulgentsia, larowlan: Attach...
-
wim leers →
committed e49bbdc3 on 0.x
Automatically closed - issue fixed for 2 weeks with no activity.
