Use case
--------
Create a select form element of with required set to true, When the form element is rendered, it has option
"- select -" set as default.

This is set so as to avoid form getting submitted with the first select option as default.

below is sample code and screen grab to demonstrate the error

function test_form($form, &$form_state) {
  $form = array();
  
  $form['default_select_element'] = array(
    '#type' => 'select',
    '#title' => t('test select'),
    '#required' => TRUE,
    '#options' => array('red', 'blue' => 'blue', 'green' => 'green'),
    '#default_value' => NULL,
  );
  
  $form['select_radios'] = array(
    '#type' => 'radios',
    '#title' => t('On & Off radios'),
    '#required' => TRUE,
    '#options' => array('on' => 'on', 'off' => 'off'),
    '#default_value' => NULL,
  );
  
  $form['optional_select_element'] = array(
    '#type' => 'select',
    '#title' => t('Select element dependent on states'),
    '#options' => array('Yes' => 'yes', 'No' => 'No'),
    '#states' => array(
      'visible' => array(
        ':input[name="select_radios"]' => array('value' => 'on'),
      ),   
      'required' => array(
        ':input[name="select_radios"]' => array('value' => 'on'),
      ),
    ),
    '#default_value' => NULL,
  );
  
  return $form;
}

Screen grab

Having '-select-' option for form elements controlled by '#states' is very important, as it stops stops from form getting submitted with the first top option selected by default.

Files: 
CommentFileSizeAuthor
#23 select-handling.png8.31 KBDavid_Rothstein
#19 form-1426646-13-before.png27.03 KBandymartha
#19 form-1426646-13-after.png27.42 KBandymartha
#13 form-1426646-13.patch848 bytesRavi.J
PASSED: [[SimpleTest]]: [MySQL] 52,239 pass(es). View
#8 form-1426646-8.patch914 bytesxjm
PASSED: [[SimpleTest]]: [MySQL] 40,255 pass(es). View
#8 interdiff.txt965 bytesxjm
#1 0729-patch-1426646-Fix-for-select-not-set-when-elements-have-states.patch1.2 KBRavi.J
PASSED: [[SimpleTest]]: [MySQL] 37,801 pass(es). View
#1 Screen Shot 2012-02-03 at 00.08.55.png34.78 KBRavi.J
Screen Shot 2012-02-02 at 23.31.18.png58.3 KBRavi.J

Comments

Ravi.J’s picture

Patch attached

Screen grab of form elements now work correctly with patch applied.

select elements working correctly

Ravi.J’s picture

Status: Active » Needs review
marcingy’s picture

Version: 7.x-dev » 8.x-dev

This needs to be fixed in d8 first otherwise looks good. A test would be nice but I don't think we can have one as it is ajax.

marcingy’s picture

Status: Needs review » Needs work
Ravi.J’s picture

I will roll out a D8 patch in the evening today.
As the patch is very small and straight forward, should be simple to go into D7 as well.

xjm’s picture

Issue tags: +needs backport to D7

Yeah, there's no way to add a test this without #237566: Automated JavaScript unit testing framework.

xjm’s picture

Priority: Major » Normal

This is normal based on http://drupal.org/node/45111.

xjm’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing, +FAPI #states
FileSize
965 bytes
914 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,255 pass(es). View

Rerolled with some cleanup and a better comment.

We'll want to manually test this with the #states test module:
http://drupal.org/sandbox/rfay/1269964

nod_’s picture

Issue tags: +JavaScript

tagging

stBorchert’s picture

Status: Needs review » Reviewed & tested by the community

During the core sprint on DrupalCon Munich I've installed a new site with a fresh checkout of Drupal 8 and installed _#states exercise_.
Test result: select elements controlled by statsAPI do not lose the "-select-" option anymore -> RTBC

catch’s picture

Status: Reviewed & tested by the community » Needs work

This comment looks a bit odd to me:

