Vienna
Account created on 21 January 2005, over 20 years ago
#

Merge Requests

More

Recent comments

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

thank you for the review! Merged and published change record.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

Also noted this. I'd rather classify this as critical, because unsupported software does not get any security coverage either.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

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!

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

I do not see your comment, did you miss submitting it?

Anyway, thank you for the review! Merging then!

🇦🇹Austria fago Vienna

lupus_decoupled_ce_api --> This is lupus_decoupled, not lupus_ce_renderer!

🇦🇹Austria fago Vienna

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!

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

fago made their first commit to this issue’s fork.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

schema_metatag support is already added and works automatically once installed via lupus-decoupled, see 📌 Add support for schema-metatag module Fixed .

🇦🇹Austria fago Vienna

Lupus Decoupled now has views support, thus I think there is a good solution to this problem now!

🇦🇹Austria fago Vienna

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

🇦🇹Austria fago Vienna

thx. I addressed your remarks, either fixed or commented - see comments above!

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

Added MR including test coverage. I also verified manually that the change fixes the described issue.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

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.

🇦🇹Austria fago Vienna

It needs something like

    <div v-if="sections" class="layout">
      <component :is="useDrupalCe().renderCustomElements(sections)" />
    </div>

added.

🇦🇹Austria fago Vienna

ok I added a basic structure which adds a sub-layout for each drupal-cms recipe, so things are organized per recipe. Also this would potentially allow for cherry-picking individual sub-layers of certain recipes if people prefer this vs simply adding all.

Generally, simply adding all layers shouldn't be an issue as long as we only have some components in there, since afaik there are no un-used nuxt components in a code base. They are only added to chunks or lazy-loaded when actually used. Still, when problematic cherry-picking sub-layers can be done.

See: https://github.com/drunomics/nuxt-drupal-cms-layer/blob/1.x/README.md
It also has usage-instructions.

What's missing now is improving the github codespaces setup to add this extends-configuration when we want to launch with drupal-cms. For that we have the child-issue 📌 Use nuxt-layers for easy frontend component distribution Active

Once we have established how we ship vue-components here, I think we can mark this done + make individual sub-issues for adding concrete vue components per recipe.

🇦🇹Austria fago Vienna

as commented in the parent issue, the approach seems to work good for https://github.com/drunomics/nuxt-drupal-cms-layer
-> what's left here is the codespaces setup

🇦🇹Austria fago Vienna

Thank you! Checked the d.o. project and its content, all good!

🇦🇹Austria fago Vienna

Thank you! Checked the d.o. project and its content, all good!

🇦🇹Austria fago Vienna

Thank you! Checked the project and the content, all good!

🇦🇹Austria fago Vienna

Thank you! Checked the project and the content, all good!

🇦🇹Austria fago Vienna

The cookie setup section needs work, but also we miss a follow-up for re-adding review-setup instructions.

🇦🇹Austria fago Vienna

This is shutdown now. We succesfully migrated to Github codespaces, but left gitpod setup in place. Thus, marking as done.

🇦🇹Austria fago Vienna

I've created a new layer for drupal-cms and starting to test it via:

> npm create nuxt@latest -- -t layer

> How do you propose I can help here?
How is your layer coming along? Do you have some usage instructions, on how it can be installed on top of e.g. the naked or some other setup? Seeing necessary steps there would help me.

I'm wondering whether we want to locally clone the layer into ~/layers/ directory and have it registered that way, or whether we want to use the extends + github:repo.. syntax. I guess for working with it + discoverability the manual clone would be a bit better. Any preferences / opinions on that?

🇦🇹Austria fago Vienna

thx, merged then!

There is a change-notice which documents the change. Not sure where we would it else?
I guess we should update the documentation about views at lupus-decopled.org!

🇦🇹Austria fago Vienna

ok, I finally tracked the problem down due to how cache-contexts are calculated.

Lupus CE renderer correctly adds this cache contexts:
'url.query_args:_content_format',
'url.query_args:_select',

But the problem is the _content_format is actually not changed via the query parameter, but lupus_decoupled_ce_api actually changes it via the request attribute. That means, lupus_ce_renderer should expose this is a new cache context which works like getContentFormatFromRequest().,
> $default_content_format = $request->attributes->get('lupus_ce_renderer.content_format', $content_format_settings);

🇦🇹Austria fago Vienna

Well, this "plugin type" is not an existing concept, this is just how I called it. The plugin IDs are custom_elements_page and custom_elements_block respective. But since with custom elements for a drupal-view it's pretty clear you are using one of those, repeating the custom_elements prefix seems stupid and makes just long ugly names. That's why I'd think we better remove it.

But given that, yeah, there is no better api to do that.

🇦🇹Austria fago Vienna

Discussed with usernamee that we do another final test round after merge, so moving on! Thanks for the review!

🇦🇹Austria fago Vienna

During prototyping I identified the following challenges:
* CORS issues. When loading static JS from another domain, we need to make Nuxt provide suiting CORS headers
* Browser JS code cannot use helpers exposed by JS modules like Vue's h(). We could expose needed helpers manually from Nuxt

🇦🇹Austria fago Vienna

I had the opportunity to dive into this a bit at DrupalDevDays. XB already does a great job rendering into components, so we should be able to re-use that and build upon it perfectly fine. There is the "hydated-component-tree" which we could use, see ComponentTreeHydrated. Or, we simply use the generated render-array consisting of nested components and blocks, and process into the tree of custom elements. (Similar to what we do with layout builder). Code-components are rendered as astro-islands, so could be output as astro-island component also.

I clarified with balint brews that we do not need to handle auto-saving storage or something, preview is going to work differently anyway. Then we should be able to do all this as simple CE-field formatter.

Created 📌 First-class experience builder support Active for the overall plan.

