Utrecht
Account created on 26 April 2005, over 20 years ago
#

Merge Requests

More

Recent comments

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

1. Regarding #43.1: PHPCS issues have been addressed

2. Regarding #43.2: I think by default the value should be false. I will correct that in the summary.

3. Regarding #43.3: can you provide reproduction steps for this?

4. I improved the label of the check box. It contained a double negation: "Do not add a menu link for taxonomy terms with no items" , which can be a bit harder understand.

6. As noted in #18, this also needs a test. So I added a functional test for checking taxonomy term menu items with and without associated nodes.

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

This route does not seem to be in use anymore. I could not find any references to it.

I proposed to remove it.

This can be reviewed.

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

batigolix created an issue. See original summary .

🇳🇱Netherlands batigolix Utrecht

I cleaned this up and removed the AI reference. This can be reviewed.

I should note, though, that I did use Claude Code to generate the first draft.

🇳🇱Netherlands batigolix Utrecht

I also confirm that the changes introduced in the merge request solve the problem

🇳🇱Netherlands batigolix Utrecht

1. This issue is quite old and it seems to me that the problems reported over the years are somewhat how to reproduce because of the code changes over those years.

2. I think the 🐛 Cannot disable Menu Link unless Taxonomy term is unpublished Active actually solves the problem that is described in the issue summary. I propose to set this status to postponed until the mentioned issue is merged.

3. The proposed change from #25 that is in the merge request it's dealing with an other issue. It mentions taxonomy term status which wasn't part of the original problem. I think the merge request should be rejected.

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

@marios anagnostopoulos : this issue is already closed, so yes: please make a new issue for this.

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

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

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

With the help of claude code I improved the issue summary so that it becomes more clear what this feature is about

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

The phpunit test that has been added works well.

The user experience is indeed not very intuitive (as mentioned in the description). I think this needs a drop-down where you can select the parent term. Maybe this option should not be available when creating the taxonomy menu.

I'm not sure if this is a desirable feature because it adds complexity and there are already options to disable items.

🇳🇱Netherlands batigolix Utrecht

With a little bit of help from claude code, I was able to fix these.

I also increased the strictness of phpstan.

This can be reviewed.

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

batigolix created an issue.

🇳🇱Netherlands batigolix Utrecht

I cannot reproduce the error anymore. This is not a surprise given the fact that the issue is already seven years old :|

Still, Damien has a good point: this needs to be covered by a PHPunit test.

With the help of Claude I created one and this can be reviewed.

🇳🇱Netherlands batigolix Utrecht

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

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

With the help of Claude Code I created a first draft of the improved readme

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht

I would propose to do even a little bit more:

1. Review the readme documentation for any confusing parts like the one mentioned in it issue description.

2. Ensure that the documentation follows the recommended template
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

3. Clean up the existing drupal handbook documentation pages. Maybe remove anything that is D7 or D6 related. https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib...

4. Refer to readme as official documentation instead.

🇳🇱Netherlands batigolix Utrecht

I hope I understand the feature request well:
you would like all new menu items created from terms to be automatically *disabled* menu items?.
You flip the problem here because this means that the users that want to add multiple terms as menu items will have to *enable* these one by one. So they will have the opposite problem then you have.

I would advise against the proposed solution in the MR. This automatically sets everything disabled.

You could think of adding an additional configuration option in the Taxonomy menu configuration. So that users who want this option can choose to have all menu items disabled by default.

🇳🇱Netherlands batigolix Utrecht

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

🇳🇱Netherlands batigolix Utrecht

Here is Claude Code vibe coded first stab at webform analysis for likert elements.

🇳🇱Netherlands batigolix Utrecht

Unfortunately, my front end skills are failing me here. I would appreciate if anybody else could pick it up from here.

🇳🇱Netherlands batigolix Utrecht

Thanks for the reminder. The release has been created

🇳🇱Netherlands batigolix Utrecht

batigolix changed the visibility of the branch improve-modal-3540181 to active.

🇳🇱Netherlands batigolix Utrecht

batigolix changed the visibility of the branch improve-modal-3540181 to hidden.

🇳🇱Netherlands batigolix Utrecht

batigolix changed the visibility of the branch 3540181-improve-modal to hidden.

🇳🇱Netherlands batigolix Utrecht

There were some conflicts as a result of 🐛 Replace usage of 'whitelist' with 'allowlist' Active

I fixed those so that it can be reviewed again

🇳🇱Netherlands batigolix Utrecht

Thanks for helping out with this one

🇳🇱Netherlands batigolix Utrecht

batigolix changed the visibility of the branch 3.0.x to hidden.

🇳🇱Netherlands batigolix Utrecht

Many thanks to everybody helping out with this one

🇳🇱Netherlands batigolix Utrecht

batigolix changed the visibility of the branch 3540181-improve-modal to hidden.

🇳🇱Netherlands batigolix Utrecht

With the help of Claude I created a first draft

🇳🇱Netherlands batigolix Utrecht

Weight is included in the output, so you can use it to order. See for example:

       "link": {
            "id": "menu_link_content:32bd5d0f-0c96-470c-bb17-f30bfd113638",
            "weight": "11",
            "title": "phanecric",
            "description": "Description of phanecric.",
            "menu_name": "main",
            "provider": "menu_link_content",
            "parent": "",
            "enabled": true,
            "expanded": false,

Feel free to re-open the issue if this does not serve your needs.

🇳🇱Netherlands batigolix Utrecht

Coding standards issues have been fixed. Phpunit still fails but that is in theory not part of this issue

🇳🇱Netherlands batigolix Utrecht

I tested this with subpathauto 1.x, redirect 1.11 and drupal 10.5 and I cannot reproduce the issue.

Please provide more detailed reproduction steps if the problem still exists.

🇳🇱Netherlands batigolix Utrecht

I tested this with subpathauto 1.x, redirect 1.11 and drupal 10.5 and I cannot reproduce the issue.

Please provide more detailed reproduction steps if the problem still exists.

🇳🇱Netherlands batigolix Utrecht

I propose to postpone this, and go for the more generic solution of Ignore specific paths Needs work

🇳🇱Netherlands batigolix Utrecht

@lazzyvn please se the merge request for any changes.

Personally, I think the generic solution provided in this issue is better than the specific solution proposed in Add option to ignore admin paths Needs review

I tested the MR and it works well.

🇳🇱Netherlands batigolix Utrecht

Check the README.md (included in the merge request) that explains how to create a separator between the prioritized and normal countries. This will enable the indentation.

As an extension of a default Drupal select field, there is no setting for the size of the field. It adapts to to the longest available value.

Please explain the issue with the whitespace in the patch (you mean in the merge request?) . The code passes the gitlab CI checks for code styling, so it should be good. But please explain in more details where it can be improved.

🇳🇱Netherlands batigolix Utrecht

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

🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
🇳🇱Netherlands batigolix Utrecht
Production build 0.71.5 2024