+    // If the element is set to #required through #states, override the
+    // element's #required setting.
+    // ['required'] element else use form['#required'] element.

Is that last fragment even needed?

Ravi.J’s picture

Status: Needs work » Needs review
FileSize
848 bytes
PASSED: [[SimpleTest]]: [MySQL] 52,239 pass(es). View

Rerolled patch with cleaned up comments.
Will be great to see this committed.

Ravi.J’s picture

Rerolled patch with cleaned up comments.
Will be great to see this committed.

Ravi.J’s picture

Status: Needs review » Reviewed & tested by the community
marcingy’s picture

Status: Reviewed & tested by the community » Needs review

You can't rtbc your own patches

Ravi.J’s picture

I have had this patch for D7 here since February. Path was already RTBC'd before and the trivial nature of the patch was hoping it would be committed.

YesCT’s picture

#13: form-1426646-13.patch queued for re-testing.

andymartha’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
27.42 KB
27.03 KB

I can confirm that after installing Drupal 8.x-dev from March 6th, 2013, the drupal form code put in the original example yielded one select box with a default blank option, but the one that used '#states' did not have that option.

After applying the form-1426646-13.patch from #13 by Ravi.J, the form example module yielded both consistant default blank options. See screenshots.
Is everyone ok with the code comments and formatting?

YesCT’s picture

(nothing in the following to hold this up. Just a note that dreditor makes it super easy to embed images in issue comments, so dreditor is highly recommended for that and lots of other reasons. Also, taking screenshots of drop downs.. it is sometimes nice to get the expanded drop down, using Jing on a mac, that is possible by using the key combination: command-shift-1. See: http://drupal.org/node/1859584)

coding style looks good.

webchick’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

This looks like a nice improvement, thanks. Not really possible to provide tests for this, I don't think, since it's JS-related.

Committed and pushed to 8.x. Moving down to 7.x.

attiks’s picture

There should be tests in testswarm, but possibly not covering all cases, see also the FAT project

David_Rothstein’s picture

Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Needs work
FileSize
8.31 KB

I don't understand this patch. Why would we want to make Drupal use terminology for required fields even when the field isn't required?

For example, with the code example shown at the top of this issue but modified slightly to remove the 'visible' part I now get this:

select-handling.png

That's inconsistent; there are two fields both showing "- Select -" even though only one of them actually requires you to select something. Normally, Drupal would use "- None -" for the second one.

I am not sure there is any bug to fix here in the first place, actually - maybe just a documentation improvement is needed? If you want the behavior described at the top of this issue, you should be able to use #empty_option and/or #empty_value; that's what they're there for. I don't think Drupal core should try to guess the intention of someone who is using #states.

Ravi.J’s picture

@David_Rothstein it is indeed a bug in core . The issue is that both the elements are required. One of them is set to required with form #states, while other is set to required using #required attribute.

At the moment their behaviours are not consistant.
@webchick , so did my patch get committed ? Wow, delighted, first core patch that got accepted even though its very small.

Ravi.J’s picture

Would be great if someone can review and RTBC the path so that the fix can be committed to D7 too

MichaelLund’s picture

Need to change to so that "Select element depending to states" drop-down, is either - Select - or - None - depending on whether the field is required or not.
comment #1 it is required and says - Select as it should

comment #23 its not required and should say - None - instead of select

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 4a78850 on 8.3.x
    Issue #1426646 by Ravi.J, xjm: Fixed '-Select-' option is lost when form...

  • webchick committed 4a78850 on 8.3.x
    Issue #1426646 by Ravi.J, xjm: Fixed '-Select-' option is lost when form...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 4a78850 on 8.4.x
    Issue #1426646 by Ravi.J, xjm: Fixed '-Select-' option is lost when form...

  • webchick committed 4a78850 on 8.4.x
    Issue #1426646 by Ravi.J, xjm: Fixed '-Select-' option is lost when form...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.