Note: ran into an issue that return docs of getSlot() is off, so fixed it on the way.
created a quick test, with the help of claude code.
This is fixed via 📌 Error when installing module Active now!
It seems 📌 Support for Experience Builder Active is the next step and the way to go. This will integrate with 📌 Improve mapping render-arrays to custom elements Active but does not depend on it - on the contrary, figuring out the conversion strategy for XB-component render trees might help getting the solution for 📌 Improve mapping render-arrays to custom elements Active right. So let's do the conversion for XB-render trees first.
It seems would could do something like the following:
Allow conversion plugins to be registered. Identify the conversion plugin to use by #type or #theme key.
Examples of #type and #theme we'll need to implement
#type component --> logic for XB, see
📌
Support component and astro_island render arrays
Active
and
📌
Support for Experience Builder
Active
#theme custom_element --> straight conversion from custom element object
#theme block --> possibly special handling for blocks
Layouts use varying #theme keys based upon the layout, but have a #layout key. So we might want to support differentiating based upon that also.
Perhaps if we reworked the renderer so that all of the processing aspects were part of 'normalizing' and then the render array to HTML was 'serializing' we could support serializing to JSON and other formats for render arrays.
These sound like the 2 steps you're talking about in #8 @fago
yes! that would perfectly fit and allow other frontends to work with the serialized JSON response to implement alternating render strategies.
this is fixed and shipped with latest nuxtjs-drupal-ce module!
this seems to be fine. here an analysis by claude, which I agree with:
The dependency structure is correct:
- lupus_decoupled_ce_api.info.yml already declares drupal:path_alias as a dependency
- The service lupus_decoupled_ce_api.frontend_path_processor correctly depends on @path_alias.manager
- Transitive dependencies through the module system should make this workThe error might be occurring in specific edge cases or test scenarios where modules aren't being loaded in the correct order. The current dependency declaration
should be sufficient for normal operation
noted this is also fixing this
actually, this is a bug, since in reality d9 is already unsupported and untested, let's mark it as such.
Still, let's release next release as a new minor to better communicate that. I don't think it warrants to create a new major for that fix.
I updated the proposed resolution with a more detailed implementation plan as well as 📌 Prototype rendering JavaScript components with Nuxt Active .
After solving the nuxt-based component rendering I was a bit hesitant we are on the right track here, since the traditional Drupal theme's site-layout is just coming into our way. After planning out the big picture, I think it can work well if we achieve the following:
* Make the full-page preview of XB directly use the frontend via iframe
* Somehow avoid showing Drupal-theme site-layout in XB canvas - make the editor provide a blank canvas.
I forgot to mention: I moved the nuxt code for rendering component previews into its own re-usable package:
https://github.com/drunomics/nuxt-component-preview
So what makes this still hard to use the necessarsy proxy-setup in the backend + generating the right Nuxt-loader JS. I think both can be handled by a backend integration module nicely. We probably want to prototype the proxy code here though and verify it works well.
Also, the module will have to take care to initiate one nuxt instance on the page and use it for rendering all components. Technically having once nuxt instance by component would work, but it's a huge overhead that is likely to kill the browser with plenty of components, so that needs to be optimized.
fago → created an issue.
@fago in #25: AFAICT https://image.nuxt.com/get-started/providers is basically the same as https://nextjs.org/docs/app/api-reference/config/next-config-js/images#e...? Both are basically "generate a URL and let whichever server that points to parse its instructions from the URL".
yes, exactly. I think introducing this concept of pluggable providers/loaders makes sense, since generating all image-size variations is often taken quite some load/compuation-time and many variations on disk of image-heavy sites quickly become huge, would it make sense to add a pugin-type for that? (follow-up material).
Also not sure XB is the right home for this, this seems to be a generally very useful addition.
this should fix it
I also ran into this, with
🐛
Warning logged: Source path aut has to start with a slash.
Active
In this case it could be a module using the API wrong, what triggers the exception.
Here a quick example screenshot!
I suggested to start prototyping like that, because if that works, there's a clear path towards a new component source plugin, which is exactly how we started experimenting with code components. 🙂
Yes, this is just for easy prototyping atm!
@wim leers: Thanks a lot for the pointers and the summary! I'll definitely reach out for implementing the component-source plugin the right way, atm we are not reay for that yet, but I want to get there soon.
This prototype is coming along well, but did not explore matching vue component metadata (props/slots) to required format yet, so we can use that nicely register components. So there is some experimentation needed here before we can move on the component-source part side of things.
> Any chance you could share a screencast of it in action? 😇🙏
It's not fancy yet, but I'll see what I can do!
Adding some more details about the Nuxt/Vue component source plugin.
I have not started working on the component-source part yet, but what Nuxt is going to need is basically a "JavaScript component plugin with externally bundled JS". So this might be something we can develop generally and make usable with multiple JavaScript frameworks with little glue to invoke the components correctly, or shipping a base-class that easy easy to extend with the glue.
added a MR which implements LegacyHooks as documented by https://www.drupal.org/node/3442349 →
What about https://github.com/partITech/php-mistral ?
This seems quite good and has a recent release.
Thank you, merged then!
ok, implemented this.
The MR now:
* Exposes the image formatter with a proper label, so it is easily discovered. I had missed that "file properties" is handling images!
* Works over the existing formatter to improve it.
* It now adds width/height correctly from the image style. Tested and verified to work with focal-scale and normal scaling correctly.
* It makes flattening optional
* It now also implements adding it as slot (even though this does not make sense always, it now works as configured)
* it optionally removes flattening-prefix, since the prefix is not needed when the image is within a media entity. so there can be a nice output easily then
* adds test coverage for everything, co-authored by copilot. Code is not 100% nice all the time, but it tests things correctly!
Thanks! Yeah outputing IDs would be nicer, happy to improve if someone wants to submit a follow-up.
I rather move on with it now, since current MR is already successfully tested.
ok, I now got the prototype up and running without web-security issues.
To test,
- use the nuxtjs-drupal-ce module from https://github.com/drunomics/nuxtjs-drupal-ce/pull/347
- run it via npm run dev -- --host 0.0.0.0
- run XB fork from the branch with nuxt-component SDC
- run it within a nginx/other-server that proxies _nuxt proxy to the frontend (this needs to move to a drupal module)
- change nuxt-compontent.twig entry.js file path as necessary and clear drupal caches
- see it working!
Example proxy config for .ddev/nginx_full/nginx-site.conf:
# Place this BEFORE the Drupal location blocks
location ^~ /_nuxt/ {
proxy_pass http://host.docker.internal:3000/_nuxt/;
proxy_http_version 1.1;
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection 'upgrade';
proxy_set_header Host $host;
proxy_cache_bypass $http_upgrade;
}
location ^~ /__vite_hmr {
proxy_pass http://host.docker.internal:3000/__vite_hmr;
proxy_http_version 1.1;
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection "upgrade";
proxy_set_header Host $host;
}
Thanks for reviewing!
>One idea that I had to have more backward compatibility is to add additional checkbox to add fields to CE display. A config option that is only visible if layout builder is checked and disabled by default.
True. Generally, it seems a bit duplicating since you could simply remove all fields. So considering new sites, the checkbox seems unnecessary duplication and it would be better to not have the checkbox. However, I agree that it would be nice for the upgrade path.
Another idea: Write an update function that removes fields for displays which have layout builder enabled. That way we do not clutter the UI and have a save update. I'll make a follow-up to clarify details!
done. co-authored by github co-pilot with sonnet 3.7 thinking model.
better title
I added the MR. The changed logic is nicely covered by the pre-existing test-case, so there is no need to create a dedicated one for this change. However, I noted that we lack coverage for testing that the generated CE for the layout-builder itself works properly, thus opened 📌 Better test-coverage for layout builder support Active for it. Fortunately, there was no need to touch this part here, so it's not a problem to continue without it atm.
The change implements the configured behavior without introducing a new option. Generally, there is no need for one, since all fields can be removed from the configuration to get to the same result. Because of that, I prefer to not introduce another option to avoid the convolution of UI/config with many options.
That said, this is a change that potentially changes the API output for existing sites, i.e. if someone left the fields in the configuration, they are going to be output now. Thus, I'll create a change record. we are going to tag 3.1 anyway, so it's acceptable to have some larger change in there also.
> I wonder whether we should test for other language.negotiation methods? I think the url.prefixes is the most widely used.
I do think that's not what we want to test here, generally they should be covered else. The test just makes sure that url-prefixes coming from it are correctley obeyed - if any.
Note: This also adds service-utils as a new dependency. I think that's fine, we are going to tag 3.1 anyway. lupus_ce_renderer requires it already.
> hrefLangcode is sometimes needed. If the the URL language detection is in place than hrefLangcode should be used as a prefix before path alias to make the path work.
actually, the output URL should just use the right language already. Then there is no need for the langcode anymore.
I made a setting to allow URLs to be absolute optionally though, that might be needed in some cases.
implemented this and added a kernel test for the formatter. I think this formatter test-case is a good blue-print test-caes for further formatters, should be rather easy to copy and adapt it as needed.
fago → created an issue. See original summary → .
agreed, would be nice to have service-utils provide that helper. but that's another issue.
> There are some new ones in tests/src/Kernel/ThunderParagraphRenderMarkupTest.php
yes, unrelated. I already opened an issue.
Re-checking this, imo it's better to not change this now and mark it as "works as designed".
For one, we figured we had this array of body-text also in the past, so not a new thing. For second, the frontend got fixed to handle it (again) meanwhile, so this is a non-issue now. Also, there is no good BC way to fix the auto-formatted part here.
I see, thanks for clarifying! Yeah, I do think this helps to simplify things in the end, sry for leading into the other direction behavior before.
Recipes also started to include some config, but yeah, I agree some people might be using it like this, so good to have the change-record. Thanks for pointing that out!
re-tested this and it actually seems to work all fine now, probably I did something wrong in my earlier tests. all good!
https://www.drupal.org/node/3527054 → - created! Moving on with this then.
> If you retroactively think your previous specification is "weird", because it makes for worse UX / you do not want to complicate UX to match the spec... I have no firm opinion on changing the spec. But it needs to be done consciously.
indeed. I was aware of this UX problem my previous opinion created. I'm now aware and I do think keeping the v3 simple+sane and is much more important than keeping the 2.x behavior intact here.
> From https://git.drupalcode.org/project/custom_elements/-/commit/c27b1ff37bbe..., I see that you do not want to give your users help upgrading from v2.
I want to keep the README brief and focused. Additional help can be available in tickets or other follow-up material, where people can collaborate and exchange code-snippets.
I was not thinking this needs a change-record, since it will only influence behavior when no config is created, what I'd not expect anyone to do, however yeah, some people will still do, so I agree a change record makes sense. will add one.
implemented
Right, the test was covering the changed logic, so it had to be adapted. So I've dig into the test-logic and adapted it.
I took a look at this. Adding message is weird, it's just a workaround to the underlying problem: The auto-creation of the entity default display during rendering is not the same as the entity you are going to save in the UI.
The difference is, in the UI you are always have to start with the "default" view mode. This is what we need to do during auto-creation also.
By fixing this, I get consistent behaviour and there is no need for a message.
-> https://git.drupalcode.org/project/custom_elements/-/merge_requests/118/...
fago → created an issue. See original summary → .
ok, I created the remaining todo then: https://www.drupal.org/project/lupus_decoupled/issues/3526744 📌 Move views tests into the lupus_decoupled_views tests folders Active
So else, this is done!
I think this is generally there and working.
#4 suggests improvements to make it nicer, what should be its own issue.
I clarified what's left/the goal of the issue here.
Generally, it would be nice to support using CE-field-formatters directly from Views somehow I guess, but that's stuff for another issue I guess.
I don't think it makes sense to point to lupus-decoupled in generall, since lupus decoupled does not cover the prgoressive rendering case. I think that little bit of duplication is acceptable.
ok, I added dedicated tasks for the remaining todos here, so we have a better overview. The main part is done here anyway, thus we can set this to fixed.
The MR is BC-breaking, since we have a stable release now, we need to improve it to keep working the same way as before when "div" is passed.
yes, clearly! RTBC!
Just found stumbled over this (very interesting) issue.
We've been working on solving the same challenge for our projects at drunomics, where we use Vue/Nuxt on the frontend side. The solution we settled is having Drupal generate a high-res image in the right aspect-ratio using editorial controlled cropping, e.g. focal point + using nuxt-image for frontend-controlled, responsive image size generation.
That said, the approach taken by nuxt-image might be worth a look here: It features providers for the image resizing, i.e. it's doing the width/size calcuation based upon the given "sizes" attribute what is convenient for responsive images. Then, by having pluggable providers one can easily swap out a local/drupal-powered resizing resolution with a CDN-provided service.
thank you for the review! Merged and published change record.
draft change notice: https://www.drupal.org/node/3526109 →
This one was a bit tricky to fix. For it to work I had to move it to dynamic content and make dynamic content to be fetched later. This seems all fine, but is a rather-big change, thus I'd rather tag it in 2.5.0 and create change notice to communicate.
However, the only thing it could influence is local-task generation, for which I added a test-case, so this is proven to still work.
Also noted this. I'd rather classify this as critical, because unsupported software does not get any security coverage either.
https://www.drupal.org/project/lupus_ce_renderer/releases/2.4.0 → got released - with the fix! Let's require it here to solve the issue.
Thx! Merged then!
Thank you for the review! Merged then! :-)