re-titling and raising prio
I cannot reproduce the described behavior. If it really happens, it certainly qualifies as bug, thus fixing the category. Please provide steps to reprodocue, e.g. with gitpod test environments.
Quick review:
* I don't think the module README is the place for drupal-CMS documentation. The README should stay general and document how to use it with Search API. But concrete drupal-CMS instructions should go elsewhere imo (not sure where), or better be not necessary since automated, but that'S a different story/issue.
* Cannot judge about the details, but let's make sure things work via automated tests. Can we also add some search-api & views config to tests that we run as part of automated tests? I think we could simply copy things over from the recipe, but I'd copy to document what is supported on the long-time and not depend on the recipe since the recipe can change over time.
so this is about supporting search api views - let's make this clearer.
I figured multi-frontend and redirect API responses were un-documented still also and quickly add pages for both. I guess we also should add some page below "Nuxt" for our varying starters.
If we could easily support confirmation pages, that would be best. Else I guess we should at least have a working default.
Generally, we are not warning users of not supported stuff, that would be lots of work, would it? Or should we make an exception for that crucial setting and grey-out/disable not supported options? This means, enabling the module takes over all webforms and considers them decoupled. Maybe it would be nice to allow configuring that, but again, complexity, so I think it's ok to keep it simple and make all webforms deocupled once turned on.
> Can it be the site-wide default theme?
That would make regular traditional rendered pages ugly. I'd not do that by default.
> * Possibly auto-enable in default project for forms.
We could do this by adding it to our routes as _theme key - the lupus_Decoupled_forms module has the theme negotiater for switching it built in. If it has a nice fallback to keep the default theme if it's not enabled, we could even set it as default.
Thank you! Additions are good, also all comments have been addressed now. So I think this is ready!
the README already has some good docs to start with: https://github.com/drunomics/nuxtjs-drupal-ce?tab=readme-ov-file#default...
Update on the todos:
* add hosting options
* #3479269: Document default components at website
New todo:
* Add a Drupal introduction page
thanks! please see my comments!
added two new "themes", simply as alternating demo starters. That way we can have multiple gitpod URLs for testing all of them.
Until we have anything better, let's use this.
See https://lupus-decoupled.org/get-started/play-online for a list of frontends, there is one nuxt with shadcn and one nextjs starter.
@glynster: I would love to add https://github.com/StirStudios/stir_nuxt_base there as well. Is it a layer or a full repo now? If a layer, I guess we could just add a small wrapper demo-repo to make it work witih gitpod? I'm opening a dedicated sub-issue for it!
thx! works great and even has solid test coverage! let's merge!
I'd have preferred keeping the code-base tidy when it's unrealistic that anyone is using that yet. Anyway, I'm fine with your proposal to do it proper from the beginning, let's move one with this.
If we don't want to have this message on lupus-decoupled, then we can ship a default CE display for each Core node bundle (and perhaps a CE display for the 'full' view mode too, if we need it). But I guess that's a followup that could wait until we implement proper output for Drupal CMS content types.
I think the message is bad for UX since users opening the screen the first time will have to read it and try to parse/understand what it means for that. Given this, I think for UX we should try hard to avoid this. Also this is not a pattern core introduced anywhere and it'S ususally good to follow core patterns in contrib.
I must say I'm not fully understanding why this message is needed and why it's confusing without. Why is it important whether the configuration you see is acutally saved or not? It should not matter, because either way the same configuration should be applied.
major: if they press the "Save" button without doing anything else, the output will likely change for any view mode that has its own entity view display (because it will be based on the default CE display, from then on).
I cannot follow, could you elaborate what changes? Does core face the same problem?
I still think we need to do what I was writing before:
We'd have to forward the selected langcode. So we must pass the entity language as $langcode when calling this.
Next, the bug might exist in other processors also. When we fix it, we should fix it in all of them.
For 2.x I think it's good enough to fix this with the call-out to the global or even not fix it -> 3.x is the main branch now.
For 3.x we should fix it properly.
i.e. we must extend our API to accept an optional $langcode parameter, so we can pass it on during processing *explicitly*. This is what core does during rendering as well. I guess this means we need to adjust the processors to do so. A new optional $langcode = NULL parameter is no big change, since it keeps BC when the interface was implemented exactly before.
there was indeed a regression for this at some point, but that has been solved. Could you report if the problem is still there with latest releases?
this seems to miss some essential test - the actual login. I think we should add that.
also, this is only about user-forms atm, what is fine. Let's make one issue per test we add, to keep a better overview.
Let's mark this is as META and only set the issues with acutal code for needs-review.
thx, all great! I've improved the README slightly to also link to https://lupus-decoupled.org/advanced-topics/block-layout
thanks! Looks all good, merged.
re-checking on the solution, I think this is the right way and solves the issue. thus, moving on - merged!
unfortunately the gitpod link does not work for lupus-decoupled branches still, see https://www.drupal.org/project/lupus_decoupled/issues/3347260#comment-15... 🐛 Gitpod branch checkouts always use 1.x-dev Active
anyway, I checked the code out manually there and tested it at the gitpod environment also. works as it should!
the DP_ISSUE_FORK variable is missing from our link. thus testing it with a link like this:
https://gitpod.io/new/#DP_PROJECT_NAME=lupus_decoupled,DP_ISSUE_FORK=lupus_decoupled-3485563,DP_ISSUE_BRANCH=3485563-improve-frontend-login,DP_PROJECT_TYPE=project_module,DP_MODULE_VERSION=1.x,DP_PATCH_FILE=,FRONTEND_REPOSITORY=https%3A%2F%2Fgithub.com%2Fdrunomics%2Flupus-decoupled-nuxt3-demo,CUSTOM_ELEMENTS_VERSION=3.*,DP_INSTALL_PROFILE=standard/https://github.com/drunomics/lupus-decoupled-project/tree/main
still, it is ignored, it just takes the 1.x from composer.
I figured it's best to create a new setting for that and implemented this.
I also improved in-code comments for the existing options, such that there is a better overview of what they actually do. During implementing I realized we need to keep paths, not routes. Since in the path processor, the route name is not available.
The MR adds /user to and the user account page to the "keep frontend" paths, that way a user login in the frontend keeps you in the frontend.
Works all good! Thus, I added the patch to the project template, that makes the user logout work.
Given that, this works! The redirection logic needs love, but this can be optimized in a followup. Thus both PRs merged!
added sub-issue 📌 Document ways of creating sitemaps Active
@glynster - wow, that's amazing!!
a few questions and thoughts:
- What dependencies / requirements does it have for testing it? Could it be just run with the gitpod demo link and it mostly would work?
- component style - I wonder why you are moving the types out of the components. Wouldn't it be easier to see what's going on if all is within the component, as in https://github.com/drunomics/lupus-decoupled-nuxt3-demo/blob/main/compon...
- we recently moved the best-practice suggestion to just stay with kebap-case for custom elements, so the 1:1 mapping is easier to see. e.g. see https://github.com/drunomics/lupus-decoupled-nuxt3-demo/blob/main/compon.... Any thoughts/concerns on adopting this?
- sitemap, interesting - we just use simple_sitemap module with a suitingly configured base-url + proxy it through on the frontend. Any thoughts on pros/cons of both approaches? I guess we should open a dedicated issue for it!
- the paragraphs seem great! I guess it would be ideal to pair it with a drupal-recipe which adds the necessary paragraph types. For a theme, that seems a bit out of scope though. So maybe the theme would instead just provide a set of optional components, what in the end, they are. So maybe it's a matter of simply documenting which components are there for optional use and maybe have a way to preview them?
> Adds a Vite hook so Nuxt dev tools work locally with DDEV.
I guess we should add this to the nux3 demo also!
Sounds like a good idea. But I'm not sure what it would entail exactly. As I understand the module adds the data model in the Drupal backend. What kind of integration for next.js/nuxt.js is needed then?
Schema Metatag module is already supported, as long the Schema.org blueprint module is using that, it should just work. Could you elaborate what kind of integation you have in mind, please?
@usernamee: please don't re-open issues that are fixed for small follow-ups, open follow-ups instead. anyway, thanks for the MR, merged!
ended up patching core in 🐛 Menu APIs provide invalid CSRF tokens Active . With that I've seen the link working, so testing it now.
I think this is not important, since generally it's better to make CSRF-token protected links work properly. The confirm-form we could make work, but including the redirect to the form, it's gonna be some additional work that might not be worth it.
It's probably safe to assume that every non-html request is not going to use the render sytem and thus a placeholder should not be created. Not perfect, but a better check than it was before.
This works for rest_menu_items and for core-linkset endpoint once it's correctly specified as json. Patch attached. Will work on a test also.
tested it. it does not - it faces the same problem. Let's open a core issue!
so, re-using the pre-exiting url object does not solve it either. Anyway, Drupal core generates the token with a placeholder, but since we are not rendering obviously placeholders are not replaced. :-( I wonder how this is solved for the drupal core menu linkset api.
to reproduce, run this with drush php
> \Drupal\Core\Url::fromUri('internal:/user/logout')->toString();
= "/user/logout?token=fzL0Ox4jS6qafdt6gzGzjWGb_hsR6kJ8L8E0D4hC5Mo"
compare the token with the right token, it's wrong, it's the placeholder value. So seems this is triggered by a core bug.
However, additionally rest_menu_items has a bug since it calls > \Drupal\Core\Url::fromUri('internal:/user/logout')->toString(TRUE); but throws the resulting bubbleablemetadata away. By throwing it away, the placeholders won't be replaced.
I tracked it down: the problem is the wrong token, is not a token, it's a placeholder, which ought to be replaced by the renderer.
See the logic of RouteProcessorCsrf:
// Adding this to the parameters means it will get merged into the query
// string when the route is compiled.
if (!$bubbleable_metadata) {
$parameters['token'] = $this->csrfToken->get($path);
}
else {
// Generate a placeholder and a render array to replace it.
$placeholder = Crypt::hashBase64($path);
$placeholder_render_array = [
'#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]],
];
// Instead of setting an actual CSRF token as the query string, we set
// the placeholder, which will be replaced at the very last moment. This
// ensures links with CSRF tokens don't break cacheability.
$parameters['token'] = $placeholder;
$bubbleable_metadata->addAttachments(['placeholders' => [$placeholder => $placeholder_render_array]]);
}
\Drupal\Core\Render\MetadataBubblingUrlGenerator::generateFromRoute() seems to activate this logic always, even when the Url is generated with $url->toString(FALSE).
opened 🐛 Wrong user logout CSRF token Active
unfortunately the logout link token has some issues also, see 🐛 Wrong user logout CSRF token Active
ok, worked over it and fixed a couple of things. the token was missing from the link, what I fixed. Still, the login link works now, but the logout link dose not work even when the token is used. It seems the token provided by the menu api is somehow not valid. investigating
works!
it seems we already have a FRONTEND_BRANCH variable, but it was not fully working yet. So fixed it. https://github.com/drunomics/lupus-decoupled-project/pull/13/files
Test-Link
https://gitpod.io/new/#DP_PROJECT_NAME=lupus_decoupled,DP_ISSUE_BRANCH=1.x,DP_PROJECT_TYPE=project_module,DP_MODULE_VERSION=1.x,DP_CORE_VERSION=10.3,DP_PATCH_FILE=,FRONTEND_REPOSITORY=https%3A%2F%2Fgithub.com%2Fdrunomics%2Flupus-decoupled-nuxt3-demo,FRONTEND_BRANCH=feature%2FLDP-2606,CUSTOM_ELEMENTS_VERSION=3.*,DP_INSTALL_PROFILE=standard/https://github.com/drunomics/lupus-decoupled-project/tree/feature/frontend-branch
probably this would be fixed by updating to the latest scripts: https://github.com/drunomics/lupus-decoupled-project/issues/10
this script seems to handle it: https://github.com/shaal/DrupalPod/blob/36b7f27b1603ac3716ba31d99bfc4208...
I read through the docs, all good! merged!
As posted at 🐛 User logout confirmation form is not supported (d10.3+) Active we can workaround the issue with the user logout route by simply showing the content of the "account" menu. Thus, instead of making a request to check the user-login state, we could simply make a request to load the "account" menu and it will give us the right links for login + logout already!
One small glitch: once logged in, it will also include the link to "my account" which redirects to the backend atm. While this is not very nice, I think it's ok and good enough. We can open a follow-up for improving the /user route redirection in the backend.
I think this is not really a bug now, because there are ways to work around it.
I came up with a simple one. Use the account-menu, which at least in standard profile, is there and provides us with working login/logout links. The logout link already contains the right token. api response for testing: /api/menu_items/account
I guess best would be a nuxt-layer, since it would take care of dependencies + registering the components with a low-prio OR providing some-sort of install hooks the copies over components automatically during install. thoughts?
I agree, this would be a great option to have as an option. I guess it would make sense to build it already using the new nuxt-ui alpha version.
However, before we do that, let's clarify how do we build this starter-kits?
Questions I'm wondering about:
* Should it work like a starter-kit, or like a nuxt-module, so you can update later on? I guess starter kits.
* Should it be a nuxt-layer you pull in once? Or just a another repo like the nuxt3 demo which has a couple of components added? The repo seems great, but had the disadvantage that we need to keep it maintained and pull in nuxt updates etc for people to have updated components still..
* Where should we host them? github vs drupal gitlab. I think it would be preferable to keep it on drupal,org, but until
✨
Allow GPL-Compatible Licensing for Non-Derivative Code and Drupal.org General Projects
Active
is fixed, I think we cannot really use drupal gitlab. Thus github and some sort of of maintained registry page (at our doc website) like it's done for nuxt modules would probably work also.
added MR to enable it and other missing modules. thus the module is now part of the setup.
added the modules: https://github.com/drunomics/lupus-decoupled-project/commit/82624968413d...
but they are not auto-enabled yet.
great! thank you, merged.
I merged this and fixed the gitpod setup to work gain, thus closing.
Status:
I worked over the docs during last weeks and I'm almost done. However, a few pages still need work - so adding todos for them.
todos:
* frontend: custom routes update
* community: ddd23 links + ddd23 page
* work over "getting started"
* add hosting options
*
📌
Document default components at website
Active
created 🐛 User logout confirmation form is not supported (d10.3+) Active for the logout issue
I was trying to to test this, but actually failed logging out.
I figured there is an issue with logouts. It used to work before, so probably https://www.drupal.org/node/2822514 → caused it.
When the logout is not working, you stay logged in, and the /user/login route shows access denied. Maybe this is what is happening here?
Please try to reproduce with the gitpod online demo and provide reproduction steps.
Yes, that makes sense to fix. But I think it's not right atm.
When another language, not the current content language, is shown, defaulting to the current content language is not what you want. We'd have to forward the selected langcode. So we must pass the entity language as $langcode when calling this.
Next, the bug might exist in other processors also. When we fix it, we should fix it in all of them.
In 3.x, we should make sure our formatters do that correctly. Since 3.x is our main development branch, we should first develop it for 3.x and then backport in 2.x
sounds great, good idea! only comment I have is that we should put the generators in the right modules, i.e. custom elements stuff into custom elements.
Chiming in here to add some background why GPL is a no-go for many/most Javascript frameworks:
The problem with React/Vue/.. any modern JS frontend is that the templates are JavaScript and transfered to client AND get bundled together typically by route or even by app. Thus any code, or components which end-up in the client-side bundle can easily "infected" by GPL as soon as some GPL library is somewhere used in the app and ends up in the client-side bundle. That would make the whole client-side JS app GPL, what is a no-go for most businesses and licenses troubles people do not want to get into (including myself). That's why in the JS frontend world permissive licenses, like MIT, are standard.
Thus, for decoupled projects, like https://www.drupal.org/project/lupus_decoupled → hosting of frontend code requires MIT. Currently that forced us to move all frontend things to github. We'd love to have things like decoupled themes on drupal.org in the future, but the license is a showstopper here atm.
> I don't think that Experience Builder is currently in violation of current d.o. policies around this. We are using MIT licensed code (e.g., React), but that doesn't prevent the Experience Builder module as a whole being licensed under GPL.
While this is not a problem for experience builder per se, it might become a problem if its used for react based frontends in the future. When Drupal provides JS/react-code, I assume it typically woudl be its own bundle, and thus not a problem. However, people might want to include this react code in some decoupled react frontends in the future, for which the GPL license woudl be a blocker again. In order to avoid blocking future integrations like this, I'd suggest to pick a more permissive license there for JS-components/libraries also.
fixed links and generally slightly improved the docs
https://www.drupal.org/project/services_env_parameter/releases/8.x-1.4 → shuold fix the first one