It would be great if the names of missing modules would be clickable and take you directly to the project page.

Putting the following code in module_filter.module (line 104) takes you a long way:

foreach($modules[$module]['#requires'] as $requires_name => $requires_text){
          if(strpos($requires_text, 'admin-missing')){
            $pos = strpos($requires_text, '<');
            $requires_name_human = substr($requires_text, 0, $pos - 2);
            $modules[$module]['#requires'][$requires_name] = l($requires_name_human, 'http://drupal.org/project/' . $requires_name) . substr($requires_text, $pos - 2);
          }
        }

The one problem I couldn't figure out is how to get the project name of a module. In most cases, this isn't a problem, but if a module requires e.g. 'page_manager', it will try to link to http://drupal.org/project/page_manager, while it chould be http://drupal.org/project/ctools.

CommentFileSizeAuthor
#19 claro D10.1 without module filter.png140.47 KBjonathan1055
#19 claro D10.1 with module filter.png174.67 KBjonathan1055
#8 requires before and after.png383.13 KBjonathan1055
#8 example using paragraphs.png206.75 KBjonathan1055

Issue fork module_filter-1860304

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greenSkin’s picture

greenSkin CreditAttribution: greenSkin commented
Version: 7.x-1.x-dev » 8.x-3.x-dev
Priority: Normal » Minor
Issue summary: View changes
smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Active » Postponed (maintainer needs more info)

Hello thank you for the ticket sorry it's been so long (Just took over the module today).

As this does sound like a valid use case do you think this could be served in core though?

maartendeblock’s picture

maartendeblock CreditAttribution: maartendeblock as a volunteer commented

It's a very basic usability feature. I very much think there is a big use case for this. Even if you end up installing it with composer, it's good to be able to go to the correct project fast.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Version: 8.x-3.x-dev » 4.x-dev
Status: Postponed (maintainer needs more info) » Active

This has always bugged me. It would be great to have a link to the project, and it is such as simple addition. Yes, Core should provide it, but realistically that is quite low on the priority list now, so it could be years more before anything gets done. If module_filter provides it, we can then demonstrate the usefulness and show that it should be moved into core.

... how to get the project name of a module. In most cases, this isn't a problem, but if a module requires e.g. 'page_manager', it will try to link to http://drupal.org/project/page_manager, while it should be http://drupal.org/project/ctools.

We need to solve this problem though. I'll start by making a MR branch from the work done by @maartendeblock then we can move on from there.

maartendeblock’s picture

maartendeblock CreditAttribution: maartendeblock as a volunteer commented

We have more modern tools now. Dependencies are now also managed through composer. Maybe that can help?

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Just a few things to consider

1. Should be a setting that could be disabled
2. 508 if you are linking to 100s of module pages now we need to make sure they pass accessibility.

jonathan1055 opened merge request !12

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Priority: Minor » Normal
Status: Active » Needs review
FileSize
example using paragraphs.png206.75 KB
requires before and after.png383.13 KB

First draft of the link for missing modules. I based it on @maartendeblock code from the issue summary, but had to update it as the form structure has changed in the TEN years since the issue was started. This must be a record for elapsed time with so few comments between.

Replying to @smustgrave points in #2, it could be a setting but I don't currently see the downside in having it on all the time. There won't be that many links created, as mostly when a module is added the admin will get the dependencies or discard the module. Yes, we will make sure the links follow 508 compliance. That can be checked now that this first version is available for testing.

I used the Paragraphs module, which has a 'paragraphs demo' sub-module, which needs a few things my test site did not have. I added <strong> tags round the module name, to alert the user that this is different from all the non-links.
paragraphs example

Here is the 'requires' item before and after.
requires - before and after
One slight difference is that the protected stringTranslation is null in the new version. If anyone has an idea why, or if it matters, or how to fix it, let me know.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

Tested the MR but I get this fatal error

TypeError: array_filter(): Argument #1 ($array) must be of type array, null given in array_filter() (line 155 of core/modules/system/system.admin.inc).
template_preprocess_system_modules_details(Array, 'system_modules_details', Array)

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

Hi @smustgrave
Thanks for testing. What core version did you use? I can't replicate this error, but it could simply be because you have a different set of modules on your page. Can you work out which one is causing the problem? Start with a new site, no contrib modules, do you get the error for core modules? Then add each of yours one by one until you get the error. Or give me a screenshot of the list and I can try it.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

On Drupal 10.1.x with a standard install.

rough steps.

Tried it with just module filter is my contrib when I install the module nothing happens.

If I add a module (tried admin_toolbar and stagefile proxy) nothing happens.

If I leave the modules page it and try to come back I see it.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Think I found the why. For development work I have
$settings['extension_discovery_scan_tests'] = TRUE;

So a test module could be triggering this.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs work » Needs review

Added a simple check that should fix it.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

From #12

For development work I have $settings['extension_discovery_scan_tests'] = TRUE; So a test module could be triggering this.

I've enabled this in local.settings on a clean 10.1 site but still cannot replicate your problem. All the test modules now show. If a module does not define 'package' the array key is NULL but it still works. It would be nice and understand to see this error before we fix it.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

If we are testing then it needs to be right down to the [$requires_name] key.
isset($form['modules'][$package][$name]['#requires'][$requires_name]
If the check you added solved your problem, it must have been $package that was causing the problem, because $name would have to be correct. But as I said, I cannot replicate the error.

Is there a second class we can add for the 'missing' word, because in 10.1 admin-missing does not render in red.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

what about that?

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Status: Needs review » Needs work

That works. But I've just searched the D10.1 source and admin-missing is already defined (and set to red) in four places -

core/modules/system/css/system.admin.css
core/themes/claro/css/components/system-admin--modules.css
core/themes/claro/css/components/system-admin--modules.pcss.css
core/themes/stable9/css/system/system.admin.css

So I don't think we should have to define it again. We may be making things worse by adding another, effectively overriding core. Need to work out why it was not red before.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

When module_filter is not installed the missing text does show in red. So this module is doing something else differently and we need to understand what it is before making the css change.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
FileSize
claro D10.1 with module filter.png174.67 KB
claro D10.1 without module filter.png140.47 KB

The styling in Claro is quite different with this module installed. That should probably be looked at in a separate issue?

Without this module
without this module

With this module
with this module

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Maybe could look as a separate issue.

Tracked it down to the module_tabs.js file is some how the one messing it up.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs work » Needs review

Updated color to match claro.

Reason it's not applying is because the js is removing the .claro-details class.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

That's good, using the Claro variable not the hard-coded colour. I also see that if Olivero is used as the admin theme, then we don't get any red. But Olivero is not designed as an admin theme, and it does not render the text red even without this module, so I don't think we need to worry about that, as it is no loss of UI.

Let's leave the overall styling differences of Claro to another issue. My only final quesion on this is the point I raised at the end of #8 - the property stringTranslation is now null where it used to be an object of type TranslationManager. I don't know if this is significant, and whether it has an impact on how the 'missing' word can get translated.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

That I don't know but seems like an important issue if something is no longer translating

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Fixed

Sweet this is an excellent feature to have!

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

Thanks.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

Attempting to add credit to @maartendeblock by ticking the checkbox but not sure this will work.
@smustgrave, as maintainer, you should be able to tick the checkbox and save, now that the status is 'fixed',

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.