Add support for checkbox attributes

Here is a first patch.

nice one. works for me.

Hi xibun,
Thanks for testing this out. Can you please tell me in what browser did you test it ?
I really didn't have time to test this in IE6/7/8 and if you have one of them please test this on that too.

Thanks!

Firefox 3.0.11 / Ubuntu 9.04

no IE here :)

:) no prob.
I have a virtual machine with windoze on it. when I'll have time I'll test this out.

Cheers

I had the big honor and privilege of staying in front of an IE6 today and tested this patch out.
It works ;)

Cheers

I observed something strange with the uc_vat module and attributes. now the source is located and disabling uc_aac fixed the problem I experience.

what is interesting with the issue is that it works as expected when displaying the attribute options as checkboxes. but it doesn't when displaying them as radio buttons or as a select box. therefore I believe something is not yet completely clean in uc_aac.

You can find more info in here.

Hi xibun,

With what version of uc_aac did can you replicate your problem ? Do you have the patch from this issue applied to your uc_aac ?

Also, I think you should open a new issue here and describe your problem in more detail (with some snapshots if possible).

Cheers !

yes, the patch is applied. I've tested it earlier on for you (see comment #2).

I've added it here as the checkbox attributes behave different from the other attributes.
also there is more then enough information over at the issue I provided the link for. (comments #5 - #14).

anyway, I might have time later this week to provide more information if it's still needed then.

cheers

Ohhh, right. Sorry about that :)
I'll try to look into this during this week if I have time.

Cheers !

I finally have some time for this...

two questions:
1. updating the price: currently the price get's updated on checkbox change & click! don't you think one of them would be enough? (lines 121 & 129)
-> what ever we do, the comment just above it should also reflect the code

2. what should we do with the price tag behind attribute options when displayed as checkboxes?
-> currently nothing is done with it. I don't understand what the code in lines 320 - 328 is supposed to do for checkboxes, since this is where updates would be implemented
-> one idea could be to change a "+€10.00" into a "(-€10.00 when removed)"

short update of recent findings...

in the website markup the checkbox shows two dimensions for the attributes (second dimension being equal to value). example:

but the radio buttons and selectbox have only one dimension. example:

if this is correct, then we need to treat checkboxes differently in uc_aac. the current code doesn't take this into account (lines 320 - 328).

following up on post #11 I have an unfinished patch:
1. -> now the checkbox prices are only updated on click.
2. -> started to implement the following: selecting a checkbox changes the price from "+€10.00" into "(-€10.00)"

BUT the price strings of 2. are not displayed. the on the fly changes work for radio buttons and select boxes but not for checkboxes.
(in my understanding they don't work in the original patch either)

Hi xibun,

Sorry for the late reply.

I had a quick look over your patch and I don't really see why it is not working. I remember I tried to accomplish the same thing when I wrote the first patch in this issue bug for some reason I just decided no to try more :)

I don't really have time to test your patch but for my case because of #498444: On price update, update only the labels I had to make uc_aac not update the whole HTML for checkboxes. When I do have some time I'll have a look over both issues.

Cheers

I've fixed a few things today:
- price adjustments for checkboxes are now displayed on the fly (see problem mentioned in post #13)
- the adjustments behave as described in post #13 (there was a bug in my last patch)

BUT there is one issue remaining: when no option of a checkbox attribute is selected the adjustments don't get updated. but this is necessary. example: unchecking the only checked option of a checkbox attribute should update the displayed option price adjustment. to solve the problem we need to POST the checkbox attributes independent of their state.

suggestions? very close to fixing this now - I hope to close it soon!

for some reasons had to re-roll the patch to make it apply (re-roll attached below). Price adjustment seems to work fine, except for a weirdness:

in a list of 6 checkboxes, from the 3rd checkbox and on it was summing the price of the option above, not the proper one... reordering options with weights from attributes/options administration fixed the problem.

many thanks for this, very needed!

@marcoBauli: I just tried to reproduce the problem you saw, but didn't manage. can you give step by step instructions to see the problem?

I've been able to identify the problem mentioned by marcoBauli. The problem also exists for radio buttons. It has to do with an inconsistency in Ubercart.
New issue opened there: #591160: Ordering of attribute options - default logic.

I don't mean to be a downer, but the module will probably start going down the road of #562100: New approach to uc_aac. It is a much cleaner implementation and adds some features. I haven't had time to look as deeply into it as I want before starting a new branch...but what I saw there looks very good and will likely be the future of this module.

No worries. I don't think fixing this is wasted energy:
1. we don't know when the new branch will happen - so fixing this will help us out here until then
2. it just helped to find a bug in Ubercart (see post #18), so now it might be fixed before final release of Ubercart 2.0 - and your future branch might also benefit from fixing that bug
3. finally when you create the future branch you might be able to re-use some code or ideas from this patch

Any progress on this issue ?

patch from #15 works for me with checkbox weirdness but good start.
Subscribing.

I have completed a port of the code mentioned above in comment #19 to Drupal 6.x, Ubercart 2.x. You can find the code at #562100: New approach to uc_aac attached to comment #2. Please download the code, try it out, and urge cYu to commit it as a base for a uc_aac 6.x-2.x branch.

Cheers,

Antoine

here a patch so v2 supports checkbox attributes.

is there a way to combine both ":radio" and ":checkbox" into the same jQuery .find() statement?

only noticed this one now: "warning: Illegal offset type in /opt/lampp/htdocs/drupal/sites/all/modules/uc_aac/uc_aac.module on line 59." -> more work needed...

is there a way to combine both ":radio" and ":checkbox" into the same jQuery .find() statement?

I think it should be possible like this: $(":radio, :checkbox").

Combining the different form types as suggested in #26 did not work for me, so I left the code as it was adding the checkbox form type.

A new patch is attached that fixes #25.

Cheers,

Antoine

Attached a patch that implements the combined jQuery find() statement (see #26).

What do you think about using brackets around the negative prices of selected checkboxes? (see #13 - 2.)
I guess it's a matter of taste.. or of what confuses customers less..
I can create the patch for it in case the brackets find agreement.

xibun,

Thanks for the combined jquery line. I like the idea of adding parentheses to negative values for a more noticeable change. If it becomes an issue down the road, we can make it optional or perhaps create an issue against ubercart for adding a currency formatting option for negative values. Patch attached.

Cheers,

Antoine

I did more extensive testing now.. in general it's working fine, but there is one small bug:

when the checkbox attribute contains an option with price 0.- and that option is not listed as first option, then it will reflect the price of the preceding option (instead of an empty string).

Just came across another problem - disappears when disabling the module:
When a product has, among others, a "text field" attribute (in my case with one option) then the product cannot be added to the cart anymore.
Error message: "An illegal choice has been detected. Please contact the site administrator."

Another patch fixing #30 and #31.

Cheers,

Antoine

subscribing

nice one! all my test cases work fine now. this is ready to be scrutinized by a wider audience.

thanks Antoine for making the new version of uc_aac happen.

Patch has been committed!

xibun, thanks so much for all your help testing.

Cheers,

Antoine