#states not affecting visibility/requirement of managed_file

Created on 27 January 2017, about 7 years ago
Updated 11 April 2024, 8 days ago

Problem/Motivation

Cannot affect the visibility or requirement of managed_file with states. Identical to #1118016: conditional visibility of a managed_file using #states attribute does not work โ†’ so either the bug wasn't fully fixed before, or a regression happened at some point and broke this functionality.

With this definition:

$form['type'] = array(
  '#type' => 'select',
  '#options' => array(
    'video' => t('Video'),
    'audio' => t('Audio'),
    'post' => t('Text'),
  ),
);

$form['audio'] = array(
  '#type' => 'managed_file',
  '#title' => t('Audio File'),
  '#states' => array(
    'visible'  => array(':input[name="type"]' => array('value' => 'audio')),
    'required' => array(':input[name="type"]' => array('value' => 'audio')),
  ),
  '#upload_location' => 'public://',
  '#upload_validators' => array(
    'file_validate_extensions' => array('mp3 wac'),
    'file_validate_size' => array(1048576),
  ),
);

I expect the visibility of "audio" to change when changing the value of "type". What happens is "audio" remains visible and not required. Simply changing "managed_file" to "file" brings the expected behavior.

Steps to reproduce

1. Create a custom form with structure as above
2. Navigate to the form
Problems:
1. On page load the "audio file upload" option is shown, which should show up only on selecting the Audio option.
2. Mandatory asterix is not shown for audio file upload field when the Audio option is selected

Proposed resolution

