Once in a life-time, the default #options handling for #type checkboxes and radios is not sufficient. If you have to define one or more option as sub-element manually, you need to start to study Form API internals to get it right.

It doesn't have to be that hard.

Files: 
CommentFileSizeAuthor
#17 drupal.form-options.17.patch10.83 KBsun
PASSED: [[SimpleTest]]: [MySQL] 27,004 pass(es).
[ View ]
#16 drupal.form-options.16.patch10.93 KBsun
PASSED: [[SimpleTest]]: [MySQL] 26,763 pass(es).
[ View ]
#13 drupal.form-options.13.patch10.36 KBsun
PASSED: [[SimpleTest]]: [MySQL] 26,693 pass(es).
[ View ]
#11 drupal.form-options.11.patch10.36 KBsun
PASSED: [[SimpleTest]]: [MySQL] 26,128 pass(es).
[ View ]
#9 drupal.form-options.9.patch9.51 KBsun
FAILED: [[SimpleTest]]: [MySQL] 26,106 pass(es), 2 fail(s), and 0 exception(es).
[ View ]
#4 drupal.form-options.4.patch4.53 KBsun
PASSED: [[SimpleTest]]: [MySQL] 26,020 pass(es).
[ View ]
#2 drupal.form-options.2.patch2.97 KBsun
PASSED: [[SimpleTest]]: [MySQL] 24,821 pass(es).
[ View ]
drupal.form-options.0.patch2.88 KBsun
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, drupal.form-options.0.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new2.97 KB
PASSED: [[SimpleTest]]: [MySQL] 24,821 pass(es).
[ View ]

Stupid me. ;)

moshe weitzman’s picture

Seems reasonable. Could you elaborate on a use case?

sun’s picture

StatusFileSize
new4.53 KB
PASSED: [[SimpleTest]]: [MySQL] 26,020 pass(es).
[ View ]

Thankfully, we have a use-case (and therefore also test case) in core :)

sun’s picture

As visible in the patch, this really simplifies setting custom properties for individual radios/checkboxes, as you only have to care for your custom properties, and nothing else.

sun’s picture

Any feedback?

sun’s picture

Each time someone tries to add descriptions to radio buttons or checkboxes, he's running into this dilemma. The patch is straightforward, though I can happily add separate tests if absolutely required.

sun’s picture

#4: drupal.form-options.4.patch queued for re-testing.

sun’s picture

StatusFileSize
new9.51 KB
FAILED: [[SimpleTest]]: [MySQL] 26,106 pass(es), 2 fail(s), and 0 exception(es).
[ View ]

Actually, this functionality is completely untested currently. Therefore, added tests.

Status:Needs review» Needs work

The last submitted patch, drupal.form-options.9.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new10.36 KB
PASSED: [[SimpleTest]]: [MySQL] 26,128 pass(es).
[ View ]

oh wow -- this clearly shows that we're missing detailed tests for expanded #options. Fixed those test failures.

sun’s picture

Any feedback? This patch comes with tests now and looks RTBC to me.

sun’s picture

StatusFileSize
new10.36 KB
PASSED: [[SimpleTest]]: [MySQL] 26,693 pass(es).
[ View ]

Added one more decimal to the default weights.

effulgentsia’s picture

Status:Needs review» Reviewed & tested by the community

This really makes a lot of sense. I can't justify escalating it to "major", so it might languish in the queue, and then be denied, but that would be a shame, because it improves DX with no downside that I can see. The minus signs in comment.module are lovely, just lovely.

+++ includes/form.inc 9 Oct 2010 16:55:17 -0000
@@ -2824,18 +2830,23 @@ function form_process_checkboxes($elemen
-          '#processed' => TRUE,

This was the one line I was concerned with, since it potentially introduces a BC break. But I looked more into it and it doesn't, because form_builder() starts off forcing #processed to FALSE anyway, so this was a pointless line to begin with.

Powered by Dreditor.

sun’s picture

#13: drupal.form-options.13.patch queued for re-testing.

sun’s picture

StatusFileSize
new10.93 KB
PASSED: [[SimpleTest]]: [MySQL] 26,763 pass(es).
[ View ]

Re-rolled against HEAD.

sun’s picture

StatusFileSize
new10.83 KB
PASSED: [[SimpleTest]]: [MySQL] 27,004 pass(es).
[ View ]

Re-rolled against HEAD.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Status:Fixed» Closed (fixed)
Issue tags:-DX (Developer Experience)

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