@solideogloria

Let me know if you need me to provide a stack trace or anything else.

I don't use either as I'm still stuck on 2.x. The new Navigation module isn't stable yet, and there are open issues in the Core module I need fixed before I will be able to use it in production.

Is it a major change to a site to get from 3.2 to 3.3? If there's no upgrade path, maybe it should stay. I'd personally want 2.x listed more than 3.1.x or 3.2.x, because of the project usage → .

Even 2.x doesn't have that anymore.

Looks good to me.

I would consider the solution proposed in ✨ Navigating to first level menu items is not obvious Active to be the best approach to solving the problem, because it solves both the issue there and this issue.

I guess the problem here is that it looks like a title and so user's ignore it. How can we update the design to have this be clear that its a link?

With the proposed solution in the issue summary, it can remain a title because the icon itself will be clickable again.

I tend to middle-click or ctrl+click on links to open them in a new tab. I try to do this with the "Configuration" top-level nav item but it's not a link, so doesn't work. It would be good to make it a link. I can do the same with the "Extend" item because it is a leaf.

Same. The new module is hard to use because of the difference between the top-level links.

I approve of the proposed resolution:

A possible solution to that one (which could apply to the lower level as well) is to separate the menu item link from the ability to navigate to its child items

I have a site that uses this structure for its non-admin menu, and it works really well on both desktop and mobile. Items that don't have children can just be links, and items that have children but aren't themselves clickable can be expanded but not clicked. Works great.

Personally, I'd prefer if the menu item containing children could itself be clicked with ctrl+click or middle-click in order to open its link in a new window. That's how admin_toolbar works. You hover to open the submenu, and you can click to open its page.

Fixed typo

I applied the MR diff as a patch to my Drupal 10.4 site, and I still get the error.

