Moodle
  1. Moodle
  2. MDL-33192

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

        Details

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

              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
                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
                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
                Andrew Nicols added a comment -

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

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

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

                Show
                Ruslan Kabalin added a comment - Thanks Sam for revision, testing instruction has been updated. Ready for integration now.
                Hide
                Permalink
                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
                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
                Ruslan Kabalin added a comment -

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

                Show
                Ruslan Kabalin added a comment - Thanks Dan, got it now, added missing bit to instruction.
                Hide
                Permalink
                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
                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
                Ruslan Kabalin added a comment -

                Thanks Dan, function names are using snakecase now.

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

                Integrated, thanks Ruslan

                Show
                Dan Poltawski added a comment - Integrated, thanks Ruslan
                Hide
                Permalink
                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
                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
                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
                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
                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
                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: