Add support for "advanced" checkout panes to not use fieldsets [#971768]

Hi y'all,

I've got a use case where the use of fieldsets in the checkout pane will cause all sorts of havoc. This will happen, for example, when a node form is embedded into a checkout pane (I've ported this module to D6 Ubercart 2.x). It's quite a simple modification yet will allow maximum flexibility. Patch to follow (yet to roll it up).

Comment	File	Size	Author
#1	971768_1.patch	2.13 KB	sammys

Comments

Comment #1

sammys commented 15 November 2010 at 02:19

Status:

Active

» Needs review

Status	File	Size
new	971768_1.patch	2.13 KB

Here is the patch.

Comment #5

rszrama commented 15 November 2010 at 13:58

If you do this, it seems you run the risk of variable name collision (though I'm guessing people would only use this if they knew what they were doing ; ). Would it hurt to include a container element (currently undocumented) with the name of the "advanced" checkout pane to preserve the #tree?

I suppose since we do include options for collapsibility of a checkout pane, we can easily add an option there to toggle this overall fieldset usage. At the same time I wonder if there should be a general checkout setting to use the fieldset for checkout buttons. This is kinda on the line of core functionality vs. contrib functionality, since it can be easily edited using hook_form_alter(), but seeing as how we have options at the moment for per-pane fieldset configuration I don't see why not to support it. Maybe some day we just move all this to contrib?

On a side note... good to hear from ya! Wish I was rockin' out with you at that Metallica show. ; )

Comment #6

sammys commented 19 November 2010 at 00:27

Hi Ryan! Wish you were at the show as well! Was fantastic!

I've since ported the rest of the panes for cart contents, billing, delivery, quotes, payment and comment. It's now mostly working on Ubercart 2.x! :) Order saving pieces need a bit more work though. I have some uc_node_checkout related cart steps as checkout pages before the checkout actually starts. Am also yet to play around with the access stuff.

While doing all that I realised you were calling callbacks directly. Thanks to your foresight one can simply make the $form parameter of the _checkout_form callback a reference. Voila!! You can do what you want. I think this allows enough flexibility for anyone wanting to break convention.

In some cases having everything inside a #tree isn't great. E.g. if you're pushing a node form into a #tree bad stuff can happen in D6. Haven't tried that for D7 yet. Being able to skip using #tree is quite a bonus when JS from some third-party module "just works" (tm). I also agree with you that the key collision is not a big concern as these use-cases are remote and for nut cases.

I think the fieldset switch for buttons might be rather trivial and I'd prefer it in core since a contrib module to do that would be unnecessary bloat. Well I guess if someone makes a commerce_tweaks module it's not so bad hehe.

Not sure what to do with this issue... Reuse it as a task to add a fieldset switch for buttons or switch it to works as designed?

Good to hear from you Ryan! Nice job on commerce and the checkout module is very sweet and a huge improvement over adv_checkout... love the redirects. :)

Comment #7

rszrama commented 23 February 2011 at 07:22

Status:

Needs review

» Needs work

Just updating this to say that I did add support for not using a fieldset element, though as I mentioned above it still uses #tree => TRUE. I need to revisit the issue in its entirety to see what we can do about bypassing the #tree and not including the checkout buttons fieldset.

Also, dunno if you saw or if I had developed it yet, but I'm doing exactly what you describe with the redirected payment method pane - it receives the form array by reference and then jacks it all up to turn the whole form into a submit form to the off-site payment service. Obviously only really makes sense if it's the only pane on the page. ; )

Here's the current commit: https://github.com/rszrama/drupalcommerce/commit/3216263412fc20c274dd8fe...

Marking needs work to see what we can do about addressing the buttons fieldset.