🇳🇱Netherlands @batigolix

The issue for adding Redis support is ✨ Add Redis support Active

If there is someone that like to work on the issues mentioned in #5, please let me know. Otherwise I will close this.

batigolix → created an issue.

Thanks for the very nice addition. I tested this and it works well.

I have made some remarks on the UI text and code in the MR/

batigolix → created an issue.

I cannot reproduce this. I use Alternate or no user endpoint option under User info endpoint . When logging in the email address is correctly filled in. Are there any more steps that I am missing?

update redirect URI instructions

I reviewed the text and updated it:

- removed general information that is also available on the project page
- added links to documentation that is available on drupal.org
- used the recommended template ( https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... → )
- removed list of maintainers (i do not find this very useful)

Updating the module name

batigolix → changed the visibility of the branch 2.0.x to hidden.

batigolix → made their first commit to this issue’s fork.

The actual logic that blocks or excludes an account or ip address is done by Drupal Core.

The contrib module Flood Control provides a UI for Core settings and for Unblocking accounts or IPs.

Feel free to move this issue to the Drupal Core queue.

megachriz → credited batigolix → .

Great. I reviewed the MR and added 1 request.

Ok, thanks for pointing out that this is a widget. I did not study the patch well enough.

Good to hear that you have time to work on the maintenance of this module. Reviews and patches are always welcome.

This widget could be considered a new feature but it needs to work.

- It should work without the custom JavaScript, so maybe by using Drupal form state (see e.g. https://git.drupalcode.org/project/examples/-/blob/4.0.x/modules/form_ap...), or with collapsible fieldsets.

- The flag icons CSS should not be part of the code, since a similar feature is offered by the Flags module

- Continents need to be configurable, since there is no objective way of deciding which continents exist and to which continents certain countries (turkiye for example) belongs to. I am thinking that maybe there should be a configurable way to group countries , not per se per continent, but something more generic. You could then group countries in all different ways.

It is possible that more requirements are being discovered later on, so keep that in mind when working on it.

I think also missing is "in_draft", which is a boolean.

batigolix → created an issue.

Thanks! This is a lovely idea.

But it adds a lot of extra code to maintain.

Also I am not sure if it is helpful in all situations if you first have to figure out which continent a country is in (what to do with Turkiye, Russia, etc.) . So it probably should be optional and configurable which adds a lot more additional code to maintain.

I will set this a "Wont fix".

People that like to have it can use the patch.

I tested this and it works well.

batigolix → created an issue.

Can the maintainer give his opinion on this generic solution for ignoring paths?:

✨ Ignore specific paths Needs work

I think that approach will clash with the one proposed in this issue above.

Also the UI is not very clear:

+      '#title' => $this->t('Apply sub-paths aliases to admin routes.'),

For the user it is not clear what "admin routes" mean. Does this also apply to taxonomy and user edit paths, for example?

I would avoid the word "route" because it is quite technical and because elsewhere in the UI it says "paths".

My proposal would be: "Provide sub-path aliases for admin paths"

+      '#description' => $this->t('This will enable routes like node/[id]/edit to also be altered.'),

This is a bit hard to understand.

My proposal would be "Creates aliases for admin paths like node/[id]/edit and user/[id]/edit"

+      '#title' => $this->t('Apply sub-paths aliases to layout builder route.'),
+      '#description' => $this->t('This will enable routes like node/[id]/layout to also be altered.'),

Same issues.

My proposal would be:
"Provide sub-path aliases for layout builder paths"
"Creates aliases for layout builder paths like node/[id]/layout"

I reviewed this and it works good.

The UI is also much clearer now.

There was a small code mistake though that I fixed. See my last commit in the PR.

Excellent job, immoreel!

So this error comes from the Flags module which cannot extend the CountryFlagAutocompleteController class anymore since a recent change in the County module.

I guess we should not have defined the CountryAutocompleteController as final class in the country module.

This needs a bit more research.

Thanks for the improvement.

Thanks for the sugegstion

The contrib module flood_control does not do this kind of logging.

Can you disable the contrib module flood_control and repeat the tests?

This may have its origin in the Drupal Core user flood functionality web/core/modules/user/src/EventSubscriber/UserFloodSubscriber.php

I tested the module with this fix and it works properly.

The pipeline has cspell and phpstan issues

A description is needed (with screenshot and or links) to reproduce the desired changed and some explanation on why this is necessary.

I created this issue by mistake in a forked sandbox project. I close it

batigolix → changed the visibility of the branch 3458111-fix_cs_issues to active.

Yes this issue still needs work.
No work has been done on this yet.

See the pipeline warnings:
https://git.drupalcode.org/project/dynamic_layouts/-/pipelines

This needs to be added to the merge request. Als o the CSS needs to be changed for this to work.

batigolix → created an issue.

I created a 3.0.x branch and a 3.0.0 release, and kept the 2.3x branch for D9.

I still need a fix for the D9 compatibility. Would be great if someone can make a MR of patch and test in with D9.

I merged this into a new 3.0.x branch for further D10 and D11 development.

The 2.3.x will not be updated and stay available for D9 for a while.

batigolix → created an issue.

The remaining fixes can be done at a later moment as they break D9 compatibility.

I tested this and it fixes the problem. This was committed and will be in the upcoming release.

Schema looks good. I committed this. Thanks for helping out!

This was already fixed in 🐛 Wrong namespace for feeds target Fixed

There is still a bunch of warnings. Let's try to fix these as well so that all pipeline checks are green

This is starting to look good.

Please update the readme to reflect these changes.

Also remove the hook_help() from the module file as I do not want to have additional explanation there, it tend to become outdated very quickly.

I rerolled the patch.

Leaving this open for future bot updates

We need to wait for D11 version for Feeds & Facets

This is outdated

batigolix → made their first commit to this issue’s fork.