Hi,

Seems within the uc_checkout_pane_cart() function the table for the checkout page runs through a theme function (great!) but the table for the review page does not (boooo!) ...

I'm happy to supply a patch to make this happen, if the patch will be applied. Is the Drupal 5.x version of Ubercart still likely to have releases? If so, let me know and I'll make a patch and stick it in here. It's pretty trivial. =)

CommentFileSizeAuthor
#9 itemtheme.patch3.23 KBarski
#5 itemtheme.patch2.57 KBarski
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Yeah, I actually will be rolling out a 1.8 to coincide with the 2.0 release... getting a patch into 2.x would be ideal, though we probably won't review / include it till 2.1.

crashtest_’s picture

Greg, I would appreciate the patch even if it doesn't get applied immediately. Still willing?

Thanks,

Pat

greg.harvey’s picture

Hi Pat - I didn't actually make the patch in the end, because thill_ from Commerce Guys put me on to a module where you can disable the whole review page. He even felt the review page sometimes harmed conversion rates, looking at stats from some of their clients.

This is what I used: http://www.ubercart.org/contrib/497

arski’s picture

Hi guys,

I just ran into the same problem - It would be really great if the output of uc_checkout_pane_cart() were themed so that it could be customized along with the rest of the review table.

PS. I'm running the 6x version of the module - don't need to open an extra issue for that do I?

Thanks a lot,
Martin

arski’s picture

Version: 5.x-1.7 » 6.x-2.2
FileSize
2.57 KB

Submitting a patch for the 6.x version. Let me know if anything is wrong with it, it's my first patch :)

greg.harvey’s picture

Version: 6.x-2.2 » 6.x-2.x-dev

I think that's a great start. However, my only comment would be since this is a D6 patch a template would be even cooler than a theme function! =)

Patches should be against dev, btw, as that's where they'll be committed.

arski’s picture

Yea, you're probably right about a template.. But that would mean updating all theme functions to templates which might be something for a later day :)

TR’s picture

Status: Active » Needs work

Instead of just passing $item->qty you should pass the whole $item, that way the theme function will have access to all the additional information about the product.

One thing I don't like about this patch is that it doesn't give the end user a way to add a column to the table, which is probably the most frequent thing people ask for. I think that will require moving more of the functionality into the theme function and not just confining the theme function to outputting one row of HTML.

And yes, I would appreciate if you made your patch against the -dev version - that requires you to download the current -dev and install it on your site. Although if you made the patch against the 6.x-2.2 version then that is the one that should be assigned to this issue.

arski’s picture

FileSize
3.23 KB

Hi,

You're absolutely right about making it more general. Even passing the whole $item would leave some of the output outside the theme function (the

tags), so I basically moved the whole table to the theme function which now receives an array of $items as the only argument.

And I updated my code to the latest -dev version before doing this so that should be right now.

Let me know what you think.

Thanks,
Martin

arski’s picture

Status: Needs work » Needs review

forgot to set it to needs review. thanks for checking it out!

TR’s picture

Issue tags: +Ubercart theme layer

Tagging.

longwave’s picture

Status: Needs review » Fixed

Committed to both branches.

http://drupalcode.org/project/ubercart.git/commit/d0a08c8
http://drupalcode.org/project/ubercart.git/commit/df58507

The theme function output could be improved, but at least there is a theme function now.

Status: Fixed » Closed (fixed)
Issue tags: -Ubercart theme layer

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