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.

CommentFileSizeAuthor
#17 drupal.form-options.17.patch10.83 KBsun
#16 drupal.form-options.16.patch10.93 KBsun
#13 drupal.form-options.13.patch10.36 KBsun
#11 drupal.form-options.11.patch10.36 KBsun
#9 drupal.form-options.9.patch9.51 KBsun
#4 drupal.form-options.4.patch4.53 KBsun
#2 drupal.form-options.2.patch2.97 KBsun
drupal.form-options.0.patch2.88 KBsun

Comments

Status: Needs review » Needs work

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
StatusFileSize
new drupal.form-options.2.patch2.97 KB

Stupid me. ;)

moshe weitzman’s picture

moshe weitzman commented

Seems reasonable. Could you elaborate on a use case?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.form-options.4.patch4.53 KB

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

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

sun
Primary language German
Location Karlsruhe
commented

Any feedback?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

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

sun
Primary language German
Location Karlsruhe
commented

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.form-options.9.patch9.51 KB

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

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
StatusFileSize
new drupal.form-options.11.patch10.36 KB

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.form-options.13.patch10.36 KB

Added one more decimal to the default weights.

effulgentsia’s picture

effulgentsia commented
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

sun
Primary language German
Location Karlsruhe
commented

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.form-options.16.patch10.93 KB

Re-rolled against HEAD.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.form-options.17.patch10.83 KB

Re-rolled against HEAD.

dries’s picture

dries commented
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.

Issue tags: +DX (Developer Experience)

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.