Modules on the listing page can have their checkboxes disabled and there is no messaging to the end user as to why this is occurring.

My recommendation would be to offer help text as a caption and/or some hover text over the checkbox that describes why a checkbox is disabled for a user.

If this is tied to the module's dependencies, I would recommend that the help text state which modules are leveraging it.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

nerdstein created an issue. See original summary.

nerdstein’s picture

I'd be happy to work on a patch for this if someone doesn't beat me to it.

nerdstein’s picture

Line 263 of shows:

$row['#required_by'][] = $distribution . (!empty($module->info['explanation']) ? ' (' . $module->info['explanation'] . ')' : '');

This doesn't appear to have any impact on what is actually rendered (not confident, but still digging).

nerdstein’s picture

Status: Active » Needs review
FileSize
4.07 KB

Adding a patch for review that creates hover text for every condition in which the checkbox is disabled on the module listing page (there are several).

Please note - I did not add a help icon, but such a thing could be a logical addition to this patch.

phenaproxima’s picture

Status: Needs review » Needs work
Issue tags: +Needs usability review, +Needs tests

This looks great! Nice usability win. I see no major problems with the fix itself; all my suggestions are nitpicks. This will need tests before it can be committed, though. Thank you for your excellent work, @nerdstein!

  1. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -261,6 +261,7 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    +      $row['enable']['#attributes']['title'] = $this->t("@module required by @distribution", ['@module' => $module->info['name'], '@distribution' => $distribution]);
    

    Nit: Could the array of translation variables be split across multiple lines for readability?

  2. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -294,6 +295,7 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    +      $row['enable']['#attributes']['title'] = $this->t("@module is incompatible. @reasons", ['@module' => $module->info['name'], '@reasons' => $status]);
    

    Same here.

  3. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -313,6 +316,7 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    +          $row['enable']['#attributes']['title'] = $this->t("@module incompatible with version @version", ['@module' => $name . $incompatible_version, '@version' => $modules[$dependency]->info['version']]);
    

    And here.

  4. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -338,6 +343,7 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    +          $row['enable']['#attributes']['title'] = $this->t("@module is required by @dependency", ['@module' => $module->info['name'], '@dependency' => $modules[$dependent]->info['name']]);
    

    Here too.

  5. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -345,6 +351,12 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    +      $row['enable']['#attributes']['title'] = $this->t('@module is enabled and can be uninstalled from the `Uninstall` tab', ['@module' => $module->info['name']]);
    

    Und here.

  6. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -321,6 +325,7 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    +          $row['enable']['#attributes']['title'] = $this->t("@module incompatible with this version of Drupal core", ['@module' => $name]);
    

    Finally...can this be "@module is incompatible..."? :)

phenaproxima’s picture

Issue tags: +Needs screenshots

I just realized that screenshots would probably be helpful to the usability team. :)

Yogesh Pawar’s picture

Assigned: Unassigned » Yogesh Pawar
Yogesh Pawar’s picture

Assigned: Yogesh Pawar » Unassigned
Status: Needs work » Needs review
FileSize
4.29 KB
4.06 KB

Made changes as per suggestions addressed in #5 & also added an interdiff.

Yogesh Pawar’s picture

Status: Needs review » Needs work

Marking this issue for Needs Work for test coverage & screenshots.

Bojhan’s picture

Issue tags: -Needs usability review

Please, add the tag back when there are screens.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Meenakshi.g’s picture

After applying the patch it will add hover text over the checkbox that describes why a checkbox is disabled for a user.here are the screenshots for that

Meenakshi.g’s picture

Status: Needs work » Needs review
Bojhan’s picture

What about "Taxonomy is enabled. Disable from the Uninstall tab.".

We can also use Uninstall as a verb, but then you use the same word twice.

Meenakshi.g’s picture

Good idea done ! here it is with screenshot

rajeevku’s picture

Status: Needs review » Reviewed & tested by the community

#15 solves the problem stated in issue summary. Tested on 8.5.x, works fine.

xjm’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +needs accessibility review

Thanks for your work on this. Those disabled checkboxes are confusing so any incremental improvements we can make for them are a good idea.

Screenshots would still be helpful here, and maybe some light test coverage. (The issue has the "Needs tests" and "Needs screenshots" tags already.)

Also tagging for accessibility review to double-check that this is a correct use of the title attribute.

Thanks all!

xjm’s picture

Issue summary: View changes

Oh I missed the new screenshot because it wasn't embedded -- here it is:
 

Could we add screenshots of a couple of the other cases?

xjm’s picture

I really would love to just entirely remove the disabled checkboxes and instead group installed modules separately from uninstalled ones. Then the enabled group could just have a link to "Uninstall modules".

xjm’s picture

Issue summary: View changes
FileSize
107.71 KB

Something along these lines, like with actual icons/labels for the installed modules, rather than an un-uncheckable checkbox. Because making confusing things less confusing is better than adding help text.

xjm’s picture

Under the "Install new modules" header there could also be a link that says "Other modules are also available to download" and links to d.o. That way you have two different context, and a pointer to the out-of-scope-but-related thing you want in each of those contexts.

xjm’s picture

Issue tags: +Needs usability review

My mockup in #20 probably could use usability review before someone tries to implement it. It should be a decent incremental change since it would mostly leave the page the same (with the handy collapsible detailed infos on each module), but just sort the modules into two build lists rather than one. The enabled ones would have an icon/label instead of a disabled checkbox, reusing existing image assets that are used on e.g. the status report, migration report, etc., Then there's two added headers as well as two short but helpful help texts with links.

xjm’s picture

#21 and the proposed "Install new module" header in #20 get tangled up in #2577407: Module install form has two "install" buttons that do different things, so leaving that one aside for now.

Attached just implements the simplest part of it: Putting the installed modules in a collapsed group at the top, getting rid of the disabled checkbox, and linking the uninstall page. The implementation could probably be cleaned up.

xjm’s picture

  1. +++ b/core/modules/system/templates/system-modules-details.html.twig
    @@ -36,11 +36,19 @@
    +        {% else %}
    +        <td></td>
    +        {% endif %}
    

    There's probably a better way of doing this.

  2. +++ b/core/themes/seven/templates/system-modules-details.html.twig
    @@ -36,11 +36,19 @@
    +          {% else %}
    +          <label class="module-name table-filter-text-source">{{ module.name }}</label>
    +          {% endif %}
    

    This should probably not use a label when it's not an actual label for a form element, but I had some issues getting my added CSS to be respected, so I just used the label markup for now.

  3. +++ b/core/modules/system/templates/system-modules-details.html.twig
    

    This should match whatever gets added to the Seven template.

Status: Needs review » Needs work

The last submitted patch, 23: system-2890791-23.patch, failed testing. View results

xjm’s picture

Status: Needs work » Needs review
FileSize
10.32 KB

I was working off an old branch tip apparently. Same patch, rerolled. #24 and all still apply and this might fail some tests, but it'd be good to get usability feedback before working on it more.

Status: Needs review » Needs work

The last submitted patch, 26: system-2890791-26.patch, failed testing. View results

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.