Problem/Motivation

#states is not working when field type is managed_file. Issue seems to have been fixed in 8.0 - https://www.drupal.org/node/1118016 but we're having issues in 8.x-4.x due to the way the field is rendering.

Steps to reproduce:
If I am changing its type to file or something else like textfield then #states is working.
This issue is occurring on latest 8.4.x dev branch.

Sample code.

$form['field1'] = array(
      '#type' => 'select',
      '#title' => $this->t('Field 1'),
      '#options' => array('op1' => 'op1', 'op2' => 'op2'),
    );

    $form['field2'] = array(
      '#title' => $this->t('Field 2'),
      '#type' => 'managed_file',
      '#upload_validators' => array(
        'file_validate_extensions' => array('jpg png gif jpeg')
        ),
      '#upload_location' => 'public://import/',
      '#states' => array(
        'visible' => array(
          ':input[name="field1"]' => array(
            'value' => 'op2'
            )
          )
        )
      );

Proposed resolution

Remove ajax-wrapper id around the widget and add a relevant file-managed id.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

shashikant_chauhan created an issue. See original summary.

shabana.navas’s picture

There seems to be some duplication in the way the attributes are added (see attached image).

shabana.navas’s picture

Issue summary: View changes
shabana.navas’s picture

Issue summary: View changes
shabana.navas’s picture

Status: Active » Needs review
FileSize
847 bytes

Basically, the issue with the #states not working for file managed was in the function template_preprocess_file_managed_file() in the following line:

$variables['attributes'] = [];

$variables['attributes'] was being reset when it was holding information about the states. The attached patch fixes the issue.

However, we still have an issue with the 'ajax-wrapper' #id being added twice to the file-managed field. We might need to file another issue for that.

shabana.navas’s picture

The last submitted patch, 5: edit_states_attribute-2871794-5.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: edit_states_attribute-2871794-6.patch, failed testing.

shabana.navas’s picture

Status: Needs work » Needs review
FileSize
845 bytes

Re-rolling patch against latest version.

Status: Needs review » Needs work

The last submitted patch, 9: edit_states_attribute-2871794-9.patch, failed testing.

shashikant_chauhan’s picture

Status: Needs work » Needs review
FileSize
1.24 KB

@shabana, Yes you are right #states properties are being reset in the function template_preprocess_file_managed_file().
We already have issue for Duplicate AJAX wrapper around a file field
I have updated the test file also in this patch, but not sure that is the right way for updating unit tests.

shashikant_chauhan’s picture

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

scott_euser’s picture

I think we should stick to https://www.drupal.org/node/2847425 and close this issue. This patch is missing the test coverage + doesn't solve the issue when you already have a file uploaded and both the wrapper and the form element need to have the states on them.

Eg, form with checkbox & manage file visibility and required both conditional on checked:
1. Check box
2. Upload managed file
3. Uncheck box
4. Need to have the managed file become both optional and hidden (including the file you've uploaded which sits above the element and within the wrapper)
5. Check box
6. Need to have the managed file and uploaded file visible again and the managed file required

scott_euser’s picture

Status: Needs review » Closed (duplicate)

As per https://www.drupal.org/node/2847425#comment-12218675 this approach also produces invalid html - there are 2 of the same id on the page by doing this. Please see the related issue where we instead move the states but leave the id's as is.

Suggesting we close this in favour of the related issue. Please reopen if you disagree.