🇦🇹Austria fago Vienna

implemented the proposed solution -> added MR and a draft change notice

🇦🇹Austria fago Vienna

created MR, draft change notice and adapted test. Ready for review.

🇦🇹Austria fago Vienna

Added MR. I made the attribute be an empty string instead of NULL, so it's explicit. a NULL attribute would end unset and a possible default value be applied, so with the empty string it should be more clear.

Also extended test-coverage and refactored the test case a bit.

🇦🇹Austria fago Vienna

Not sure we can easily hide the option, I don't think we have the control over the block configuration form without lots of kungfu. Maybe the most pragmatic way would be to simply only send the "title" attribute if the display option is checked.

🇦🇹Austria fago Vienna

> When no view title is set (which is the default) an empty title attribute is rendered. What about using the view's name as fallback?

hm, interesting. I'd leave it up to the frontend to make this fallback if desirable. When we set the view name it's hard for the frontend to differentiate from a real title, so it should be done there imo. When no view title is given/set, an attribute seems ok / honest to me.

> All view formats other than Custom Elements don't show any results. Should we limit the options?
Sounds good, but I don't know whether this is feasible. I'd suggest this is a follow-up improvement + applies to pages also.

> The Display Title option in the block configuration does not have any effect
Good point. So maybe we should hide that option? Imo, we could do this as a follow-up improvement.

🇦🇹Austria fago Vienna

ok, I've added the test coverage also. I shortly tried doing a kernel test, but somehow the setup made troubles, so I extended the pre-existing functional test for the page instead. Next, I added follow-ups for the related issues and improvements I found and mentioned before. So this should be ready now.

🇦🇹Austria fago Vienna

This is at least major. This has the potential to confuse people + might break actual sites.

🇦🇹Austria fago Vienna

I can confirm this is fixed now. With the fix in core, this works, so let's mark this as fixed also!

🇦🇹Austria fago Vienna

ok, some more updates:
* Fixed block title overrides
* Fixed php8.1 compatibility
* Add config schema

Now, we just need to add test. Then this should be complete.

🇦🇹Austria fago Vienna

ok, I got things working now. I refactored the code such that both display plugins, page and block, use the regular views control view for rendering. Before we have been bypassing it, instead of implementing render(). That's fixed now.

One thing that does not yet work, is the overridden title.
Then, we probably want to improve the custom-element tag to append the display id. But since this is a change for the page also, I think this should be its own change notice.

Also, this still needs a test. Meanwhile, this could be already reviewed.

🇦🇹Austria fago Vienna

With site templates coming up, decoupled site templates will be a thing, at least we and another company I know would like to create them. While possible with composer, it would be a pity if we would have to host the frontend outside of drupal.org. But if this does not get resolved, we are not left with another choice.

Likewise, with design systems coming up, it will be a topic to create the with MIT such that they are portable to React, Vue, ... If this does not get resolved, they would have to move elsewhere also.

🇦🇹Austria fago Vienna

Thank you for the pointer! I've commented on the other issue.

For keeping this issue up2date, I'm also crossposting here. I think 🌱 Unify & simplify render & theme system: component-based rendering (enables pattern library, style guides, interface previews, client-side re-rendering) Active would be the perfect first step for going towards a multi-frontend architecture. As I commented at #2702061-114: Unify & simplify render & theme system: component-based rendering (enables pattern library, style guides, interface previews, client-side re-rendering) we need to have a 2nd-step through to entangle the control flow such that we are able to generate a complete tree of components first and render it in a 2nd step. Once we have that we can also serve it via an HTTP API for a) multi-frontend usage and b) better debugging XP.

🇦🇹Austria fago Vienna

Coming over from 📌 Make Drupal core multi-frontend Active
@catch - Thank you for pointing me to this issue.

Moving it all over to components and solving challenges like getting rid of pre-processors would be an amazing and a big first step for moving multi-frontend. However, there is another related level of complexity in Drupal theming: The back and forth between Twig and Drupal rendering.

It's been a while since, a tried a twig-powered component approach with Drupal. pdureau, could you share you experience on this, I assume this is stilll how it works, i.e. when you build up a page with components? Like a node component rendering fields, and the fields "content" triggering the rendering of a field renderable again, would again render nodes and... - you get the idea. In my experience, that back and forth leads to very complex control flow, that becomes hard to debug when needed.

Instead, I'd love to have a a tree of components generated first, then in a second step have it rendered. When required for performance, it could have some explicit support for lazy-rendering/generating a sub-tree of components, but I think that should be explicit and used only rarely. If we could generate that kind of tree, it would be very hand to debug/see what the frontend is going to output by e.g. exposing that tree via API (optionally expanded). This would aid theming DX quite a bit imo.

Second, a rendered component tree served via API comes with the additional benefit of becoming renderable by different frontends. E.g., this is all what Lupus Decoupled does, make Drupal serve a tree of components via API, so a Vue/Nuxt frontend can pick it up.

🇦🇹Austria fago Vienna

IT's merged, but the resulting description is now a weird mix between gitpod and github!? see https://github.com/drunomics/lupus-decoupled-project?tab=readme-ov-file#...

Development in Cloud
GitHub Codespaces
Gitpod testing options

🇦🇹Austria fago Vienna

The code looks mostly good now!

However, as discussed with roderik, the remaining todo is that we need to unwrap the resulting custom-element. It seems the views-block integration results into some render-array wrapper, what we need to get rid off, such that the API contract is on the block-level holds.

🇦🇹Austria fago Vienna

I think we are pretty much done here, but there is one issue which confused me: Test location. It seems we have some tests under the main module, while others are split in the moduels we are testing. Where should tests go? I think we shall decide on a rule and then make a child-task to make sure it's enforced. Thoughts on the right rule?

Production build 0.71.5 2024