Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#11 | edit_states_attribute-2871794-11.patch | 1.24 KB | shashikant_chauhan |
#6 | edit_states_attribute-2871794-6.patch | 845 bytes | shabana.navas |
#5 | edit_states_attribute-2871794-5.patch | 847 bytes | shabana.navas |
#2 | file_managed_states.PNG | 57.98 KB | shabana.navas |
Comments
Comment #2
shabana.navas CreditAttribution: shabana.navas at Acro Commerce commentedThere seems to be some duplication in the way the attributes are added (see attached image).
Comment #3
shabana.navas CreditAttribution: shabana.navas at Acro Commerce commentedComment #4
shabana.navas CreditAttribution: shabana.navas at Acro Commerce commentedComment #5
shabana.navas CreditAttribution: shabana.navas at Acro Commerce commentedBasically, 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'] 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.
Comment #6
shabana.navas CreditAttribution: shabana.navas at Acro Commerce commentedRemoved unnecessary spaces.
Comment #9
shabana.navas CreditAttribution: shabana.navas at Acro Commerce commentedRe-rolling patch against latest version.
Comment #11
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented@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.
Comment #12
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commentedRelated issue.
https://www.drupal.org/node/2847425
Comment #14
scott_euser CreditAttribution: scott_euser as a volunteer and at Fat Beehive commentedI 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
Comment #15
scott_euser CreditAttribution: scott_euser as a volunteer and at Fat Beehive commentedAs 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.