So I fired this up and Snap. Awesome forms. Except one.
Theres a select in the payment details section of Ubercart that are not getting the divs wrapped around them, and thus no styling.
Any hints or tips on what I should look for regarding why that might be so, such that I can submit a patch to the UC queue? Or is this something that can be modified for uniform?
The function that is getting called does the following..
/**
* Display the payment details when a payment method radio button is clicked.
*/
function get_payment_details(path) {
var progress = new Drupal.progressBar('paymentProgress');
progress.setProgress(-1, '');
$('#payment_details').empty().append(progress.element).removeClass('display-none');
// Get the timestamp for the current update.
var this_update = new Date();
// Set the global timestamp for the update.
payment_update = this_update.getTime();
var data;
if ($('#edit-payment-details-data').length) {
data = { 'payment-details-data' : $('#edit-payment-details-data').val() };
}
else {
data = {};
}
// Make the post to get the details for the chosen payment method.
$.post(path, data,
function(details) {
if (this_update.getTime() == payment_update) {
// If the response was empty, throw up the default message.
if (details == '') {
$('#payment_details').empty().html(Drupal.settings.defPaymentMsg);
}
// Otherwise display the returned details.
else {
$('#payment_details').empty().append(details);
//I hacked this in just to make it work.. but... whats the right way?
$("select, input:checkbox, input:radio, input:file").uniform();
}
}
// If on the order edit screen, clear out the order save hold.
if (window.remove_order_save_hold) {
remove_order_save_hold();
}
}
);
}
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | Ubercart_attachbehaviors-949172-17-2.patch | 534 bytes | afox |
| #20 | Ubercart_attachbehaviors-949172-17.patch | 451 bytes | afox |
| #10 | 949172-take2-uniform.js_.patch | 849 bytes | Mixologic |
| #8 | 949172-uniform.js_.patch | 702 bytes | Mixologic |
Comments
Comment #1
MixologicEr whoops.. shoulda hacked in a more specific selector:
Comment #2
realityloop commentedUniform styles content at a point in time, if other form element are redrawn after initial uniforming then they won't have uniform style applied.
What UC needs to do is have a jQuery hook triggered after a form element is redrawn so I can trigger a uniform re-styling, this sounds to be same issue that we're facing with Hierarchical Select, you could look at the HS API.txt for an example of their trigger.
The code that should fix this issue in HS, but isn't currently working, is:
$('.hierarchical-select-wrapper').bind('change-hierarchical-select', function() { $("select").uniform(); });I imagine that over time there will need to be several pieces of JS code in Uniform to deal with these other modules that redraw any form elements.
Comment #3
MixologicHmm.. actually it sounds like there's a need for a centralized function in drupal core that all dom changes (should)pass through, with hooks to allow other modules to react to those changes. So instead of $('#payment_details').empty().append(details); we would have something like Drupal.changeDom($('#payment_details').empty().append(details)); which would change the dom, and then fire any hooks. The best way to know if javascript is changing the dom is to track the changes in a central location. Of course not all modules would adhere to that, just like module after module still does $.(ready) instead of Drupal.behaviors.namespace =.
Comment #4
MixologicThe other option is to encourage the developer of Uniform to use jquery's .live() method instead of .bind(), if at all possible. I opened a bug in his issue queue to find out if this is a viable option. .live() watches the DOM for changes and catches new selectors as they are generated (like input:select etc)..
Report here: http://github.com/pixelmatrix/uniform/issues#issue/94
Comment #5
realityloop commentedcool lets see if it gets a response
Comment #6
tr commentedRe: #3
Drupal.attachBehaviors() could and probably should be used in Ubercart to allow behaviors to be attached to the new elements when the DOM is modified. Try it out and if it works submit a patch to the Ubercart queue.
Comment #7
realityloop commentedMixologic: as I'm not using Ubercart, are you able to try TR's suggestion?
Comment #8
MixologicYes, I've been digging into this a fair bit, and one issue that Im running into is that Uniform is choking when it calls .uniform on a context that has already been 'Uniformed'. For example, I have a bit of javascript that conditionally attaches and detaches a part of the DOM when its checked (attaches)/unchecked (detaches). If I check and uncheck and recheck the box, Drupal.attachBehaviors(mystuff) is called by uniform more than once on this particular context, which breaks the select boxes. Another example is in ubercart when the 'shipping quotes' pop up.. I believe this actually uses the Drupal.attachBehaviors(context), but calls it for each shipping method, which ends up with weird stacked radio buttons.
According to the comments in drupal.js:
* Behaviors should use a class in the form behaviorName-processed to ensure
* the behavior is attached only once to a given element. (Doing so enables
* the reprocessing of given elements, which may be needed on occasion despite
* the ability to limit behavior attachment to a particular element.)
So I threw together a quick patch to add that in and voila! lots of stuff stopped acting funny..
Comment #9
MixologicStrange though regarding Ubercart.. I added in the Drupal.attachBehaviors(details), and it doesnt work. I stepped through with the debugger, and Uniform is getting called on the details context, but it doesnt seem to affect it any. only when I call uniform after its added and specifically target the #payment_details selectors like I am does it have any affect.
I thought it might have something to do with the $(document).ready( in uc_payment, but when I switched that over to a Drupal.behaviors had even uglier results (it obviously was not intended to be js that is executed on every behavior attach call).
In any case using Drupal.attachBehaviors doesnt work for this, even though it should. I'll try using the unminified uniform js and see if I can get it sorted out.
Comment #10
MixologicAnd ignore my previous patch. I forgot about the else.
Attached is mo proper.
Comment #11
tr commentedJust to be clear ... am I right in saying the patch in #10 should allow Uniform to properly deal with dynamically changing DOMs iff the DOM change is registered using Drupal.attachBehaviors() ? And that it should not change how Uniform works for a static DOM? And that this patch is necessary but not sufficient to let you use Uniform with Ubercart - some Ubercart jQuery patches will still be needed?
Comment #12
realityloop commentedMixologic: #10 committed, thanks. I've left the issue open as it sounds like this hasn't resolved the Ubercart issue?
Comment #13
Mixologic@TR yes, the patch in #10 allows uniform.js to react to attachBehaviors, and not reattach behaviors to something it has already processed, however yes, Drupal.attachBehaviors wasnt working for me in payment.js, but it was primarily due to me not fully grokking 'context' in jquery.
I orginally had this, and it works, but it was a hack:
I tried this, but it didnt work:
But when i had an ahah moment (no pun intended):
It works great, and it all makes sense to me know. Now that I know that the context is telling Jquery *where to look in the dom* and not *what to look at* it all gels much more for me now.
So looking in payment.js there are a *lot* of places where the DOM gets manipulated. Obviously we dont want to call attachBehaviors after every line item is inserted, but the attachBehaviors should probably be used throughout..
Comment #14
realityloop commentedRequired changes made to uniform, moving this to Ubercart queue
Comment #15
tr commentedComment #16
longwaveNot sure this is worth doing now in 6.x as it's late in the development cycle and may break contrib modules and existing sites that rely on the current behaviour (no pun intended); comments welcome either way. This is already fixed in 7.x.
Comment #17
afox commentedI just faced the same need for Drupal.attachBehaviors(). I had written a payment method which has modalframe supported popup info-windows, which don't work in the checkout page.
I'm not sure how it would break the current behavior if Drupal.attachBehaviors(); is added into get_payment_details() -function. Correct me if I'm wrong, but doesn't Drupal.attachBehaviors just re-attach the jQuery to the AJAX-loaded HTML? If this affects other contrib module behavior, shouldn't it be the other modules' responsibility because this is the Drupal way to to re-attach jQuery into AJAX-loaded content?
I would consider this more of a bug, because allowing jQuery use is expected in this scenario.
Comment #19
longwaveThere is no patch here to review.
Comment #20
afox commentedSorry about that. I always forget that the "needs review" is for patches. But as I hacked my version of the code, I did create a patch for you. ;)
Comment #21
longwaveYou need to pass only the part of the DOM that was changed to Drupal.attachBehaviors(), this call will (re)attach behaviors across the entire page every time the payment method is changed.
Comment #22
afox commentedOk, here's a new one, targeting only #payment-details.
Comment #23
longwaveNot worth changing this now and risking breaking checkout on existing sites in 6.x.