Have we considered the merits of using javascript to enhance the existing server-side validation of modules' dependencies?

Here's an initial take on how we could do this. We pass modules' dependency data to the browser as a setting. When a module is enabled, we also enable any modules it depends on (and any modules that that dependency depends on, etc.) and set the dependencies to disabled (so they can't be unchecked). When a module is disabled, we check its dependencies and, if they have no remaining dependent modules that are checked, reenable them so they can be unchecked if desired.

The main advantages are potentially saving some back and forth to the server and improving the visual display of dependencies.

Would we also need to change the existing messages about what dependencies are and are not enabled? I'm not sure. Maybe not, if we consider that this is about what is already saved, not what is currently being edited.

Files: 
CommentFileSizeAuthor
#22 107038-javascript-module-selection.patch3.86 KBjcnventura
#19 107038-javascript-module-selection.patch3.31 KBjcnventura
FAILED: [[SimpleTest]]: [MySQL] 23,319 pass(es), 0 fail(s), and 116 exception(es).
[ View ]
#18 107038-javascript-module-selection.patch1.99 KBjcnventura
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 107038-javascript-module-selection.patch.
[ View ]
#3 system-module-dependencies-js_1.patch4.83 KBnedjo
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-module-dependencies-js_1.patch.
[ View ]
#1 system-module-dependencies-js_0.patch3.54 KBnedjo
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-module-dependencies-js_0.patch.
[ View ]
system-module-dependencies-js.patch3.5 KBnedjo
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in system-module-dependencies-js.patch.
[ View ]

Comments

nedjo’s picture

StatusFileSize
new3.54 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-module-dependencies-js_0.patch.
[ View ]

Minor bugfix: replace underscores with dashes when selecting module checkboxes by id (needed because some modules have underscores in their names, which are replaced by dashes when they're fed through form_clean_id()).

To attempt a clearer summary:
Currently, checkboxes on the modules admin page are disabled selectively to reflect dependencies (a module's checkbox is disabled if any module dependent on it is already activated). This patch adds some client-side functionality:

1. When a module is clicked from unchecked to checked, look at its dependencies. If there are any, ensure they're checked and disable them (because they're now required).

Example:
Taxonomy module is checked, forum and comment are unchecked. User selects forum (dependent on comment and taxonomy). Comment becomes checked, and both comment and taxonomy become disabled.

2. When a module is clicked from checked to unchecked, look at its dependencies. If there are any, see if they have remaining dependent modules that are checked. If not, reenable the checkbox so the dependency can be unchecked by the user (if desired).

Example:
Forum, comment, taxonomy, and taxonomy_access are all checked (comment and taxonomy disabled). Taxonomy has two dependent modules (forum and taxonomy_access) and comment has one (forum). User unchecks forum. Comment will become enabled--it has no further dependencies checked. Taxonomy will remain disabled--it still has its taxonomy_access dependent module checked.

dmitrig01’s picture

+1 from me
If you could re-roll (assuming it doesn't apply :P ), then I would test ;)

nedjo’s picture

StatusFileSize
new4.83 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-module-dependencies-js_1.patch.
[ View ]

Thanks!

Well, here's a quick refresh, compeltely untested I'm afraid. I've attempted to add one thing since the last iteration: if a module is missing, we need to know about that so as not to enable its checkbox.

If you're willing to test this as is, great! If not, let me know and I'll try to test and do any basic bugfixing (from the changes I made in this iteration) first.

chx’s picture

The idea is pretty good but I am a bit afraid of Drupal enabling anything without warning the user. I am not thinking of alert() that's a terrible solution, let's find something better

dmitrig01’s picture

We could flash something in $messages.
But that would probably be for a separate patch...

catch’s picture

Version:6.x-dev» 7.x-dev
Status:Needs review» Needs work

No longer applies and bumping to D6.

starbow’s picture

Subscribe

sun’s picture

I'll probably add qamodules module's feature to Drupal Administration Menu in the meanwhile, see #270547: Add Quick Admin modules feature.

meba’s picture

I was looking for this feature and luckily found this issue.

I think it may be more simple - When you disable a module1 which depends on module2, enable the checkbox for module2 so it can be disabled in the same step...

babbage’s picture

Assigned:Unassigned» babbage

The code to achieve this is now working for 6.x in Contrib Toggle module:
http://drupal.org/project/contrib_toggle

I am happy to port this code to 7.x and will look at this issue.

babbage’s picture

Implementation of this will require another patch along the lines described in #114546: Confirmation alerts done with JS/AJAX (delete confirmation page and any others). I would propose the best approach is to prompt the user for confirmation when due to an activation or deactivation of a module, other modules now need to be activated or deactivated due to dependencies. To do this without submitting the form (as is done in Contrib Toggle) requires a javascript confirmation dialog, however, and an infrastructure for that in core would be required to do a good job of that.

seutje’s picture

how about modals, on by default with and option to turn into regular js alert/confirmation boxes?

chx’s picture

Issue tags:+Usability

tagging

mlncn’s picture

Can this still go in? Came here by way of Starbow's module Quick Admin Modules, which seems to be the simplest base to be working in D6.

chx’s picture

I am fairly sure this can and needs to get in and also probably JS states now helps it a lot.

babbage’s picture

Assigned:babbage» Unassigned

JavaScript States system: #557272: FAPI: JavaScript States system

Also, while I may still work on this I'm more than happy for someone else to do this too, so unassigning it from myself.

jcnventura’s picture

StatusFileSize
new1.99 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 107038-javascript-module-selection.patch.
[ View ]

I've got it running fine using the Javascript states, with the only problem being that Drupal doesn't deactivate the modules which are required by others.

jcnventura’s picture

Component:javascript» system.module
Assigned:Unassigned» jcnventura
Priority:Normal» Major
Status:Needs work» Needs review
Issue tags:+D7UX
StatusFileSize
new3.31 KB
FAILED: [[SimpleTest]]: [MySQL] 23,319 pass(es), 0 fail(s), and 116 exception(es).
[ View ]

Improved version which disables the module selection checkbox for a given module when:
1) one of the modules that it depends on is not checked, and
2) any of the modules that requires it is checked.

The problem is that $form_state['values']['modules'] is passed to system_modules_submit() with the wrong info for disabled modules. I think this may be a bug with form processing that is going to cause some problems with the #states processing stuff.

Status:Needs review» Needs work

The last submitted patch, 107038-javascript-module-selection.patch, failed testing.

jcnventura’s picture

There's is a catch-22 in here... Imagine 2 modules where mod A requires mod B.

A) If mods A and B are enabled, Drupal will mark mod B disabled. In order to disable both of them in a single operation, that 'disabled' attribute must be reverted. So we need to change the local 'enabled' state.
B) If mods A and B are disabled, when trying to enable both at the same time, the states logic will mark mod B disabled, and by #335035: drupalPost() incorrectly submits input for disabled elements the input for mod B is not transmitted in the $_POST data. We would need to use the 'readonly' attribute for that, but we can't do that because of point A. Besides, readonly seems not to work properly in Firefox at least.
C) If mod A is disabled and mod B is enabled, enabling A or disabling B will be successful, but that worked fine without states anyway.

So, either Drupal submits data for disabled inputs in this case or I am afraid we'll never get this working with states.

João

jcnventura’s picture

StatusFileSize
new3.86 KB

Still not much progress. I am calling it quits at this time.. I did document the code a bit better and added a check on distribution-required modules so that they can't be enabled back by the #states logic.

This can probably be done right once the 'readonly' attribute is properly supported for checkboxes in all browsers.

João

sun’s picture

Version:7.x-dev» 8.x-dev

Yes, this is D8 material at this point.

jcnventura’s picture

This is a MAJOR usability annoyance in Drupal, so I would hope that it could be applied before D8. I am thinking about enabling the modules via Javascript in order to circumvent the forms logic that is preventing this from working correctly.

João

lpalgarvio’s picture

i agree with jcnventura, but since there is already a code-freeze imposed, let's get the module patched for D7 or merged with some other module for now

lpalgarvio’s picture

bump!

nod_’s picture

nod_’s picture

Status:Needs work» Closed (duplicate)

With dialogs in core, it should be fairly easy to support it. I'd rather go the data- attribute way than the #states API way, that doesn't really scale to the crazyness of the module page.