MDL-34209 JavaScript: Simplify section drag/drop to fix reordering issues
authorAndrew Nicols <andrew@nicols.co.uk>
Wed, 9 Oct 2013 14:41:33 +0000 (22:41 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Thu, 10 Oct 2013 06:42:06 +0000 (14:42 +0800)
course/yui/dragdrop/dragdrop.js
lib/yui/dragdrop/dragdrop.js

index d7d8d4e..a5085d2 100644 (file)
@@ -129,20 +129,37 @@ YUI.add('moodle-course-dragdrop', function(Y) {
             this.drop_hit(e);
         },
 
             this.drop_hit(e);
         },
 
+        get_section_index: function(node) {
+            var sectionlistselector = '.' + CSS.COURSECONTENT + ' ' + M.course.format.get_section_selector(Y),
+                sectionList = Y.all(sectionlistselector),
+                nodeIndex = sectionList.indexOf(node),
+                zeroIndex = sectionList.indexOf(Y.one('#section-0'));
+
+            return (nodeIndex - zeroIndex);
+        },
+
         drop_hit : function(e) {
             var drag = e.drag;
         drop_hit : function(e) {
             var drag = e.drag;
-            // Get a reference to our drag node
-            var dragnode = drag.get('node');
-            var dropnode = e.drop.get('node');
-            // Prepare some variables
-            var dragnodeid = Y.Moodle.core_course.util.section.getId(dragnode);
-            var dropnodeid = Y.Moodle.core_course.util.section.getId(dropnode);
 
 
-            var loopstart = dragnodeid;
-            var loopend = dropnodeid;
+            // Get references to our nodes and their IDs.
+            var dragnode = drag.get('node'),
+                dragnodeid = Y.Moodle.core_course.util.section.getId(dragnode),
+                loopstart = dragnodeid,
 
 
-            if (this.goingup) {
-                loopstart = dropnodeid;
+                dropnodeindex = this.get_section_index(dragnode),
+                loopend = dropnodeindex;
+
+            if (dragnodeid === dropnodeindex) {
+                Y.log("Skipping move - same location moving " + dragnodeid + " to " + dropnodeindex, 'debug', 'moodle-course-dragdrop');
+                return;
+            }
+
+            Y.log("Moving from position " + dragnodeid + " to position " + dropnodeindex, 'debug', 'moodle-course-dragdrop');
+
+            if (loopstart > loopend) {
+                // If we're going up, we need to swap the loop order
+                // because loops can't go backwards.
+                loopstart = dropnodeindex;
                 loopend = dragnodeid;
             }
 
                 loopend = dragnodeid;
             }
 
@@ -167,7 +184,7 @@ YUI.add('moodle-course-dragdrop', function(Y) {
             params['class'] = 'section';
             params.field = 'move';
             params.id = dragnodeid;
             params['class'] = 'section';
             params.field = 'move';
             params.id = dragnodeid;
-            params.value = dropnodeid;
+            params.value = dropnodeindex;
 
             // Do AJAX request
             var uri = M.cfg.wwwroot + this.get('ajaxurl');
 
             // Do AJAX request
             var uri = M.cfg.wwwroot + this.get('ajaxurl');
@@ -190,27 +207,36 @@ YUI.add('moodle-course-dragdrop', function(Y) {
                             M.course.format.process_sections(Y, sectionlist, responsetext, loopstart, loopend);
                         } catch (e) {}
 
                             M.course.format.process_sections(Y, sectionlist, responsetext, loopstart, loopend);
                         } catch (e) {}
 
+                        // Update all of the section IDs - first unset them, then set them
+                        // to avoid duplicates in the DOM.
+                        var index;
+
                         // Classic bubble sort algorithm is applied to the section
                         // nodes between original drag node location and the new one.
                         // Classic bubble sort algorithm is applied to the section
                         // nodes between original drag node location and the new one.
+                        var swapped = false;
                         do {
                         do {
-                            var swapped = false;
-                            for (var i = loopstart; i <= loopend; i++) {
-                                if (Y.Moodle.core_course.util.section.getId(sectionlist.item(i-1)) > Y.Moodle.core_course.util.section.getId(sectionlist.item(i))) {
-                                    // Swap section id
-                                    var sectionid = sectionlist.item(i-1).get('id');
-                                    sectionlist.item(i-1).set('id', sectionlist.item(i).get('id'));
-                                    sectionlist.item(i).set('id', sectionid);
-                                    // See what format needs to swap
-                                    M.course.format.swap_sections(Y, i-1, i);
-                                    // Update flag
+                            swapped = false;
+                            for (index = loopstart; index <= loopend; index++) {
+                                if (Y.Moodle.core_course.util.section.getId(sectionlist.item(index - 1)) >
+                                            Y.Moodle.core_course.util.section.getId(sectionlist.item(index))) {
+                                    Y.log("Swapping " + Y.Moodle.core_course.util.section.getId(sectionlist.item(index - 1)) +
+                                            " with " + Y.Moodle.core_course.util.section.getId(sectionlist.item(index)));
+                                    // Swap section id.
+                                    var sectionid = sectionlist.item(index - 1).get('id');
+                                    sectionlist.item(index - 1).set('id', sectionlist.item(index).get('id'));
+                                    sectionlist.item(index).set('id', sectionid);
+
+                                    // See what format needs to swap.
+                                    M.course.format.swap_sections(Y, index - 1, index);
+
+                                    // Update flag.
                                     swapped = true;
                                 }
                             }
                             loopend = loopend - 1;
                         } while (swapped);
 
                                     swapped = true;
                                 }
                             }
                             loopend = loopend - 1;
                         } while (swapped);
 
-                        // Finally, hide the lightbox
-                        window.setTimeout(function(e) {
+                        window.setTimeout(function() {
                             lightbox.hide();
                         }, 250);
                     },
                             lightbox.hide();
                         }, 250);
                     },
index 46f251b..dddfee4 100644 (file)
@@ -16,7 +16,7 @@ YUI.add('moodle-core-dragdrop', function(Y) {
 
     Y.extend(DRAGDROP, Y.Base, {
         goingup : null,
 
     Y.extend(DRAGDROP, Y.Base, {
         goingup : null,
-        lasty   : null,
+        absgoingup : null,
         samenodeclass : null,
         parentnodeclass : null,
         groups : [],
         samenodeclass : null,
         parentnodeclass : null,
         groups : [],
@@ -125,23 +125,38 @@ YUI.add('moodle-core-dragdrop', function(Y) {
         },
 
         global_drag_drag : function(e) {
         },
 
         global_drag_drag : function(e) {
-            var drag = e.target;
+            var drag = e.target,
+                info = e.info;
+
             // Check that drag object belongs to correct group
             if (!this.in_group(drag)) {
                 return;
             }
             // Check that drag object belongs to correct group
             if (!this.in_group(drag)) {
                 return;
             }
-            //Get the last y point
-            var y = drag.lastXY[1];
-            //is it greater than the lasty var?
-            if (y < this.lasty) {
-                //We are going up
+
+            // Note, we test both < and > situations here. We don't want to
+            // effect a change in direction if the user is only moving side
+            // to side with no Y position change.
+
+            // Detect changes in the position relative to the start point.
+            if (info.start[1] < info.xy[1]) {
+                // We are going up if our final position is higher than our start position.
+                this.absgoingup = true;
+
+            } else if (info.start[1] > info.xy[1]) {
+                // Otherwise we're going down.
+                this.absgoingup = false;
+            }
+
+            // Detect changes in the position relative to the last movement.
+            if (info.delta[1] < 0) {
+                // We are going up if our final position is higher than our start position.
                 this.goingup = true;
                 this.goingup = true;
-            } else {
-                //We are going down.
+
+            } else if (info.delta[1] > 0) {
+                // Otherwise we're going down.
                 this.goingup = false;
             }
                 this.goingup = false;
             }
-            //Cache for next check
-            this.lasty = y;
+
             this.drag_drag(e);
         },
 
             this.drag_drag(e);
         },
 
@@ -157,12 +172,16 @@ YUI.add('moodle-core-dragdrop', function(Y) {
             this.lastdroptarget = e.drop;
             //Are we dropping on the same node?
             if (drop.hasClass(this.samenodeclass)) {
             this.lastdroptarget = e.drop;
             //Are we dropping on the same node?
             if (drop.hasClass(this.samenodeclass)) {
-                //Are we not going up?
-                if (!this.goingup) {
-                    drop = drop.next('.'+this.samenodeclass);
+                var where;
+
+                if (this.goingup) {
+                    where = "before";
+                } else {
+                    where = "after";
                 }
                 }
-                //Add the node
-                e.drop.get('node').ancestor().insertBefore(drag, drop);
+
+                // Add the node contents so that it's moved, otherwise only the drag handle is moved.
+                drop.insert(drag, where);
             } else if ((drop.hasClass(this.parentnodeclass) || drop.test('[data-droptarget="1"]')) && !drop.contains(drag)) {
                 // We are dropping on parent node and it is empty
                 if (this.goingup) {
             } else if ((drop.hasClass(this.parentnodeclass) || drop.test('[data-droptarget="1"]')) && !drop.contains(drag)) {
                 // We are dropping on parent node and it is empty
                 if (this.goingup) {