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

Drag-drop global listener may conflict with other Drag-drop instances

          Details

          • Type: Bug
          • Status: Closed
          • Priority: Minor
          • Resolution: Fixed
          • Affects Version/s: 2.3
          • Fix Version/s: 2.3
          • Component/s: Course
          • Labels:
            None
          • Testing Instructions:
            Hide

            For regression testing, make sure that drag-drop of sections, resources and blocks still work as before after this patch is applied.

            To test the feature itself, try adding a separate drag-drop instance on the page that already uses moodle-core-dragdrop module (e.g. course/view.php). You may use MDL-33292 for testing, it will not work without this patch.

            Show
            For regression testing, make sure that drag-drop of sections, resources and blocks still work as before after this patch is applied. To test the feature itself, try adding a separate drag-drop instance on the page that already uses moodle-core-dragdrop module (e.g. course/view.php). You may use MDL-33292 for testing, it will not work without this patch.
          • Affected Branches:
            MOODLE_23_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            MDL-33192-master-1
          • Pull Master Diff URL:

            Description

            Drag-drop global listener may conflict with other Drag-drop instances that do not use moodle-core-dragdrop YUI module. inGroup check should reply on drag instance where applicable, not on drop instance only (inGroup). Custom inGroup function for drag instances needs to be added to moodle-core-dragdrop module.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  blocks

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-33292 Add ability to drag/drop the chooser dialogues (module chooser)

                  • Minor - Minor loss of function where workaround is possible
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

                    Hi Ruslan,

                    The changes look fine. I have to admit without seeing the conflict I can only speculate on its need or effectiveness.
                    However as this doesn't appear to break anything I think get it up for integration.
                    Although could you please update the test instruction to include testing the two areas moodle-core-dragdrop is being used so that we can at least be positive its not breaking anything.

                    Cheers
                    Sam

                    Show
                    samhemelryk Sam Hemelryk added a comment - Hi Ruslan, The changes look fine. I have to admit without seeing the conflict I can only speculate on its need or effectiveness. However as this doesn't appear to break anything I think get it up for integration. Although could you please update the test instruction to include testing the two areas moodle-core-dragdrop is being used so that we can at least be positive its not breaking anything. Cheers Sam
                    Hide
                    Permalink
                    dobedobedoh Andrew Nicols added a comment -

                    FYI, something that was breaking without this fix was MDL-33292

                    Show
                    dobedobedoh Andrew Nicols added a comment - FYI, something that was breaking without this fix was MDL-33292
                    Hide
                    Permalink
                    kabalin Ruslan Kabalin added a comment -

                    Thanks Sam for revision, testing instruction has been updated. Ready for integration now.

                    Show
                    kabalin Ruslan Kabalin added a comment - Thanks Sam for revision, testing instruction has been updated. Ready for integration now.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Ruslan, I don't think you have taken what Sam was getting at with he testing instructions. What could break by doing this change? Where should we test for regressions?

                    Until the related issue is integrated the important point is that we don't introduce regressions.

                    Show
                    poltawski Dan Poltawski added a comment - Ruslan, I don't think you have taken what Sam was getting at with he testing instructions. What could break by doing this change? Where should we test for regressions? Until the related issue is integrated the important point is that we don't introduce regressions.
                    Hide
                    Permalink
                    kabalin Ruslan Kabalin added a comment -

                    Thanks Dan, got it now, added missing bit to instruction.

                    Show
                    kabalin Ruslan Kabalin added a comment - Thanks Dan, got it now, added missing bit to instruction.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Hi Ruslan,

                    inGroup is using camelCase rather than underscores. Its both inconsistent with the other code and against our guidelines.

                    Please make it consistent.

                    Show
                    poltawski Dan Poltawski added a comment - Hi Ruslan, inGroup is using camelCase rather than underscores. Its both inconsistent with the other code and against our guidelines. Please make it consistent.
                    Hide
                    Permalink
                    kabalin Ruslan Kabalin added a comment -

                    Thanks Dan, function names are using snakecase now.

                    Show
                    kabalin Ruslan Kabalin added a comment - Thanks Dan, function names are using snakecase now.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Integrated, thanks Ruslan

                    Show
                    poltawski Dan Poltawski added a comment - Integrated, thanks Ruslan
                    Hide
                    Permalink
                    abgreeve Adrian Greeve added a comment -

                    I tried out the different drag and drop sections. I didn't have any luck with the patch from MDL-33292, but I didn't spot any regressions either.
                    Test passed

                    Show
                    abgreeve Adrian Greeve added a comment - I tried out the different drag and drop sections. I didn't have any luck with the patch from MDL-33292 , but I didn't spot any regressions either. Test passed
                    Hide
                    Permalink
                    abgreeve Adrian Greeve added a comment -

                    I managed to get patch MDL-33292 working. There were conflicts with the rebase that was stopping it from working.

                    Show
                    abgreeve Adrian Greeve added a comment - I managed to get patch MDL-33292 working. There were conflicts with the rebase that was stopping it from working.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Congratulations!

                    Your work has made into the latest Moodle release!

                    You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

                    Show
                    poltawski Dan Poltawski added a comment - Congratulations! Your work has made into the latest Moodle release! You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

                      People

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

                        Dates

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