Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Hi,
I have an small contribution, which helps to select the weight of the js or css added. How could I send it to you? I am used to git but not too much to patch, is it better if I make a patch instead of contributing via git?
Thank you.
Comment | File | Size | Author |
---|---|---|---|
#25 | cpn-n1912834-25.patch | 1.15 KB | DamienMcKenna |
#17 | cpn-n1912834-17.patch | 10.15 KB | DamienMcKenna |
Comments
Comment #1
joelstein CreditAttribution: joelstein commentedYou can upload a patch; that will do. Thanks!
Comment #2
alexmoreno CreditAttribution: alexmoreno commentedHere is the patch, let me know if everything is ok if someone try it :-).
Comment #3
DamienMcKennaCorrected the issue status.
Comment #4
DamienMcKennaClarified the issue title.
Comment #5
DamienMcKennaThis needs a little work:
Comment #6
alexmoreno CreditAttribution: alexmoreno commentedno problem, i will make the amends.
Thank you.
Comment #7
alexmoreno CreditAttribution: alexmoreno commentedready. Select box added instead textform and cleaned the code searching for possible problems with Drupal coding standards.
Regarding that last point, i found a comment in the wrong place, but I am not sure if the issue in the function which you are pointing is the set_error function:
I have modified it like that:
form_set_error('cpn', t('You must select a numeric value.'));
Let me know if it's right now please.
Thank you.
Comment #8
alexmoreno CreditAttribution: alexmoreno commentedsorry, found the issue with the Drupal coding standars in the tab. This is the right file.
Comment #10
alexmoreno CreditAttribution: alexmoreno commentedhow/where can i check why is this failing?
Comment #11
alexmoreno CreditAttribution: alexmoreno commentedI don't understand the issue, I made the patch under bluefish/Ubuntu.
I will create the park again just using vim
Comment #12
DamienMcKennaThe codebase had changed since you last pulled down the dev version.
I've made a few corrections to the setting, added a variable_del() line to hook_uninstall, and changed the weight values back to using JS_THEME rather than JS_LIBRARY. That said, I haven't tested it yet.
Comment #13
alexmoreno CreditAttribution: alexmoreno commentedthat's why it was failing my simpletest?
I will test it for you, no problem. Regarding using JS_THEME or JS_LIBRARY, I am thinking in some situations in which the administrator could want to have their js being shown before the LIBRARY (like jquery?). Could that happen? Maybe it would be a good idea to give the possibility of choose this value (JS_THEME or JS_LIBRARY)?
Thanks for your help Damien.
Comment #14
DamienMcKennaThere's a separate issue for adding an option to control the scope: #1294896: Option to control the scope the JS/CSS are loaded in
Comment #15
alexmoreno CreditAttribution: alexmoreno commentedI see it. I'd love to help, if you like my approach I can work with that idea. Assign it to me if you think I can help.
Also, i wasthinking that it would be better idea to move this global weight to the node creation itself, so instead of having the same global variable for all the nodes,the author could decide which weight give to each node individually.
Comment #16
DamienMcKenna@urwen: That'd be overkill, IMHO, you shouldn't need to monkey with the weight that much. If anything it might be worth expanding to have separate weights for the JS and CSS.
Comment #17
DamienMcKenna@urwen: How about this... First off it lets you change the CSS and JS weights separately. Secondly, it lets you manually enter an exact number for the fields, rather than only being able to adjust it slightly. This should cover for all (normal) use cases.
Comment #18
alexmoreno CreditAttribution: alexmoreno commentedSorry, I've been in holidays, in my home town in Spain... it's brilliant Damien, you have done a lot of good job.
I will test the path if that helps :-)
Comment #19
tusik CreditAttribution: tusik commentedIs this patch already included in the Sept 30th 2013 dev version?
Comment #20
DamienMcKenna@anficr: no.
Comment #21
tusik CreditAttribution: tusik commentedThank you.
Comment #22
alexmoreno CreditAttribution: alexmoreno commentedwhat do we need to include it in the next one?
Comment #23
DamienMcKennaComment #24
DamienMcKennaCommitted! Thanks for the help!
Comment #25
DamienMcKennaI spotted a typo after committing the change.
Comment #26
DamienMcKennaTypo fixed :)
Comment #27
alexmoreno CreditAttribution: alexmoreno commentedThanks Damien, glad to help :-)
Comment #28
DamienMcKennaNeeds to be backported to D6.
Comment #29
DamienMcKennaComment #30
alexmoreno CreditAttribution: alexmoreno commentedunfortunately, it cannot be done as Drupal 6 add_js and drupal_add_css does not support using a weight.
Comment #31
DamienMcKennaComment #32
DamienMcKennaThe D6 version of this module is no longer supported, so I'm closing this issue.