I have a payment form that fails to work in my custom theme and I finally tracked down my problem - I recommend that we change line 43 of "uc_payment_checkout_pane.inc" from this:
drupal_add_js("\$(document).ready( function () { show_default_payment_details('". base_path() ."?q=cart/checkout/payment_details/'); } );", 'inline');to this:
drupal_add_js("\$(document).ready( function () { get_payment_details('". base_path() ."?q=cart/checkout/payment_details/". key($options) ."'); } );", 'inline');This will essentially remove the need for the show_default_payment_details function altogether, by running the same call using the first payment option keyed by line 31.
Here is why:
The show_default_payment_details function is using a Jquery selector that only works with forms that have been themed with the default form classes. This essentially ties the functionality to the theme layer, and caused me quite a bit of havoc when using my own custom theme elements that do not have classes of "form-radio" wrapped around radio inputs.
While it is possible to change the jquery selector to use "$('#payment-pane input[@type=radio]').each(" instead of "$('#payment-pane .form-radios .form-radio').each("
I recommend less code and the removal of the show_default_payment_details function altogether, and hopefully open the door to a possible future solution of showing the first payment option form without needing javascript to do it.
Thanks!
| Attachment | Size |
|---|---|
| uc_payment_checkout_pane.diff | 723 bytes |
Re: Bug in uc_payment_checkout_pane.inc and Its use of JS from u
Attempting this fix didn't quite work for me... partially because it doesn't take into account that we don't always want a selection. For example, on the first pageload, there should be no selection. On subsequent pageloads, going to review and coming back for example, we want the previous selection which may not have been the first option. That's why the #default_value in the element sets the default to be the first only if there's only one option... otherwise it attempts to use the previous selection.
I think maybe altering the JS would be the way to go... although I'm a little curious why you've changed the classes on the radio buttons in a theme func instead of just removing/altering the CSS rules for those elements.
Hey Ryan - I probably
Hey Ryan -
I probably submitted that too quick without looking at all the use cases - Here is a better patch.
I think it is useful to have the default one automatically load for the user, this should take that into account - as far as my theme, We have our own custom theme/engine we affectionately call the "naked" theme - and we've removed a lot of classes from elements that in our humble opinion are unneccessarily verbose. The goal is not to change the styles, but to reduce the markup.
Thanks for looking at this - I will do whatever I can to get this acceppted...
Otherwise my handoff to a client this week is a patched ubercart.... 
Re: Hey Ryan - I probably
Hmm... that patch looks essentially like what I did when testing this earlier, and it caused an error when there was no default and there were more than one payment methods. Perhaps the drupal_add_js() needs to be wrapped in an if to not add that Javascript if there is no default?
Re: Re: Hey Ryan - I probably
Hi Ryan - I also encountered that - but the conditional check on line 34 should take that into account, If there is no default arg passed in, it will default to the first - Am I missing something below -
$default = (count($options) == 1 || !isset($arg1->payment_method)) ? key($options) : $arg1->payment_method;Thanks
Re: Re: Re: Hey Ryan -
Well, we actually don't want the first option to be selected when there isn't a default. In other words, I would take out the !isset in your inline if there and add something like this...
<?phpif (empty($default)) {
drupal_add_js("\$(document) ...
}
?>
Will try to test soon... on a roll with recurring payment stuff I think.
Re: Re: Re: Re: Hey Ryan -
Hi Ryan - I certainly don't want to take away from your work on recurring payments - But from a user perspective, I believe the first option should be selected by default the first time you come to the form. I think we should save the user "a click" here - and if they want to change it, they can click the radio button.
An admin can then set the weight of a payment option to give the user their preffered option first.
Re: Re: Re: Re: Re: Hey Ryan -
Hey rad, I agree. This will also allow us to display the default payment method details box on the page load. I saw a custom job that did this showing CC details first and liked the way it worked. I'll look into committing the change today!
Re: Re: Re: Re: Re: Re: Hey Ryan -
Haha, that's the first time anyone has used the first syllable as my name - I kind of like it.
Thanks for all your work on ubercart - it was ridiculously easy for me to add that cybersource SOAP gateway, whereas in most ecommerce frameworks I've used, everything is so hard-coded that it is kind of a kludge to change anything. It is nice to be able to hook into some things.
As a side note - I ran into an issue while implementing my custom cybersource subscription pricing setup, and I know you are working on the uc_recurring module. I made a separate post here http://www.ubercart.org/issue/2798/some_notes_implementing_cybersource_s...
Thanks!