Closed (fixed)
Project:
Ubercart
Version:
7.x-3.x-dev
Component:
User Interface
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
19 Dec 2011 at 23:35 UTC
Updated:
12 Jan 2012 at 16:40 UTC
Jump to comment: Most recent file
Comments
Comment #1
longwaveThis patch uses the Unicode × character instead of an HTML entity, and changes two more uses for consistency.
Comment #2
wodenx commentedI was getting a line break in the qty column of the cart block (Bartik/Chrome). This patch wraps the qty in a
<span class="uc-qty">to enforce no wrapping (and removes the deprecated 'nowrap' attribute on the <td> tags). Also changes the '#markup' tags in the order view tables to use'#theme' => 'uc_qty'. Otherwise, seems to work well.Comment #3
longwaveIf the output is going to be wrapped in <span> I think the uc_paypal change needs to be reverted, as that expects plaintext output. That one is minor and it's arguable whether it should be themable or not anyway.
Comment #4
longwaveActually could we just apply the class to the <td> tags directly and/or use #prefix/#suffix to apply a span where needed?
Comment #5
longwave..or even simpler, just make the theme function use instead of a plain space.
Comment #6
wodenx commentedWell - I thought about applying to the td tags, and that would be fine, but would need to do it in several places instead of just one. is probably better than the span - but wouldn't we still have to revert the paypal change if we're using an html entity? And mightn't it be helpful to some themers to be able to control the appearance of quantities with CSS (as they can now with prices)? I don't have a strong opinion about it - just let me know what you think and I'll make the change.
Comment #7
longwaveI think a class on the <td> tag is better than a <span> inside the tag; less markup, and the <td> covers the entire table cell, the <span> covers just the inline text unless you change it to a block element, by which point you may as well be classing the <td>. As long as all our quantity-related tags have the same parent class it should be fine.
Comment #8
longwaveThe attached patch keeps your #markup to #theme change, but reverts the PayPal change and adds CSS to fix the cart block. Fixing the nowrap attributes is outside the scope of what we're doing here, and there's already an open issue for it.
Comment #9
longwaveCommitted #8.