Problem/Motivation
Follow up of #2643158: AJAX library not added when cart is empty
Right now we are using dc_ajax_add_cart_include_cart_resources() to add CSS, JS resources and AJAX library which are required to render the cart and make it functional.
As Matt suggested here #2643158-10: AJAX library not added when cart is empty, lets use hook_library() to load the resources.
Proposed resolution
Remove dc_ajax_add_cart_include_cart_resources() and use hook_library() to load the resources.
Remaining tasks
N/A
User interface changes
N/A
API changes
Remove dc_ajax_add_cart_include_cart_resources() and use hook_library() to load the resources.
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | Screen Shot 2017-07-05 at 11.58.10 PM.png | 71.1 KB | subhojit777 |
| #7 | interdiff.txt | 1.02 KB | joelpittet |
| #7 | 2891050-7.patch | 2.11 KB | joelpittet |
| #5 | 2891050-5.patch | 2.08 KB | joelpittet |
| #5 | interdiff.txt | 1.02 KB | joelpittet |
Comments
Comment #2
subhojit777Comment #3
joelpittetNot sure what to add for some of the things, like what to name the library, I went with 'replace' because that is what it was doing for the command, kinda boring you could probably think of something better;)
Comment #4
subhojit777Thank you. Looks good to me. Can we call it
main, as that is the main library that makes the ajax cart functional. We could have called itreplace, but the CSS, that is not playing any part in "replace"-ing, so I think the namemainsuites here more :) I am open to more suggestions.Also I was thinking why
dc_ajax_add_cart.jsis not part of the library. I checkeddc_ajax_add_cart.js, and I decided we can optionally attach that JS as perdc_ajax_add_cart_display_popupsetting. We are going to add that check insidedc_ajax_add_cart_form_alter(). We can open a new issue for that. Let me know what you think.Comment #5
joelpittet@subhojit777 yeah 'main' is fine with me, I've seen other's use 'base' too, but it's all a bit of personal preference I believe and makes itself more evident when you add more.
The
dc_ajax_add_cart.jslooks like it's included in the right place, it could be in it's own library and attached that way but I don't see any really benefits off hand. Maybe a separate issue if you'd like to explore that more.Comment #6
subhojit777Makes sense.
The patch is perfect!! Can you please reroll the patch. And while you are doing it, can you please update the library name to
base:DComment #7
joelpittetNo problem, changed to base and re-base-rolled.
Comment #8
subhojit777Thank you. I am seeing this JS error, just on page load.
Comment #9
joelpittetThanks that is bizarre, the order of the files seems to be wrong
Comment #11
subhojit777The ordering was right. The issue was with the weight. https://api.drupal.org/api/drupal/modules%21system%21system.module/funct...
Comment #12
subhojit777The tests were passing because simpletests are buggy when it comes to testing JS behavior. Ideally the tests should be browser automated tests.
Comment #13
joelpittetI would have expected not putting a group would have done JS_DEFAULT, which is the weight of 0. AND because it has a dependency it would order after the thing that's needed... hmmm
Comment #14
subhojit777Yeah.. it is a supposed-to-be assumption, but still not :)