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();
      }
    }
  );
}

Comments

Mixologic’s picture

Er whoops.. shoulda hacked in a more specific selector:

          $("#payment_details select, #payment_details input:checkbox, #payment_details input:radio, #payment_details input:file").uniform();
realityloop’s picture

Uniform 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.

Mixologic’s picture

Hmm.. 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 =.

Mixologic’s picture

The 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

realityloop’s picture

cool lets see if it gets a response

tr’s picture

Re: #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.

realityloop’s picture

Mixologic: as I'm not using Ubercart, are you able to try TR's suggestion?

Mixologic’s picture

Version: 6.x-1.4 » 6.x-1.7
Status: Active » Needs review
StatusFileSize
new702 bytes

Yes, 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..

Mixologic’s picture

Strange 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.

Mixologic’s picture

StatusFileSize
new849 bytes

And ignore my previous patch. I forgot about the else.

Attached is mo proper.

tr’s picture

Just 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?

realityloop’s picture

Mixologic: #10 committed, thanks. I've left the issue open as it sounds like this hasn't resolved the Ubercart issue?

Mixologic’s picture

@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:

          $('#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();

I tried this, but it didnt work:

          $('#payment_details').empty().append(details);
          Drupal.attachBehaviors(details);

But when i had an ahah moment (no pun intended):

          $('#payment_details').empty().append(details);
          Drupal.attachBehaviors('#payment_details');

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..

realityloop’s picture

Title: Ubercart/Uniform Challenges » Ubercart/Uniform Challenges add Drupal.attachBehaviors() support
Project: Uniform » Ubercart
Version: 6.x-1.7 » 6.x-2.x-dev

Required changes made to uniform, moving this to Ubercart queue

tr’s picture

Status: Needs review » Active
longwave’s picture

Title: Ubercart/Uniform Challenges add Drupal.attachBehaviors() support » Add Drupal.attachBehaviors() support
Category: support » task
Status: Active » Postponed (maintainer needs more info)

Not 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.

afox’s picture

Category: task » bug
Status: Postponed (maintainer needs more info) » Needs review

I 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.

Status: Needs review » Needs work

The last submitted patch, 949172-take2-uniform.js_.patch, failed testing.

longwave’s picture

Status: Needs work » Active

There is no patch here to review.

afox’s picture

Status: Active » Needs review
StatusFileSize
new451 bytes

Sorry 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. ;)

longwave’s picture

Status: Needs review » Needs work

You 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.

afox’s picture

Status: Needs work » Needs review
StatusFileSize
new534 bytes

Ok, here's a new one, targeting only #payment-details.

longwave’s picture

Status: Needs review » Closed (won't fix)

Not worth changing this now and risking breaking checkout on existing sites in 6.x.