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! :-)
Thank you, that looks great now. Output is very nice for single-value fields and decent for multi-value fields + comes with test coverage! merged!
MR is based upon
🐛
Messages sent with redirect responses cannot be server-rendered
Active
since that MR already had a test-case which was simple to extend to to prove this issue.
I figured the problem got introduced when converting to the core messenger service, it's using the API wrong and not deleting messages.
Note: MR shows both changes, let's merge the other issue first. Please review last commit instead meanwhile.
I implemented the feature. Disclosure: The majority of the code is authored by github copilot.
Thanks the the update function there is no BC break for existing sites. New installs will get the better SSR suiting behaviour, thus we should tag the next release as a new minor.
I do not see your comment, did you miss submitting it?
Anyway, thank you for the review! Merging then!
lupus_decoupled_ce_api --> This is lupus_decoupled, not lupus_ce_renderer!
based upon the new kernel test, added https://git.drupalcode.org/project/lupus_ce_renderer/-/merge_requests/47
thanks!
> Overriding settings parameter is not allowed
True. Thanks for spotting. I agree it's not important to test, the whole feature of enabling it via settings is quite obsolete, still supported, but this detail seems to be not important for it either, so I'm fine continuing without testing this.
Thus, merged - thank you!
thank you for the clarifications and better test, this seems pretty good already.
However, as commented above I think we shall still optimize for the 90% use-case to be better and keep that simple in the output, i.e. output only the single data value in the single case and avoid wrapping it in the multi-value data-structure when there are is only one value.
ok, I took a look at this. First off, I made sure we have a good kernel test as base and ported over the existing functional tests to a kernel test. While I were there I also extended coverage for some things that had no coverage before, like redirect handling.
Then I also added the test for this and verified the core user-login-status json API still works with the module.
schema_metatag support is already added and works automatically once installed via lupus-decoupled, see 📌 Add support for schema-metatag module Fixed .
Lupus Decoupled now has views support, thus I think there is a good solution to this problem now!
thank you, that looks pretty good!
I've added one question about the output-data-structure above, could you check that and make sure it's good? Maybe it is, but it seems not.
Besides that, it's great to see test-coverage here. I'd suggest to add a couple of cases though, so we cover the features of our formatter:
- run once without custom-header and once with
- set weights differently and make sure sorting is applied correctly
thx. I addressed your remarks, either fixed or commented - see comments above!
Developed a fix over at https://github.com/drunomics/nuxtjs-drupal-ce/pull/343/files
Ready to test there, but for shipping we still need to release it.
Added MR including test coverage. I also verified manually that the change fixes the described issue.
Thx! I reviewed the PR and added some comments.
I think we should also add a basic (kernel?) test which makes sure that the output is working as expected.
Got a working prototype with teleports. This seems to be the way to go! Now I need to brush it up a bit more.
Need to figure out how to deal with the CORS issue best now. Maybe proxying through assets through the Drupal web-server would be the most convenient.
I figured there is another problem: The nuxt entry JS file always auto-mounts the Nuxt app to a the pre-configured DOM element, by default #__nuxt. This is not what we want size we need a custom component + mount target. Thus, I explored the option to extend Nuxt via module, such that it exposes an additional JS entry point / JS chunk along with the needed APIs. Unfortunately, I did not get this working because Nuxt has some nuxt-import protection that disallows importing nuxt directly in JS code, what breaks the custom-entry.
So how else could it work? We could load each nuxt app in isolation, e.g. via iframes with srcDoc. But given that, I figured having one nuxt-app per preview-components is probably not the best idea resource-wise anyway. It would probably get tot resource hungry when there are ~100+ components on a page. Even worse with so many iframes.
So I came up with a new idea. Use a single nuxt app + Vue teleports: https://vuejs.org/guide/built-ins/teleport As far as I understand, it should be possible to dynamically create teleports - all from one nuxt app - and use it to render dynamically created preview-Components. Will experiment with that next.
Is the issue maybe the strict comparison?
https://www.drupal.org/node/3478662 →
I've learned from recipe sessions that this is mostly a good idea to not use them, since small config changes/extension e.g. by used modules can lead to errors already. Better to only have it strict when really needed for some reason.
It needs something like
<div v-if="sections" class="layout">
<component :is="useDrupalCe().renderCustomElements(sections)" />
</div>
added.