An idea from sun and coming from #1090592: [meta] Use HTML5 data-drupal-* attributes instead of #ID selectors in Drupal.settings the module page is a pretty good use-case.
When checking to enable a module there is a JS check on dependencies and check all unchecked dependencies hence avoiding the confirm step.
The patch is very rough but it works :) any comment on this is welcome. It should be using jquery ui instead of a confirm (or maybe not, which is better for screen-readers?).
I'm not certain i'll be super-fast with 100+ modules enabled, there is a lot of room for perf improvement at this point.
Comment | File | Size | Author |
---|---|---|---|
#24 | use_data_to_check-1473760-24.patch | 4.26 KB | alansaviolobo |
#22 | use_data_to_check-1473760-22.patch | 1023 bytes | alansaviolobo |
| |||
#20 | core-js-modules-dependencies-1473760-12.patch | 4.99 KB | capynet |
#18 | core-js-modules-dependencies-1473760-11.patch | 4.41 KB | capynet |
#7 | core-js-modules-dependencies-1473760-7.patch | 3.42 KB | nod_ |
Comments
Comment #1
nod_me no right spelling.
Comment #2
nod_forgot this too.
Comment #3
cosmicdreams CreditAttribution: cosmicdreams commented@nod_ I looked into doing something like this a while back as well. But quickly got distracted by all the markup that is injected into the render array for the module page and sought out to search and destroy all instances of from that page.
Find the code that builds up the markup for the module page and you'll see what I mean.
IMO, the addition of data attributes gives us a great opportunity to work on the markup being output here. I think a refactoring of the markup should be apart of this patch.
As a next step, I'll try to put out a patch that shows what I mean.
Comment #4
nod_Comment #5
RobLoach#1: core-js-modules-dependencies-1473760-1.patch queued for re-testing.
Comment #6
Fabianx CreditAttribution: Fabianx commentedWow, this would be great to have in ...
Comment #7
nod_reroll for the fun of it :)
Comment #8
Wim LeersI think this can happen post-feature freeze?
Comment #9
Bojhan CreditAttribution: Bojhan commentedI am a little skeptical about this, could this have some screens on the intended flow?
Comment #10
nod_#7: core-js-modules-dependencies-1473760-7.patch queued for re-testing.
Comment #12
capynet CreditAttribution: capynet commentedRe-rolled.
And added modal with a list of modules being marked.
Comment #13
capynet CreditAttribution: capynet commentedReplaced $.ui.dialog() by Drupal.dialog();
Comment #14
capynet CreditAttribution: capynet commentedReplaced wrong library "drupal.dialog.ajax" by "drupal.dialog"
Comment #15
tstoecklerThe patch includes the patch file itself... :-)
Comment #16
capynet CreditAttribution: capynet commentedFix patch loop o_O
Comment #18
capynet CreditAttribution: capynet commentedRe-rolled.
Comment #19
nod_the dialog dependency should be in the library definition of drupal.system.modules, not attached with it.
Can you put
gettingDependencies: false
outside Drupal.behavior? We should only have attach/detach in there.$that.is(':checked')
to replace by$that.prop('checked')
Drupal.dialog expects a HTMLElement, not a jQuery object.
Can you use the Array version of the button option? (We're told it's the preferred way to declare them http://api.jqueryui.com/dialog/#option-buttons).
Drupal.system is useless after all, I can bet people will confuse it for something it's not. Let's drop it and have the getDeps as a banal function in the closure.
The wording could use a little help. Feels different than the usual confirm step.
Can you also make the click handler a function instead of anon function? and also separate the declaration of the dialog options from the call of Drupal.dialog? There is too much nesting, we need to bring it down.
And as an optimisation for getDeps, we could try to use CSS selectors to do the work, kinda same way it's done in #2180921: [META] Use data attributes rather than classes wherever possible.
Thanks!
Comment #20
capynet CreditAttribution: capynet commented"The wording could use a little help. Feels different than the usual confirm step."
In fact, I copied the confirm step.
"And as an optimisation for getDeps, we could try to use CSS selectors to do the work..."
Sorry, I dont understand what you mean.
Comment #21
nod_What changed exactly, still seeing extra stuff in Drupal.behaviors, anon functions, not a HTMLElement in Drupal.dialog call and too much nesting.
We can delegate the event as well here.
Comment #22
alansaviolobo CreditAttribution: alansaviolobo commentedreroll
Comment #24
alansaviolobo CreditAttribution: alansaviolobo commentedsorry. wrong patch added earlier.
Comment #29
martin107 CreditAttribution: martin107 as a volunteer commentedwe MUST no longer be modifying *.js files directly
We modify *.es6.js and then transpile as described here :-
https://www.drupal.org/node/2815083
as a ultra trivial point new code in ModulesListForm uses array() when [] is preferred.
Comment #35
nod_hesitating on closing this one. Is there really a UX benefit of doing this?