From 4c2bd1c2876b981eb39c1b3b12dc7597ab9a9bf6 Mon Sep 17 00:00:00 2001 From: Dan LaMotte Date: Wed, 14 Oct 2015 16:09:02 -0500 Subject: [PATCH 1/2] fix for sorting elements within a non-homogenus list of elements For instance, say you are sorting tbody's within a table element. There can also exist tfoot and thead within the table element. The _index() function assumes that the list of elements within the parent is homogenus (ie: just li's below an ol/ul). This isnt the case when sorting tbody under table. This is edge case until you want to group tr's under a table and sort on those groupings. --- Sortable.js | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/Sortable.js b/Sortable.js index 8699317..1c7b2b6 100644 --- a/Sortable.js +++ b/Sortable.js @@ -271,7 +271,7 @@ } // get the index of the dragged element within its parent - oldIndex = _index(target); + oldIndex = _index(target, options.draggable); // Check filter if (typeof filter === 'function') { @@ -766,7 +766,7 @@ _toggleClass(dragEl, this.options.chosenClass, false); if (rootEl !== parentEl) { - newIndex = _index(dragEl); + newIndex = _index(dragEl, options.draggable); if (newIndex >= 0) { // drag from one list and drop into another @@ -786,7 +786,7 @@ if (dragEl.nextSibling !== nextEl) { // Get the index of the dragged element within its parent - newIndex = _index(dragEl); + newIndex = _index(dragEl, options.draggable); if (newIndex >= 0) { // drag & drop within the same list @@ -1162,11 +1162,13 @@ } /** - * Returns the index of an element within its parent + * Returns the index of an element within its parent for a selected set of + * elements * @param {HTMLElement} el + * @param {selector} selector * @return {number} */ - function _index(el) { + function _index(el, selector) { var index = 0; if (!el || !el.parentNode) { @@ -1174,7 +1176,8 @@ } while (el && (el = el.previousElementSibling)) { - if (el.nodeName.toUpperCase() !== 'TEMPLATE') { + if (el.nodeName.toUpperCase() !== 'TEMPLATE' + && _matches(el, selector)) { index++; } } @@ -1182,6 +1185,23 @@ return index; } + function _matches(/**HTMLElement*/el, /**String*/selector) { + var matches, + i = 0; + + if (el.matches) { + return el.matches(selector); + } else if (el.matchesSelector) { + return el.matchesSelector(selector); + } else { + matches = (el.document || el.ownerDocument).querySelectorAll(selector); + while (matches[i] && matches[i] !== el) { + i++; + } + return matches[i] ? true : false; + } + } + function _throttle(callback, ms) { var args, _this; From 4facaff7155128db751fda51aca582742e1d6f1c Mon Sep 17 00:00:00 2001 From: Dan LaMotte Date: Wed, 18 Nov 2015 14:38:12 -0600 Subject: [PATCH 2/2] refactor _matches() out of _closest() The _closest() method already defined a way to do what _matches() was trying to accomplish. This pulls the functionality out of _closest() so that it can be reused outside of _closest() but refactors _closest() to use this new _matches() method. --- Sortable.js | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/Sortable.js b/Sortable.js index 1c7b2b6..12beb9d 100644 --- a/Sortable.js +++ b/Sortable.js @@ -979,17 +979,11 @@ function _closest(/**HTMLElement*/el, /**String*/selector, /**HTMLElement*/ctx) { if (el) { ctx = ctx || document; - selector = selector.split('.'); - - var tag = selector.shift().toUpperCase(), - re = new RegExp('\\s(' + selector.join('|') + ')(?=\\s)', 'g'); do { if ( - (tag === '>*' && el.parentNode === ctx) || ( - (tag === '' || el.nodeName.toUpperCase() == tag) && - (!selector.length || ((' ' + el.className + ' ').match(re) || []).length == selector.length) - ) + (selector === '>*' && el.parentNode === ctx) + || _matches(el, selector) ) { return el; } @@ -1186,20 +1180,19 @@ } function _matches(/**HTMLElement*/el, /**String*/selector) { - var matches, - i = 0; - - if (el.matches) { - return el.matches(selector); - } else if (el.matchesSelector) { - return el.matchesSelector(selector); - } else { - matches = (el.document || el.ownerDocument).querySelectorAll(selector); - while (matches[i] && matches[i] !== el) { - i++; - } - return matches[i] ? true : false; + if (el) { + selector = selector.split('.'); + + var tag = selector.shift().toUpperCase(), + re = new RegExp('\\s(' + selector.join('|') + ')(?=\\s)', 'g'); + + return ( + (tag === '' || el.nodeName.toUpperCase() == tag) && + (!selector.length || ((' ' + el.className + ' ').match(re) || []).length == selector.length) + ); } + + return false; } function _throttle(callback, ms) {