Root cause: During porting of fix from 8.x to 9.x the bug seems to be introduced. In the
function template_preprocess_file_managed_file(&$variables) {
there
$variables['attributes'] = [];
is overridden.

Proposed fix: Conditionally override to ensure if that the attributes set for #states is not lost.

For the "required" capability of #states to work, the label has to be correctly set, so that the mandatory asterix can be set. The managed_file introduces a "upload" index. Set the #states to the "upload" index as the label refers to the this and not the original "audio" field.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ› Bug report
Status

Needs review

Version

11.0 ๐Ÿ”ฅ

Component
File systemย  โ†’

Last updated about 2 hours ago

Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States bander2

Live updates comments and jobs are added and updated live.
  • Accessibility

    It affects the ability of people with disabilities or special needs (such as blindness or color-blindness) to use Drupal.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Missing content requested by

๐Ÿ‡ฆ๐Ÿ‡บAustralia dpi
5 months ago
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @bander2
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    There appears to be two issues with the managed file. This is my understanding of it, but please correct me if I have it wrong.

    The managed file input is loaded via ajax and appears within an 'ajax-wrapper' div
    By the time it is loaded it appears that this bit of core/misc/states.js

    Drupal.behaviors.states = {
      attach: function (context, settings) {
      }
    };

    which assigns the fields as Dependants of the triggering input (in this case the select) has already been run. The ajax loaded file_managed field is then not assigned as a Dependent. Because of that, when the 'state:visible' event is triggered, the ajax loaded file_managed is uneffected.

    The 'data-drupal-states' attribute is not being added to the ajax loaded file_managed input
    With a file input instead of file managed, a 'data-drupal-states' attribute is added to the input with json encoded settings specifying which element the Dependent is dependent upon and when it should be made visible. This information is not available in the ajax loaded element, so even if triggered the attach function (above) again, it wouldn't know how to handle this input.

    Possible solution
    Perhaps we can attach the data-drupal-states directly to the ajax-wrapper container so regardless of what happens within the file_managed, we can be sure the states are obeyed. I am not sure if that might have other unintended consequences so perhaps the above two issues need to be addressed independently.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    The attached patch follows my possible solution listed above and appears to correctly hide the managed file on load as well as on change of the field it depends upon.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom Saphyel

    Reviewed!

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom Saphyel
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany tstoeckler Essen, Germany
    1. +++ b/core/modules/file/src/Element/ManagedFile.php
      @@ -348,8 +349,20 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
      +    // Add states to ajax wrapper to states.js can potentially attach this
      

      The second "to" should be "so"

    2. +++ b/core/modules/file/src/Element/ManagedFile.php
      @@ -348,8 +349,20 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
      +      // Add a preceeding space.
      +      $attributes = ' ' . $attribute;
      

      This should not be necessary, the Attribute class should be adding that. In fact I'm fairly sure there will be a double space currently.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    Thanks both for the review! I have updated the patch based on your feedback tstoeckler; you're right, it does add the space already.
    If you are happy with it, good to switch to 'reviewed & tested by the community'?

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany tstoeckler Essen, Germany

    I can't say anything about the JS changes, but other than that looks good. Since this was RTBC before, marking as such.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States xjm

    Thanks everyone!

    I think we should add a JS test for this, especially if it's a major bug. The states system is definitely something where JS tests help.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States xjm

    Oh and @Saphyel, when you review a patch, can you document what you reviewed and how? Code, manual testing, etc. Screenshots are always good for visual issues like this; see: https://www.drupal.org/contributor-tasks/add-screenshots โ†’

    Thanks also @scott_euser for the careful explanations!

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    @xjm Thanks! I will work on adding a JS test for it, probably tomorrow or Wednesday.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    Updated with a javascript test.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    Fixed docblocks in last patch. Interdiff to last major change as well to make it easier to review.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly gambry Milan

    Wonderful work, well done everyone!

    Below my feedbacks.

    1. +++ b/core/modules/file/src/Element/ManagedFile.php
      @@ -352,8 +353,17 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
      +    // Add states to ajax wrapper so states.js can potentially attach this
      +    // element as a Dependent.
      +    $attributes = '';
      +    if (isset($element['#states']) && !empty($element['#states'])) {
      +      $attributes = new Attribute([
      +        'data-drupal-states' => json_encode($element['#states']),
      +      ]);
      +    }
      +
           // Prefix and suffix used for Ajax replacement.
      -    $element['#prefix'] = '<div id="' . $ajax_wrapper_id . '">';
      +    $element['#prefix'] = '<div id="' . $ajax_wrapper_id . '"' . $attributes . '>';
      

      $attributes is first a string, then an Attribute() instance. Why don't we make it an Attribute() instance since definition, and maybe we create it with [id=$ajax_wrapper_id] and so print all prefix element attributes in the same way?

    2. +++ b/core/modules/file/tests/src/FunctionalJavascript/FileManagedStateTest.php
      @@ -0,0 +1,60 @@
      +  protected function setUp() {
      +    parent::setUp();
      +  }
      

      I don't think this is needed then, as it's overriding the parent::setUp() just to call it. :D

    3. +++ b/core/modules/file/file.js
      @@ -36,6 +36,10 @@
      +      $(document).trigger('state:visible');
      

      This looks to handle only the visible, so I tried some other states and something looks broken. Apparently all states are applied to the parent wrapper and not to the input element itself. So some states work (visible, invisible, disabled) but other like required don't.
      States changes should affect the element and not the parent.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia ajayNimbolkar

    Hi,

    I have used Patch #14.

    Following are my code snippet, When i apply patch then entire field set get disable.
    When i check using firebug then i got to know that the fieldset has apply "display:none" attribute.

    $form['test'] = [
    '#title' => t('Login Button Settings'),
    '#type' => 'details',
    '#open' => TRUE,
    ];

    // Login link type.
    $form['test']['test1'] = [
    '#type' => 'select',
    '#options' => [
    '' => t('- Select - '),
    'text' => t('Text'),
    'image' => t('Image'),
    ],
    '#default_value' => $settings->get('o365_user_auth_login_type'),
    '#title' => t('What type login link you want to show?'),
    ];

    // Login link text.
    $form['test']['test2'] = array(
    '#type' => 'textfield',
    '#title' => t('Login Link Text'),
    '#default_value' => $settings->get('o365_user_auth_login_link_text'),
    '#states' => [
    'visible' => [':input[name="test[test1]"]' => ['value' => 'text']],
    ],
    );

    // Image link
    $form['test']['test2'] = [
    '#title' => t('Image Login Link'),
    '#type' => 'managed_file',
    '#description' => t('The uploaded image will be displayed on this page using the image style choosen below.'),
    '#default_value' => $settings->get('o365_user_auth_login_link_image'),
    '#upload_location' => 'public://o365_user_auth/',
    '#states' => [
    'visible' => [
    ':input[name="test[test1]"]' => ['value' => 'image'],
    ],
    ],
    ];

    please help me.

    Thanks,
    Ajay

  • 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 โ†’ .

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    I've updated the patch so the state settings are applied to the element as well as wrapper. This now works for required, visible, and all other options. I've attempted to update the tests to cover required as well so we can see the two cases (where the wrapper needs to change + where the form element needs to change) - not sure if tests will pass though - need to get phantomjs working in my docker containers.

    I've fixed the other feedback items, thank you for reviewing.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly gambry Milan

    Hey @scott_euser have you seen work on the related issue?
    Can we merge the two issues, and close the duplicate?

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    Hey @gambry, just took a look at it - seems to not be solving the whole issue. Have posted a comment there suggesting we close that.
    Not sure if we should bring this bit into here:

    diff --git a/core/modules/file/file.module b/core/modules/file/file.module
    index a6fc3f6..d0bfe07 100644
    --- a/core/modules/file/file.module
    +++ b/core/modules/file/file.module
    @@ -1220,7 +1220,6 @@ function file_managed_file_save_upload($element, FormStateInterface $form_state)
     function template_preprocess_file_managed_file(&$variables) {
       $element = $variables['element'];
     
    -  $variables['attributes'] = [];
       if (isset($element['#id'])) {
         $variables['attributes']['id'] = $element['#id'];
       }
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    Have taken a look, that results in invalid html actually. Essentially results in this:

    <div data-drupal-selector="edit-managed-file" data-drupal-states="{&quot;visible&quot;:{&quot;:input[name=\u0022toggle_me\u0022]&quot;:{&quot;checked&quot;:true}}}" id="edit-managed-file-upload" class="js-form-managed-file form-managed-file">
      <div id="ajax-wrapper" data-drupal-states="{&quot;visible&quot;:{&quot;:input[name=\&quot;toggle_me\&quot;]&quot;:{&quot;checked&quot;:true}}}"><input data-drupal-selector="edit-managed-file-upload" type="file" id="edit-managed-file-upload" name="files[managed_file]" size="22" class="js-form-file form-file" data-drupal-states="{&quot;visible&quot;:{&quot;:input[name=\u0022toggle_me\u0022]&quot;:{&quot;checked&quot;:true}}}">
        <input class="js-hide button js-form-submit form-submit" data-drupal-selector="edit-managed-file-upload-button" formnovalidate="formnovalidate" type="submit" id="edit-managed-file-upload-button" name="managed_file_upload_button" value="Upload">
        <input data-drupal-selector="edit-managed-file-fids" type="hidden" name="managed_file[fids]">
      </div>
    </div>

    Notice 2 id's 'edit-managed-file-upload' where there should of course only be one on the page.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom scott_euser

    Re-rolling without any change to file.js as that was a lingering change simply from saving the file (ie, code editor automatically added in new line at end of file that was missing), but no need to keep that in scope. Otherwise identical to #20. Ready for review.

  • Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule โ†’ and the Allowed changes during the Drupal 8 release cycle โ†’ .

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wesleydv

    When a managed_file field is inside a fieldset, the entire fieldset is hidden when using visible or invisible states on the instead of just the field.
    Have not tested this using other states.

    I believe that @ajayNimbolkar in #16 is describing a similar issue while using 'details' as a wrapper instead of 'fieldset'.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States nwoodland

    Patch from #24 is working great for me, thanks for all your work on this scott_euser!

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jrockowitz Brooklyn, NY
  • Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule โ†’ and the Allowed changes during the Drupal 8 release cycle โ†’ .

  • I encountered this problem when creating a custom field widget where managed_file fields along others should be visible according to a select filed. Applying this patch made the whole custom field disappearing because states.js now changed the visibility of the entire fieldset. Because of state.js function $(e.target).closest('.js-form-item, .js-form-submit, .js-form-wrapper').toggle(e.value) the fieldset appeared als closest to the ajax-wrapper element. I discovered, that the upload element didn't had states by default. So for my needs, it was enough to add states to just the upload input element without the ajax wrapper, because states.js can still get the states of these elements and toggle them accordingly. I placed it right below the $element['upload'] definition, to keep it in line. Starting from line 303

            // The file upload field itself.
            $element['upload'] = [
                '#name'             => 'files[' . $parents_prefix . ']',
                '#type'             => 'file',
                '#title'            => t('Choose a file'),
                '#title_display'    => 'invisible',
                '#size'             => $element['#size'],
                '#multiple'         => $element['#multiple'],
                '#theme_wrappers'   => [],
                '#weight'           => -10,
                '#error_no_message' => true,
            ];
            if (!empty($element['#accept'])) {
                $element['upload']['#attributes'] = ['accept' => $element['#accept']];
            }
            // add #states to upload element
            if (isset($element['#states']) && !empty($element['#states'])) {
                $element['upload']['#states'] = $element['#states'];
            }
    
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia aadeshvermaster@gmail.com

    Hi Team,

    I have checked patch #24 with Drupal 8.5.8 & PHP 7.2. Its working for me.

    Thanks

  • ๐Ÿ‡จ๐Ÿ‡ดColombia meickol

    I am using #states for in a node edit form, with this code

    $form["field_ad_image"]["widget"][0]['#states'] = [
          'visible' => [
            ':input[name="field_interstitial_ad[value]"]' => ['checked' => TRUE]
          ],
          'required' => [
            ':input[name="field_interstitial_ad[value]"]' => ['checked' => TRUE]
          ]
        ];
    

    Using the patch #24 the field `field_ad_image` get visible if the `field_interstitial_ad` is checked, if not get hidden, but with the required is not working right because the field get the required mark but anyway you can save the form without upload any image.

    Drupal/core 8.6.4 , PHP 7.2.5

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    tagging, relevant for WCAG guideline 3.3 Input assistance.

  • Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule โ†’ and the Allowed changes during the Drupal 8 release cycle โ†’ .

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tim.plunkett Philadelphia

    Fixing tags

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States acrosman South Carolina, US

    Adding re-roll tag. Looks like the current patch does not apply to 8.8.

  • Drupal 9.2.0-alpha1 โ†’ will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • Drupal 9.1.0-alpha1 โ†’ will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule โ†’ and the Allowed changes during the Drupal 9 release cycle โ†’ .

  • Drupal 8.9.0-beta1 โ†’ was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule โ†’ and the Allowed changes during the Drupal 8 and 9 release cycles โ†’ .

  • Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9โ€™s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule โ†’ and the Allowed changes during the Drupal 8 and 9 release cycles โ†’ .

  • ๐Ÿ‡ง๐Ÿ‡พBelarus Yury N

    Patch #37 works for required state when field has no uploaded file. But with file uploaded 'upload' element is hidden and field label's 'for' points to nothing, so required state does not work. I made a label and states to refer to 'fids' element in this case.

    P.S. You need a patch from https://www.drupal.org/project/drupal/issues/1091852 ๐Ÿ› Display Bug when using #states (Forms API) with Ajax Request Needs work to have states working with AJAX elements

  • ๐Ÿ‡ฏ๐Ÿ‡ดJordan abu-zakham

    I have re-rolled patch #39, and removed core from info file to fix core_version_requirement issue.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Gauravvv Delhi, India

    Re-rolled patch #44, Updating prefix for dependencies.
    Added interdiff for same.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia vsujeetkumar Delhi

    Fixed the fail tests.
    Changed Class "Drupal\FunctionalJavascriptTests\JavascriptTestBase" to "Drupal\FunctionalJavascriptTests\WebDriverTestBase".

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia vsujeetkumar Delhi

    Fixed the fail tests. Added "$defaultTheme" and "$modules" property changed to protected.

  • ๐Ÿ‡ช๐Ÿ‡ธSpain evamtinez

    Patch #24 is working for me in Drupal 8.9.16 and PHP 7.1.

  • Drupal 9.3.0-rc1 โ†’ was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance berramou

    Patch #24 is working for me with Drupal 9.2.9 and PHP 7.4 for require state.

  • ๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia samaphp Riyadh, SA ๐Ÿ‡ธ๐Ÿ‡ฆ
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone New Zealand

    Doing a review.

    The Issue Summary begins with a statement that this issue is identical to another one, and the one has been fixed. I am not sure what this issue is fixing. Tagging for an issue summary update, see Write an issue summary for an existing issue โ†’ for guidance.

    There was a gap between Sept 2019 and July 2021 after which there have been rerolls only and no code reviews. The last code review was in #15, in May 2017.

    This patch no longer applies, tagging for a reroll.

    I applied the patch so I could run the test but then I saw it was a FunctionalJavascript test which I am not set up for. So I followed the steps by hand. I installed the test module and navigated to the form. The toggle button had no effect so I stopped. Because of that I am adding a tag for steps to reproduce.

    Not sure if this should be Major, but leaving for now.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia anushrikumari

    Re-roll patch #47 for 9.4.x-dev

  • Fixed failed tests of patch #56.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Gauravvv Delhi, India
  • Drupal 9.4.0-alpha1 โ†’ was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Narendra@drupalchamp

    hi, There
    I am using drupal 9.2.7 and i have an issue with managed_file. I used a custom form with managed_file, it creates a directory and uploads the document but in submitForm function, it gives only a "blank array". there are no details about fid, file type, size, etc on submit function. i am using $form_state->getValues('file_name'); as below code-

    $form['select_file'] = [
    '#type' => 'managed_file',
    '#title' => $this->t('choose file'),
    '#multiple' => FALSE,
    '#description' => t("Document should be in .jpeg, .jpg, .png, .gif, .txt, .docx and .pdf format!"),
    '#upload_location' => 'public://Applicant-file/applicant_'.$id.'',
    '#upload_validators' => array
    (
    'file_validate_extensions' => array('pdf jpg jpeg png gif txt docx '),
    'file_validate_size' => array(25600000),
    ),
    ];

    public function submitForm(array &$form, FormStateInterface $form_state) {
    $select_file = $form_state->getValue('select_file');
    ................................
    ................................
    }

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States DamienMcKenna NH, USA

    Slight update to the issue summary to note that while this may have been working in #1118016, it doesn't work right now.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia ameymudras

    Reviewed the code and tested the patch on 9.5.x

    - The patch applies cleanly
    - All the issues mentioned above are already taken care off
    - Tests pass, would be good to add test only patch
    - states work as expected after the patch is applied

    +1 for RTBC

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States DamienMcKenna NH, USA

    @ameymudras: Did you actually test to see if the field worked correctly, i.e. that you could not submit a form without selecting a file?

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia ameymudras

    @DamienMcKenna I had tested the form submit without adding a file which worked fine. I tested again and noticed an issue:

    - When we select Audio, file is marked as required with asterisks but it lets me submit the form. As in the required field is not respected here.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States dww

    I reverted a bunch of hunks from #57 that were out of scope code style and other refactoring. Here's a patch with only the actually necessary parts of it, both test-only (fails locally) and full (passes). No longer needs reroll.

    Re: #64:

    - When we select Audio, file is marked as required with asterisks but it lets me submit the form. As in the required field is not respected here.

    This is why I think #states should drop support for toggling required. As documented, this #states toggle has nothing to do with form validation. It literally just paints or erases your little red * next to the field. ๐Ÿ˜‚ That's it. This "works as designed".

    Back to NR.

    Thanks! -Derek

  • Drupal 9.5.0-beta2 โ†’ and Drupal 10.0.0-beta2 โ†’ were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • ๐Ÿ‡ช๐Ÿ‡ธSpain rcodina

    Patch on #65 works for me on Drupal 9.3.19. Thanks!

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joekers UK

    Patch from #65 is working for me too - on 9.4.4. Cheers!

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia ameymudras

    Reviewed the code and tested the patch on 9.5.x

    - The patch #65 applies cleanly
    - Issue summary is clear and explains the problem
    - Was able to replicate the issue with the code snippet provided in summary
    - After applying the patch field works correctly and was able to submit the form
    - Did a quick code review and no issues were identified

    Marking this issue as RTBC unless there are anything else pending here. Not including screenshots to avoid duplicate ones

  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone New Zealand

    @ameymudras, thanks for reviewing this patch. To answer your question, yes there is more work to do here. Have a look at the tags, this issues is tagged for a summary update and for steps to reproduce. That was asked for in #54, 10 months ago and has yet to be done.

    Setting back to NW for an issue summary update. At the very least the proposed resolution needs to be completed to explain the agreed solution so that any other reviewers and the committers know what should be implemented. Steps to repI've added the standard template to make it a bit easier. It shouldn't take too long for someone to update.

  • Drupal core is moving towards using a โ€œmainโ€ branch. As an interim step, a new 11.x branch has been opened โ†’ , as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada mgifford Ottawa, Ontario

    Think this is most closely tied to 3.3.1.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium tim-diels Belgium ๐Ÿ‡ง๐Ÿ‡ช

    Using Drupal 10.0.9

    When putting these inside a fieldset, the fieldset is targetted and set to display none when changing the values.
    It does work without a fieldset. So the test and code should be updated to also address this issue.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada mgifford Ottawa, Ontario

    Yes, we really want to avoid using display:none; unless we really want it to be invisible to everyone including screen reader users.

  • ๐Ÿ‡ต๐Ÿ‡ฑPoland piotrkonefal

    The same issue seems to be occurring on Webforms. Conditional logic options based on state of `managed_file` field does not seem to work.
    Steps to reproduce:

    1. Create a webform with a file field.
    2. In Webform configuration (Build > Elements section), choose 'Edit' option for 'Submit button(s)'.
    3. Switch to 'Conditions' tab.
    4. Set a condition as: Visible - If - File Field (is) - Filled
    5. Save webform settings.
    6. Go to the webform. Observe that submit button is hidden, based on configured condition.
    7. Upload a file - see that 'Submit' button is still NOT visible (but it should be)

    There is a working workaround for that scenario:
    1. Add custom webform handler and use it on affected webform.
    2. Define alterForm() method and build conditional visibility there.
    Example:

    $form['elements']['actions']['#states'] = [
          'visible' => [
              '.webform-submission-activity-tasks-form :input[data-drupal-selector="edit-presentation-fids"]' => ['filled' => TRUE],
          ],
        ];
    

    In terms of Webforms there is another issue I've caught which is related to this one. I've created a separate thread: https://www.drupal.org/project/webform/issues/3399906#comment-15308665 ๐Ÿ› Conditional logic options are not getting saved Active

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia er.garg.karan Chandigarh

    The patch #65 not working for me when I am trying to add a #required state.
    Drupal version 9.5.9

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia adwivedi008

    Patch #65 works for me i am using it on D-9.5

  • First commit to issue fork.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s
  • Merge request !7305support for #states for managed_file โ†’ (Open) created by sukr_s
  • Pipeline finished with Failed
    16 days ago
    Total: 177s
    #136162
  • Pipeline finished with Failed
    16 days ago
    Total: 630s
    #136183
  • Pipeline finished with Failed
    16 days ago
    Total: 580s
    #136228
  • Pipeline finished with Success
    16 days ago
    Total: 648s
    #136266
  • Status changed to Needs review 16 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s

    - Issue summary has been updated.
    - Root cause of #1118016: conditional visibility of a managed_file using #states attribute does not work โ†’ not working has been identified and fixed. Different from the patch.
    - Test case reused from #2847425-65: #states not affecting visibility/requirement of managed_file โ†’ and included.

  • Status changed to Needs work 9 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Reviewing the proposed solution seems to describe the problem or steps to reproduce vs what the fix was. So leaving both issue summary and steps to reproduce tags.

    Also some comments on the MR.

  • Pipeline finished with Canceled
    8 days ago
    Total: 225s
    #143587
  • Pipeline finished with Success
    8 days ago
    Total: 604s
    #143597
  • Status changed to Needs review 8 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s

    - Updated the IS
    - Updated the proposed changes

Production build https://api.contrib.social 0.62.1