I have been working on one of my modules for Ubercart and found myself learning the new TAPIr functionality and found some Ubercart utilizing this new functionality rather inefficiently. I want to say before delving into my example, I have not looked throughout Ubercart's codebase to check for all instances where TAPIr is being used and I do not know if my proposed method will work well for all cases, I have been primarily working with altering the uc_cart_view_table.
Currently, with the latest Bazaar snapshot, the uc_cart_view_table function that builds the TAPIr table gets called in the theme function of uc_cart_view_form. The problem with this is that now one form is being generated by two separate forms, but this in itself is not the main issue. When the theme for uc_cart_view_form is called, it has to mark all the cart items as been printed without having printing them itself, the actual items are being printed inside of the uc_cart_view_table function. An issue brought up by this is when altering a table for the inclusion of another cell in a row. Each module which wants to add cells to the table will find themselves having to alter both the uc_cart_view_form and uc_cart_view_table functions. The uc_cart_view_table will need to be altered to include their new cell into it, grabbing the items array from the #parameters of the form array and unsetting each #printed element inside of the item they will be rendering out. The uc_cart_view_form will need to be altered to mark each new element inside of the item as been printed so the elements don't get rendered twice, once in uc_cart_view_table and once in uc_cart_view_form.
To solve this for uc_cart_view_form and any other occurrences we should only need to add the uc_cart_view_table form into the uc_cart_view_form by as simply as:
<?php
$form = array_merge($form, uc_cart_view_table($form_state, $items));
?>
Of course there are other ways of adding the table form to the cart form.
I would like to hear any thoughts, comments, or questions on this.
Comments
Comment #1
rszrama commentedThanks for bringing this up. I noticed that our tables were never actually migrated to the new system either, and so I did a little work but not all the necessary work to fix the situation. You're right on. The theme function for the cart form shouldn't have to call tapir_get_table() at all. The form should simply specify its theme function to 'tapir_table' and let TAPIr automatically stick all the data in a table. I'll see if I can schedule some time to do a better update, and please let me know if you see other instances of this. I'll maybe update the docs to reflect the fact that form theme functions shouldn't have to do anything if they simply specify the theme function for the proper element in the form array.
Comment #2
greenskin commentedLet me know when this gets implemented. I'm going to hold off updating my module more until this is fixed.
Comment #3
Island Usurper commentedHere you go: #334656-7: Subtotal footer in cart doesn't update when columns are added (hard-coded colspan).
I posted more information here as well: #334069-8: Can't alter form uc_product_table.