Missing label for boolean fields when adding content

This is the intended behavior. Single checkboxes do not use a label.

However, this has been continuously confusing users in the CCK era. Ideally, this behavior would be controlled by a widget setting.

Thus, turning into a feature request. Patches welcome :-).

subscribe

Glad to see this recognised as a feature request :-)

Hi there,

I had the same idea :)
Here's the Patch - it adds a instance setting to boolean fields which allows to define that the title has to be used as label.
The default behaviour of the field is not affected.
If the setting is available and activated it's triggered - otherwise everything remains unchanged.

Cheers,
Peter

Triggert automated testing

Shame on me - had just eyes for the new feature... :)

Didn't test yet, but this looks sweet.

'use_title_as_label' should be a widget setting rather than an instance setting, though. It applies only if you use the 'on / off checkbox' widget. See how text.module does with the 'size' setting for the 'text_textfield' widget.

Also, the name of the setting is inaccurate, fields don't have titles. It should rather be 'display_label' ?

+++ modules/field/modules/list/list.module	1 Sep 2010 10:23:00 -0000
@@ -387,3 +388,21 @@
+  ¶

whitespaces on empty lines (3 occurrences in the patch)

+++ modules/field/modules/options/options.module	1 Sep 2010 10:23:01 -0000
@@ -122,6 +122,10 @@
+      if(isset($instance['settings']['use_title_as_label'])&& $instance['settings']['use_title_as_label']){

invalid code style - mind the spaces after the 'if', around the '&&' and before the final { :
if ($a && $b) {

+ the isset() check is needless, Field API makes sure every setting comes present, with at least the default value. $instance['settings']['use_title_as_label'] will always be set.

Powered by Dreditor.

Hi yched,

Thanks for your feedback. If converted your suggestions into code ;)
SimpleTests included - but they should really be reviewed, it's my first simpleTestCase for Drupal...
Special thanks for your hint about the Coding Standards - I didn't notice that my validator was broken :|
If your're working with eclipse I could provide you the Drupal Coding Standards Validator as I've described it here: http://drupal.org/node/897116

Cheers,
Peter

PS: Hopefully the patch is now Coding Standard compliant otherwise I'll look like a fool :D

Some clean-up

From my #8 : 'use_title_as_label' should be a widget setting rather than an instance setting

You turned it into a 'field setting' instead. No good either :-). As I suggested above, you can look at how text.module does for the 'size' setting for the 'text_textfield' widget.

d'oh! Now I understand what you mean - at least I think so :D
Attached the next "php-fication"of my interpretation.
All the changes are moved into the options module, simpletests included.
Since my Drupal coding standard validator runs again, I'd some clean-up too.

+++ modules/field/modules/options/options.module	2 Sep 2010 16:11:32 -0000
@@ -25,7 +25,7 @@
-      'variables' => array('instance' => NULL, 'option' => NULL),
+      'variables' => array('instance' => NULL, 'option' => NULL, ),

Please revert that rule from your formatting rules - no comma after last element of inlined arrays.
The patch is full of those invalid + completely unrelated changes, which makes reviewing impossible :-(

At the end of the day, I don't think I'd advise automated drupal formatting, because the rules are hard to adjust, and even if you get them right you'll generate patches crufted with fixes for any existing errors in the files you touch, which no-one will want to review.

Just get the drupal coding standards in your fingers. Reviewers will point style errors when there are, and after a short while they'll come natural to you.

Powered by Dreditor.

Hi yched,

Hmm, the coding standard says

Note the comma at the end of the last array element; This is not a typo! It helps prevent parsing errors if another element is placed at the end of the list later.

Maybe that applies only to multi-line arrays? But I don't wanna start a flame about that here... :)
Nevertheless I thought it would be a good idea to clean-up some off the code. :|
Luckily reverting is faster than change the stuff (I've a validator, not a "rewriter" ;) )

I'm still working on the validator - do you have any suggestions from where I can get more detailed information - besides the well known docu page?

Cheers,
Peter

Maybe that applies only to multi-line arrays? But I don't wanna start a flame about that here... :)

Take my word for it :-). No trailing commas on inlined arrays.

do you have any suggestions from where I can get more detailed information - besides the well known docu page?

Not that I know of.

On the patch : we're getting there :-)

+++ modules/field/modules/options/options.module	2 Sep 2010 17:42:35 -0000
@@ -61,6 +61,7 @@
+      'settings' => array('display_label' => 0, ),

;-)

+++ modules/field/modules/options/options.module	2 Sep 2010 17:42:35 -0000
@@ -135,6 +140,31 @@
+  $type = str_replace('options_', '', $instance['widget']['type']);
+  switch ($type) {
+    case 'select':
+      break;
+
+    case 'buttons':
+      break;
+
+    case 'onoff':
+      $form['display_label'] = array(

We'll refactor if other widgets get settings too later, but right now a switch reads like a WTF. For now we're better off with a simple if ($instance['widget']['type'] == 'options_onoff')

+++ modules/field/modules/options/options.test	2 Sep 2010 17:42:35 -0000
@@ -456,6 +456,55 @@
+    //Go to the edit page and check if the default settings works as expected

Missing space after the // (several occurrences)

+++ modules/field/modules/options/options.test	2 Sep 2010 17:42:35 -0000
@@ -456,6 +456,55 @@
+    $this->assertRaw('>MyOnValue </label>', t('Default case shows "On value"'));

$this->xpath() would be more appropriate here. Search for $this->xpath() in test files to check how it's used.
My bet is something like

$this->assertTrue($this->xpath('//label[text()=MyOnValue'), t('Default case shows "On value"'));

+++ modules/field/modules/options/options.test	2 Sep 2010 17:42:35 -0000
@@ -456,6 +456,55 @@
+    //Go again to the edit page and check if the setting is stored and has the expected effect

Comment lines should wrap at 80 chars.

Powered by Dreditor.

Hi yched,

Thanks for your patience - for me it's like having a Drupal training. ;)
Takes some time from patch to patch - mainly because I extend my coding standard validator according to the information I get.
But here's the next one.

Btw: Are there any other line length restrictions? Just found restrictions for arrays and comments

Let's do this. This has been endlessly confusing users in CCK.

+1
Please commit this patch this before d7 string-freeze!!!!
For me this is not an problem of confusing interface, but rather a problem of not being able to do what my clients are asking, regardless of how I configure it. They want a true "yes/no" check box that does not display "yes" as the label, but does display yes or no on the node page. This can not be done without this patch and a "Yes, I want this option is selected/No, this option was not selected" check box is not acceptable. For the last two years I have been manually hacking one line of code in cck each time a new release came out (I never say no to my clients). Thanks for the patch!!

Works. The boolean field appeared broken. This patch fixes it nicely.

#18: fields-make-checkbox-label-configurable-841266-18.diff queued for re-testing.

not sure if 'string freeze' issues are supposed to be bumped to 'critical' or if the tag is enough.
In doubt, giving this some attention.

No. string freeze is string freeze. critical is critical.

This seems like a decent improvement though, even though it violates UX freeze and feature freeze both. :P~

The UX is broken here. We either have to commit this patch or add a message that explains what is going on to the confused user that is seeing their check box as
[ ] 1
instead of the expected
[ ] Do you agree to the terms of service?
Either way, we will have change something, so why not fix it?

Agreed, I just needed to deal with some actually critical/major bugs first. ;P

This looks pretty harmless, and comes with tests. I've been pretty harsh on UX freeze busting patches, but in this case I think Field UI (and in particular, boolean fields) have enough WTFisms that trying to resolve some of them before RC1 is a good thing, within reason. Since this is just adding another option that clearly people are confused about not being there, I think it's ok to go.

Committed to HEAD. Thanks.

Thks webchick.

Great to see this committed - thank you!

I really like this new feature, but just had one really minor grammar nitpick. Namely, one word is missing from the text that appears in the admin UI:

'Use field label instead of the "On value" as label'

I'll post a patch making the correction later when I'm back on my development machine.

(I took the liberty of changing this issue's status -- but I'm not sure whether it would have been more appropriate to start a new issue given that this one was about to be closed...)

As promised, a patch with the modified wording.

Good catch. :)

Committed to HEAD. It breaks string freeze, but the existing string is just wrong. :P

Yay! My first patch committed to HEAD.

Except it's somewhat tainted by the fact that I broke the options tests.... So here's another patch that hopefully makes the tests happy.

Well! This is exactly why I should always wait for testbot before committing patches, no matter how simple they look on the surface. :D

Committed #53 to HEAD.

And Nephele, great job! You know you're a real core developer once you've broken HEAD for the first time! :D

For an assertion like "Display setting checkbox is available", we should really be using $this->xpath(). That way the test doesn't depend on particular language in the field label.

Thanks, webchick! It's definitely a first for me: joining a new community by immediately breaking the committed code ;)

(but forgive me if I leave the proper xpath fix to someone else!)

Any backport to 6.x?
See: #115026: Labels for single on/off checkbox field aren't appearing

i would like to have it also for 6.x.
thanks for the patch and the cck module :)

#edit: i found a solution and i put it on the documentation page :)
http://drupal.org/node/162247
since i found so many people confused with this issue when i was searching for a solution, i think it would be a good idea to put it on the help or instructions for the field, wouldnt it?

I'm using 7.9, encounter this problem, 'label missing on "Boolean" field with widget "Single on/off checkbox"'.

This issue seems to remain unresolved.

1. Create new field: Boolean
2. Set widget to: single on/off checkbox
3. Leave Field Settings empty and save field settings
4. Leave Basic Page settings same (Label is prefilled with label text from step 1)
5. save settings

When going to create content, newly created checkbox has no label.

Edit: Read through the complete issue again, and this might be possibly works as designed.
However, as a user it appears inconsistent that a label is entered, a label is confirmed, but then the label is never used.
And by using the default settings, a standalone checkbox is created with no reference to what it does.

@darvit, Have you checked the 'Use field label instead of the "On value" as label ' checkbox on the field options, I tihnk this will give you what you need?

This was fixed a long time ago. If damiankloip's suggestion doesn't fix it for you, please open a new support request.

I have similar question at first. And #42 is the answer. Thanks!