Comments

gavri’s picture

I just want to add that this is very helpful alter hook.

it allows you to change the callback of the pane - and add new fields:
on each $op, at the the beginning, you can call the old callback - meaning you cab add more fields to existing panes.

longwave’s picture

Can't you add extra fields using hook_form_alter? The disadvantage of changing the callback is that only one module can do this - what if two different modules want to alter the same checkout pane?

amitaibu’s picture

@longwave,

I think this is a valid concern, but:

1) In D7 every hook_foo() has hook_foo_alter() -- and It's good to follow core patterns.
2) IMO the problem is bigger as the panes design is currently flawed, and hard to extend. I think panes should be more conacted to the FAPI system, truelly allowing modules to extend it via hook_form_alter(). So the callback should be something like this:


/**
 * Implemen hook_cart_checkout_pane().
 */
function foo_cart_checkout_pane() {

  $panes = array();
  $panes['foo_pane_callback'] = array(
     ...
     'callback' => 'foo_pane_callback',
  );
  return $panes;
}

/**
 * Pane callback form builder.
 */
function foo_pane_callback() {
  $form['pane_bar'] = array(
    '#type' => 'textfield',
    '#title' => t('Bar'),
   ...
   // We are using FAPI to declare validate and submit handlers.
   '#validate' => 'foo_pane_validate',
   '#submit' => 'foo_pane_submit',
   // Add private keys to declare callbacks for things like the review or process .
   // Any module can add it's own callback or remove existing ones via hook_form_alter().
   '#review' => 'foo_pane_review',

  );

}

rszrama’s picture

I actually have a reworked pane system proposed in the forums at Ubercart.org... don't have time this second to dig it up, but it is indeed much more closely tied to FAPI.

amitaibu’s picture

@rszrama,

Cool, would be interesting to see the code (I could only find http://www.ubercart.org/comment/40965/Re_Alter_checkout_panes)
Anyway, the proposed patch is still needed with the current system, to give it a bit more flexibility.

Anonymous’s picture

Will this patch be wrapped into the next release of Ubercart (6.x-2.1)? The reworked pane system sounds great, but that won't be available until Ubercart 3, correct?

Thanks!

longwave’s picture

Bumping this, as when looking at extending Ubercart Marketplace, I've found it would be useful to be able to alter order panes through a similar drupal_alter() call in _order_pane_list().

guypaddock’s picture

+1 for this feature. I just found myself needing it as well.

@longwave: Technically, there is a way to write a module so that calls are chained, if necessary, to allow multiple modules to alter the same pane but not step on each other's toes, but it really depends on why the previous pane definition is being altered.

In my case, I just want to add some extra form elements to the shipping quote pane, but without this patch, I have to work on the entire cart and checkout forms instead of just the panes.

guypaddock’s picture

StatusFileSize
new2.26 KB

The attached patch combines the original patch with an additional patch to add an alter callback for hook_cart_pane() as well (i.e. hook_cart_pane_alter()).

guypaddock’s picture

Scratch the patch in my previous comment (#9); I didn't realize that cart panes don't use callbacks (they contain the form body).

The attached patch handles this correctly by passing along the $items array to the alter callback. It also has a correct example for how to use the alter hook.

guypaddock’s picture

New re-roll with more correct file paths. GNU patch seemed to handle the old one okay, but some patch clients don't.

neilnz’s picture

Status: Needs review » Reviewed & tested by the community

Applied your patch from #11 in order to make all checkout panes non-collapsible. Implemented the alter hook and successfully made all panes non-collapsible.

RTBC in my opinion.

Thanks

Island Usurper’s picture

Status: Reviewed & tested by the community » Fixed

Sounds good to me.

Committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

guypaddock’s picture

@Island Usurper: It looks like this was included in UC 2.4; am I correct? The issue number wasn't added to the release notes.