From b57bc5855ceca30f43571c3e6b726ed9f18b2626 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 9 Oct 2013 22:41:33 +0800 Subject: [PATCH] MDL-34209 JavaScript: Simplify section drag/drop to fix reordering issues --- course/yui/dragdrop/dragdrop.js | 72 ++++++++++++++++++++++----------- lib/yui/dragdrop/dragdrop.js | 51 +++++++++++++++-------- 2 files changed, 84 insertions(+), 39 deletions(-) diff --git a/course/yui/dragdrop/dragdrop.js b/course/yui/dragdrop/dragdrop.js index d7d8d4e89d9..a5085d276d7 100644 --- a/course/yui/dragdrop/dragdrop.js +++ b/course/yui/dragdrop/dragdrop.js @@ -129,20 +129,37 @@ YUI.add('moodle-course-dragdrop', function(Y) { 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; - // 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; } @@ -167,7 +184,7 @@ YUI.add('moodle-course-dragdrop', function(Y) { 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'); @@ -190,27 +207,36 @@ YUI.add('moodle-course-dragdrop', function(Y) { 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. + var swapped = false; 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); - // Finally, hide the lightbox - window.setTimeout(function(e) { + window.setTimeout(function() { lightbox.hide(); }, 250); }, diff --git a/lib/yui/dragdrop/dragdrop.js b/lib/yui/dragdrop/dragdrop.js index 46f251b5573..dddfee47e26 100644 --- a/lib/yui/dragdrop/dragdrop.js +++ b/lib/yui/dragdrop/dragdrop.js @@ -16,7 +16,7 @@ YUI.add('moodle-core-dragdrop', function(Y) { Y.extend(DRAGDROP, Y.Base, { goingup : null, - lasty : null, + absgoingup : null, samenodeclass : null, parentnodeclass : null, groups : [], @@ -125,23 +125,38 @@ YUI.add('moodle-core-dragdrop', function(Y) { }, 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; } - //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; - } else { - //We are going down. + + } else if (info.delta[1] > 0) { + // Otherwise we're going down. this.goingup = false; } - //Cache for next check - this.lasty = y; + 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)) { - //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) {