🇧🇪Belgium @svendecabooter

I have provided some feedback to the MR.

Thanks! Committed

Added support.

svendecabooter → created an issue.

Thanks for the MR. Committed now

Applied and committed

Looks like a good improvement. Thanks for the MR.

Patch does no longer apply to latest codebase in 1.0.x

Thanks for the MR. Committed now.

Thanks for the MR & reviews. Committed now.

Provided patch no longer applies on the 1.0.x branch.
Could you reroll the patch or create a MR with the changes?

Thanks Pieter! Committed now.

Thanks for the MR & reviews. Committed now.

Thanks for the MR & reviews. Committed now.

Thanks for the MR and reviews. Merged now.

Patch #8 committed

Thanks for the MR & reviews.
I have merged this in.

Sent a mail to maintainer ayalon through their drupal.org contact page:

Hi,

I noticed the module layout_builder_iframe_modal that you maintain, has not received any recent updates, while there are a bunch of RTBC issues in the queue, including D11 compatibility.

I would like to offer myself as comaintainer of this module.
Please see the public issue regarding this in the issue queue: https://www.drupal.org/project/layout_builder_iframe_modal/issues/3493550 📌 Offering to co-maintain layout_builder_iframe_modal Active

Let me know what you think.
Glad to help with keeping this module up to date.

Kind regards
Sven

Other maintainer dulnan cannot be reached through their drupal.org profile.

svendecabooter → created an issue.

esmoves → credited svendecabooter → .

pjonckiere → credited svendecabooter → .

I have updated the MR with split permissions.
Through permissions you can now decide whether to give a user access to:
- clear caches
- run cron
- run updates (no extra permission added for this, since it is managed by core routing)

I don't think individual cache flush actions don't need their own specific permissions?

I have also added a specific `/admin/tools` route that just lists the children menu items. This is so the "Tools" heading in the navigation bar points to that page, instead of the frontpage. This also avoids the "Tools" menu being shown in the navigation toolbar, when a user does not have any of the new permissions assigned to them.

Attached also a .patch version of the full MR, for composer patch workflows.

Looks good to me.
Hopefully a new D11-compatible release can be tagged soon by the maintainers.

Can a new release be created for Drupal 11, so we do not have to rely on the lenient Composer endpoint?
Especially given D11 support does not require any code changes, other than tagging the module as D11 compatible.

Reviewed the MR and this seems to work as intended for Drupal 11.
Can this be merged in and a new release created with Drupal 11 support, given Drupal 11 has been released for a while now?

Sounds like a good idea, we will need a D11 version of this module soon too.
@dillix can you already add an issue with an issue fork with D11 compatibility perhaps?
Then I can have a look at reviewing that MR.

borisson_ → credited svendecabooter → .

Potentially related core issue: ✨ Allow modules to hook into top of content/footer sections of new core navigation Active

Updated patch file.

Created MR to add this functionality.
Attached is a patch file for composer based patching workflow.

svendecabooter → created an issue.

borisson_ → credited svendecabooter → .

Thanks, that looks like a good solution.

Thanks, the updated README has been committed.

Looks good to me to be merged in!
As far as I can see this will both keep working in D10 and D11.

borisson_ → credited svendecabooter → .

Regarding the categories:

The "Developer tools" category was already selected, together with "Access control" and "Integrations".
Is the recommendation to remove the latter two? Or keep it as it is?

Gave some comment in 📌 External Authentication - create a short description Needs work and taken the liberty to adapt the summary field with my suggestion. Please let me know if this is considered inappropriate.

Thanks for the help in creating a short description for the module.

Would it be OK if I still adapt the description, if it's within the 200 character count limit?
I'd adapt it to be more in sync with the text that is already on the module page:

Provides a generic service for logging in and registering users that are authenticated against an external site or service and storing the authentication details.

This because "registering authenticated users against an external site or service" does not sound to me like what the module is actually doing. It is not connecting to external services and registering there. It is helping modules that connect with external systems - where users got registered - to register an accompanying / connected Drupal user account.

So the proposed description feels like it would create more confusion about the actual purpose of the module.
Maybe it's because I'm not a native English speaker and interpret it a bit differently...

Thanks!
I raised a remark about the logo in 📌 External Authentication - create a logo Needs work , but will commit it already to the project.

I'm not sure this module is very well suited within the concept of Project Browser, since it's purely a helper module that is required by other modules. People should not be encouraged to install it stand-alone, and expect it to do something. I'm not sure if there's a way in Project Browser to indicate that. I'm not really up to speed with the Project Browser initiative...

Hello,

I was made aware of this logo creation through the issue 📌 Update logo for compatibility with Project Browser Active .
First of all, thanks everyone involved in creating the logo!
I really like the logo style.

I'm not sure it's appropriate to re-open the discussion here though, but as roderik points out above, conceptually it would make more sense if the arrow in the logo would be reversed, pointing inwards, since the module's purpose is to help external authentication systems authenticate users into Drupal.

But even with non-existing design skills I can see it's not as easy as just flipping the arrow around, so I'm OK with using the logo as is, unless the community doesn't mind another iteration. I don't want to sound like a crappy "make the logo bigger" client though :)

PS: i'm not sure how structured the logo design initiative is, but it would be good to make module maintainers aware of the existence of the logo issue earlier on in the process, so feedback like this could be added in a more timely matter.

Created a new MR for the 4.x branch + adding patch file.

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

This module's functionality is available in Drupal core, so no need to provide a D10 version of this module.

The module does not have a dependency on SMTP module, nor on Token.
Not sure what this patch is trying to solve...

Thanks, fixed now.

Thanks, I have added the module to the list on the frontpage.
You could also add "External Authentication" to the Ecosystem field on your module settings, so it's also linked there...

I rebased the MR branch with the latest 11.x.
Locally I also got the '5 errors in system.theme.global:' error, thrown in the pipeline output as well.
I then fixed incorrect config in 2 core themes. When running config:inspect locally, that fixes the issue.
However, the latest Gitlab CI pipeline still shows this error, plus (unrelated?) phpunit test failures.
Not sure what's going on there...

svendecabooter → created an issue.

svendecabooter → created an issue.

8.x-1.2 does support D10.

svendecabooter → created an issue.

You probably need to update the tocSelector, content_selector and heading_selector settings?

Thanks for the work and review.
Can this be created as a merge request, and the mentioned issues looked into?

svendecabooter → created an issue.

Fixed - marking issue active for further updates.

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

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

Added D11 support in 3.0.x branch

Marking this one as fixed, since https://www.drupal.org/project/facets/issues/3455217 📌 Automated Drupal 11 compatibility fixes for facets Active takes care of the 3.x D11 compatibility