search

demianturner / sgl-docs-tickets-migration-test

0 stars 0 forks source link

improvement to Output::generateCheckbox #1292

Open demianturner opened 11 years ago

demianturner commented 11 years ago

What do you think about the supplied patch to Output::generateCheckbox(). I would like it to be able to handle a caption other than the value, since value is usually some numeric value from a database or whatever, and I tend to want another caption on those. :)

As for the !empty check on the options, its needed to be able to get by the check since i cant seem to send a null value from flexy:

{generateCheckbox(#search[property][]#,property.property_id,property.checked,##,property.property_name):h}

The ugly part of this patch is that it doesnt keep the $options argument last, but that could be changed, though it would need more code changes. If you like it, I can send a patch for those, but it would break BC for code depending on this behaviour.

demianturner commented 11 years ago

[ericpersson] Some comments from the mailinglist can be found at http://groups.google.com/group/seagull_general/browse_thread/thread/00af32c20dfd7b70/cb6366ed0aeb45fd?hl=en#cb6366ed0aeb45fd

demianturner commented 11 years ago

[demian] Hi Eric

I agree that it's an improvement to let the input tag's caption be different from the value, but I don't think it's worth breaking the convention we've established where the $options argument is last.

Perhaps the best compromise is to make the 2nd arg optionally an array, 1st elem value, 2nd elem caption, the test for an array in the method body, that way BC could be preserved as could method prototype.

demianturner commented 11 years ago

[demian] Eric - wakey wakey, can you fix this so we can get a release out or at least close the ticket if you don't have time to work on it. A response to my email would be nice too.

demianturner commented 11 years ago

[ericpersson] "Perhaps the best compromise is to make the 2nd arg optionally an array, 1st elem value, 2nd elem caption"

Yeah, perhaps, but that wouldnt let us do it all in the flexy template? I'm not sure, but I think this wouldnt work: {generateCheckbox(#my_checkbox#,array(checkbox.value,checkbox.caption),checkbox.checked):h}

demianturner commented 11 years ago

[demian] that's a good point, didn't think of that. in this case i'd suggest we leave it for 0.7, break the API, and just order the args better.

demianturner commented 11 years ago

[ericpersson] Sounds like a plan! I agree.

demianturner commented 11 years ago

[demian] Milestone 0.7.0 deleted