Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-34221

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            aaronw@catalyst.net.nz 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
            aaronw@catalyst.net.nz 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
            danmarsden 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
            danmarsden 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 Marina Glancy added a comment -

            Davo, can you please look at this issue

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

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

            Show
            davosmith Davo Smith added a comment - I'll take a look this evening (UK time)
            Hide
            davosmith 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
            davosmith 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
            poltawski Dan Poltawski added a comment -

            Marina are you able to review this?

            Show
            poltawski Dan Poltawski added a comment - Marina are you able to review this?
            Hide
            marina 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 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
            davosmith 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
            davosmith 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
            poltawski 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
            poltawski 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys, this has been integrated now

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

            Test successful on 2.3 and master. Cheers!

            Show
            fred Frédéric Massart added a comment - Test successful on 2.3 and master. Cheers!
            Hide
            nebgor 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
            nebgor 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:
                  Fix Release Date:
                  10/Sep/12