I get this error when installing the new Navigation module. (I'm not using any patches for this issue)

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "navigation_layout" plugin does not exist.

This issue is prohibiting me from converting my site to the new module.

I think this is a Core issue.

solideogloria → created an issue. See original summary → .

AI is just a tool like autocomplete

Yes, however, autocomplete is not good enough to replace a human at a programming job, nor is it enough to create a module entirely on its own that is ready to be published.

but this is future tool that companies are quickly adopting and it improve teams velocity and also quality in long term with accurate review process and appropried prompts it is still human driven the AI will not do nothing that is not already done by humans. I understand that there is some resistence to the changes more when they are radical like this but we will see shortly how situation will evolve, Microsoft already start layoffs of engeeners and developers.

It's not as helpful as you think it is, nor as helpful as Microsoft thinks it is. Just look at the GitHub pull requests that Copilate is "helping" with:

https://old.reddit.com/r/ExperiencedDevs/comments/1krttqo/my_new_hobby_w...

The code written is full of errors and mistakes. A knowledgeable human can do it better. So rather than relying on AI-generated "slop" and pushing it live as-is, you either need to do a lot of manual (human-led) testing and review of the results, or you need to write it yourself, using the AI only for ideas and boilerplate code.

FYI, I scanned comments #17 and #19 with GPTZero, and it detects both comments as 100% written by AI. I have reported the comments as spam.

@mkalkbrenner Thanks for that! Can we have a new release as well?

Please update the template. The file is more concise now.

https://git.drupalcode.org/project/gitlab_templates/-/blob/main/gitlab-c...

Should this issue be closed? I've been using the module in Drupal 10 without issue for a while.

Is this issue still needed? Work could probably continue in

📌 New branch for 11.x, fix deprecation notices Active

Bump. This needs to be reviewed.

+1 RTBC. Looks great.

I also approve of the idea of a 2.x branch and release.

Confirmed. Please merge MR !9 rather than 7. MR 9 adds composer.json to the project.

When looking at the Update Status module's report, these two changes are the only changes it sees as needed for D11.

Very simple changes. RTBC. Please merge.

web/modules/contrib/webform_submissions_delete/src/Form/WebformResultsBulkDeleteForm.php (lines 124, 152)

Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.

Updates to these files cannot be patched with Composer. This needs to be merged for people to use it.

Looks good to me.

solideogloria → created an issue.

solideogloria → created an issue. See original summary → .

I don't know how to add a hook like this in a contrib module, since it's called in the core module's functions.

@jrockowitz Is there another way to solve this then, that would be acceptable? Perhaps a way to indicate that certain private fields should be viewable by "own node" users?

Could someone please review this? I've been using these changes successfully in production for around a year.

Both of the suggested issues are RTBC. I would like to see a D11-compatible release.

I agree. It's also becoming more important, since the 10.x EOL is this year.

Also, please use merge requests instead of patches. That way, the code review pipeline will run with the changes.

The issue summary should be updated to reflect whether this is specifically for D11 compatibility (in which case this issue should be closed as a duplicate of 💬 Drupal 11 Compatibility Active ), or to include information about the return type functionality that was re-added.

Re-adding removed functionality should be in a different issue. Also, it wouldn't be good to merge a patch that contains commented out code, as that doesn't comply with Drupal coding standards.

Please look at and review 🐛 2.0.x not compatible with Drupal 11 Active . That will help for 2.x, and then we can reroll the 2.x MR once that issue is fixed.

Please look at and review 🐛 2.0.x not compatible with Drupal 11 Active , which contains the change that was added to 1.x

Please look at and review 🐛 2.0.x not compatible with Drupal 11 Active , which contains the file that was added to 1.x

Please look at and review 🐛 2.0.x not compatible with Drupal 11 Active , which contains the README.md that was added to 1.x

Please look at and review 🐛 2.0.x not compatible with Drupal 11 Active , which contains the README.md that was added to 1.x

Also, could a maintainer please make it so that there is a dev version of 2.0.0-beta? Currently, the issues all target a specific version of 2.x since there's no "2.0.x-dev" in the version dropdown. Same with 1.x

The changes here will not apply once 🐛 2.0.x not compatible with Drupal 11 Active is merged. It will need to be rerolled.

However, that issue is for D11 compatibility, so please review and test it, and then we can reroll this issue.

This merge incorporates all of the D11 compatibility fixes that were applied to 1.x but not 2.x

Please review.

Would it be possible to allow strings to be entered for min/max date, and then call strtotime() in the lines something like this?

$max_date = new \DateTime(strtotime($this->options['date_range']['max_date']));

That way, users could set the min/max to relative values like -2 years

solideogloria → changed the visibility of the branch 3096023-add-option-to to hidden.

Never mind. I was wrong and missed that the CSRF fixes were applied, but none of the other commits were applied. The issue is fixed in 2.x

See the comparison between 1.x and 2.x:

https://git.drupalcode.org/project/restrict_route_by_ip/-/compare/2.0.x....

solideogloria → created an issue.

For some reason, the D11 changes were only made on 1.x.

https://git.drupalcode.org/project/restrict_route_by_ip/-/commit/059cc32...

These changes need to be made on 2.x as well.

Is it possible to use service injection and avoid the \Drupal calls in the class?

solideogloria → created an issue.

Given #309040: Add hook_requirements_alter() is in core since 9.5.x and up, seems the update_notifications_disable contrib could be ported to use that mechanism to suppress or alter the warning, not the problematic methodology it used per the summary here.

I’d strongly be in favor of closing this as “works as designed”

I disagree. The use of that contrib module is not the only use-case where the warning occurs. It also occurs if you actually disable the update module, for example if the server doesn't have access to the Internet, and the check for updates creates network connection errors in the logs every time cron runs. That's my use-case.

I would like a stable release. Are there any blockers?

Is this issue still relevant? The latest release says it added support for views.

If so, please create a Merge Request instead of using patch files.

The fixes are already in the branch now.

Looks like a lot of great code cleanup!

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

Ah, I see. Thanks.

Thank you for saying so!

Also, the link above doesn't filter to only my issues, for some reason. Here's a working link:

https://www.drupal.org/project/issues/search?submitted=solideogloria&sta... →

Looks great!

My quick guess is that the IF statement needs to be entered for more cases now, in order to track the middle days during a multiday event, so that concurrent multiday events get properly "pushed down", rather than displaying on top of each other.

It wasn't the newly merged-in changes that added $day_no, it's been there for the last 9 years, however, it does need to be removed for the existing changes in this issue to work.

Personally, I don't see why it was added, as it makes it so that any multiday event on the month calendar that isn't on the first day would be skipped over by the IF statement.

Or, if it was needed from another issue and the rebase, then we need to dig further to determine the proper change.

After looking on a whim, the issue is the addition of && $day_no === 1. Please remove that change.

The changes seem to break normal multiday events, even without concurrent ones. I'm going to look and try to see why.

There's several phpcs changes that are needed to pass code review.