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

unwanted confirmation popup in feedback

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Feedback
    • Labels:
    • Testing Instructions:
      Hide
      • Enable the feedback activity
      • Add a new feedback activity
        • Choose Save and Display
      • Select 'Edit questions'
      • Choose a question
        • Confirm that the page changes without showing a form change warning
      • Create the question
      • Choose the Templates tab
      • Save questions as a new template
      • Select the template you just created from the dropdown list
        • Confirm that the page changes without showing a form change warning
      Show
      Enable the feedback activity Add a new feedback activity Choose Save and Display Select 'Edit questions' Choose a question Confirm that the page changes without showing a form change warning Create the question Choose the Templates tab Save questions as a new template Select the template you just created from the dropdown list Confirm that the page changes without showing a form change warning
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32641-master-1

      Description

      1. Add a feedback activity
      2. try editing questions of this activity
        Each time you select an entry from drop down it pops up a confirmation msg "do you want to stay or leave"
        This popup is not at all required at this point and is really annoying.
        Thanks

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              poltawski Dan Poltawski added a comment -

              Andrew - assigning this to you to see if you can solve this problem since you added the confirmation alert

              Reassign if not able to do it.

              Show
              poltawski Dan Poltawski added a comment - Andrew - assigning this to you to see if you can solve this problem since you added the confirmation alert Reassign if not able to do it.
              Hide
              dobedobedoh Andrew Nicols added a comment -

              Ah - sorry, this was vaguely on my radar but it had fallen by the way-side. I have two thoughts for fixes:

              We can either add the following to the onChange handler on the select:

              onChange="M.core_formchangechecker.set_form_submitted(); this.form.submit();"

              Or, we can rewrite the add Question dropdown to use a single_select() instead of an mform.

              Any preference on which to take?

              Obviously, the set_form_submitted route is less work, but I think that an mform for this is probably overkill - maybe a TODO for 2.4 though instead.

              Show
              dobedobedoh Andrew Nicols added a comment - Ah - sorry, this was vaguely on my radar but it had fallen by the way-side. I have two thoughts for fixes: We can either add the following to the onChange handler on the select: onChange="M.core_formchangechecker.set_form_submitted(); this.form.submit();" Or, we can rewrite the add Question dropdown to use a single_select() instead of an mform. Any preference on which to take? Obviously, the set_form_submitted route is less work, but I think that an mform for this is probably overkill - maybe a TODO for 2.4 though instead.
              Hide
              dobedobedoh Andrew Nicols added a comment -

              I've gone for the simple option. It may be worth changing this at a later date to not use mform but single_select instead.

              Show
              dobedobedoh Andrew Nicols added a comment - I've gone for the simple option. It may be worth changing this at a later date to not use mform but single_select instead.
              Hide
              poltawski Dan Poltawski added a comment -

              Andrew, this is the second issue i've come across which has the wrong git diff url (pointing at a version number change). Is your git diff thing broken, or are you copying and pasting wrong?

              Show
              poltawski Dan Poltawski added a comment - Andrew, this is the second issue i've come across which has the wrong git diff url (pointing at a version number change). Is your git diff thing broken, or are you copying and pasting wrong?
              Hide
              poltawski Dan Poltawski added a comment -

              Looks good, pushing the integration button.

              Show
              poltawski Dan Poltawski added a comment - Looks good, pushing the integration button.
              Hide
              stronk7 Eloy Lafuente (stronk7) 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
              stronk7 Eloy Lafuente (stronk7) 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 -

              Hi Andrew,
              Just checking; which branches does this need to land on? 2.2 and master only correct?

              Cheers
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Hi Andrew, Just checking; which branches does this need to land on? 2.2 and master only correct? Cheers Sam
              Hide
              dobedobedoh Andrew Nicols added a comment -

              Hi Sam. This is a master only feature.

              Cheers,

              Andrew

              Show
              dobedobedoh Andrew Nicols added a comment - Hi Sam. This is a master only feature. Cheers, Andrew
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Andrew, this has been integrated now

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Andrew, this has been integrated now
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              working as expected.
              Passing
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - working as expected. Passing Thanks
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              U P S T R E A M I Z E D !

              Many thanks for the hard work, closing this as fixed.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12