Moodle
  1. Moodle
  2. MDL-34221

disabledIf doesn't work on drag-and-drop filepicker form elements

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.3.2
    • Component/s: Filepicker
    • Labels:
    • Testing Instructions:
      Hide

      1. Write a Moodle form which contains a form element of type 'filepicker' AND the form element of type 'filemanager'
      2. Add a form element of type 'select' to the form, with options 'yes' and 'no'.
      3. Use $mform->disabledIf() to make it so that when the 'select' element is set to 'no', the filepicker should be disabled.
      4. Print the form in a standard Moodle page.
      5. Set the 'select' element to 'no'

      Expected outcome: The filepicker/filemanager will become disabled, user is unable to add files either by drag-and-drop or by using filepicker. User also can not see the list of files inside the elements (if there were any)

      Show
      1. Write a Moodle form which contains a form element of type 'filepicker' AND the form element of type 'filemanager' 2. Add a form element of type 'select' to the form, with options 'yes' and 'no'. 3. Use $mform->disabledIf() to make it so that when the 'select' element is set to 'no', the filepicker should be disabled. 4. Print the form in a standard Moodle page. 5. Set the 'select' element to 'no' Expected outcome: The filepicker/filemanager will become disabled, user is unable to add files either by drag-and-drop or by using filepicker. User also can not see the list of files inside the elements (if there were any)
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-34221-master
    • Rank:
      42562

      Description

      The new drag-and-drop filepickers don't get disabled by the Javascript generated by $mform->disabledIf(). The "Choose a file..." button gets disabled, but the drag-and-drop box with the arrow and the text "You can drag and drop files here to add them" does not disable.

        Activity

        Hide
        Aaron Wells added a comment -

        This is quite similar to MDL-26167, which popped up when the Moodle 2 filepickers were first added. Here's a test script to demonstrate the issue:

        <?php
        
        require_once('config.php');
        require_once($CFG->dirroot.'/lib/formslib.php');
        
        class testform extends moodleform {
            function definition() {
                global $CFG;
        
                $mform =& $this->_form;
        
                $mform->addElement('select','enabler','Enable other form elements?', array(0=>'No',1=>'Yes'));
        
                // A text field will enable/disable properly
                $mform->addElement('text','foo','This is a text field');
                $mform->disabledIf('foo','enabler','neq',1);
        
                // A filepicker's drag-and-drop area will remain enabled no matter what
                $filemanager_options = array();
                $filemanager_options['return_types'] = 3;
                $filemanager_options['accepted_types'] = 'web_image';
                $filemanager_options['maxbytes'] = get_max_upload_file_size($CFG->maxbytes);
                $mform->addElement('filepicker', 'bar', 'This is a filepicker', null, $filemanager_options);
                $mform->disabledIf('bar','enabler','neq',1);
            }
        }
        require_login();
        echo $OUTPUT->header();
        echo $OUTPUT->heading('Test');
        $form = new testform();
        
        $form->display();
        
        echo $OUTPUT->footer();
        Show
        Aaron Wells added a comment - This is quite similar to MDL-26167 , which popped up when the Moodle 2 filepickers were first added. Here's a test script to demonstrate the issue: <?php require_once('config.php'); require_once($CFG->dirroot.'/lib/formslib.php'); class testform extends moodleform { function definition() { global $CFG; $mform =& $ this ->_form; $mform->addElement('select','enabler','Enable other form elements?', array(0=>'No',1=>'Yes')); // A text field will enable/disable properly $mform->addElement('text','foo','This is a text field'); $mform->disabledIf('foo','enabler','neq',1); // A filepicker's drag-and-drop area will remain enabled no matter what $filemanager_options = array(); $filemanager_options['return_types'] = 3; $filemanager_options['accepted_types'] = 'web_image'; $filemanager_options['maxbytes'] = get_max_upload_file_size($CFG->maxbytes); $mform->addElement('filepicker', 'bar', 'This is a filepicker', null , $filemanager_options); $mform->disabledIf('bar','enabler','neq',1); } } require_login(); echo $OUTPUT->header(); echo $OUTPUT->heading('Test'); $form = new testform(); $form->display(); echo $OUTPUT->footer();
        Hide
        Dan Marsden added a comment -

        you can probably also test this on the add scorm page - when other scorm types are enabled in admin settings.

        Show
        Dan Marsden added a comment - you can probably also test this on the add scorm page - when other scorm types are enabled in admin settings.
        Hide
        Marina Glancy added a comment -

        Davo, can you please look at this issue

        Show
        Marina Glancy added a comment - Davo, can you please look at this issue
        Hide
        Davo Smith added a comment -

        I'll take a look this evening (UK time)

        Show
        Davo Smith added a comment - I'll take a look this evening (UK time)
        Hide
        Davo Smith added a comment -

        The styling of the disabled filepicker could possibly do with a bit of tweaking, but the code works (in the tests I've completed).

        Show
        Davo Smith added a comment - The styling of the disabled filepicker could possibly do with a bit of tweaking, but the code works (in the tests I've completed).
        Hide
        Dan Poltawski added a comment -

        Marina are you able to review this?

        Show
        Dan Poltawski added a comment - Marina are you able to review this?
        Hide
        Marina Glancy added a comment -

        this looks pretty good. But I would go further and do the same for filemanager form element as well.
        This is my additional changes:
        https://github.com/marinaglancy/moodle/compare/master...wip-MDL-34221-master

        Show
        Marina Glancy added a comment - this looks pretty good. But I would go further and do the same for filemanager form element as well. This is my additional changes: https://github.com/marinaglancy/moodle/compare/master...wip-MDL-34221-master
        Hide
        Davo Smith added a comment -

        Those additional changes look good to me (from reading the code, I haven't pulled and tested) - do you want to update the repo / branch in the issue and then submit for integration?

        Show
        Davo Smith added a comment - Those additional changes look good to me (from reading the code, I haven't pulled and tested) - do you want to update the repo / branch in the issue and then submit for integration?
        Hide
        Dan Poltawski added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Thanks guys, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks guys, this has been integrated now
        Hide
        Frédéric Massart added a comment -

        Test successful on 2.3 and master. Cheers!

        Show
        Frédéric Massart added a comment - Test successful on 2.3 and master. Cheers!
        Hide
        Aparup Banerjee added a comment -

        yay, it works!

        This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

        Thank you all for taking the time to get us here.

        cheers!

        Show
        Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: