Skip to content

Commit 854fed7

Browse files
committed
#1539: Better loop safety
1 parent 528dfc4 commit 854fed7

File tree

5 files changed

+153
-143
lines changed

5 files changed

+153
-143
lines changed

plugins/MultiDrag/MultiDrag.js

Lines changed: 89 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ function MultiDragPlugin() {
5151
setData(dataTransfer, dragEl) {
5252
let data = '';
5353
if (multiDragElements.length && multiDragSortable === sortable) {
54-
for (let i in multiDragElements) {
55-
data += (!i ? '' : ', ') + multiDragElements[i].textContent;
56-
}
54+
multiDragElements.forEach((multiDragElement, i) => {
55+
data += (!i ? '' : ', ') + multiDragElement.textContent;
56+
});
5757
} else {
5858
data = dragEl.textContent;
5959
}
@@ -77,7 +77,7 @@ function MultiDragPlugin() {
7777

7878
setupClone({ sortable }) {
7979
if (!this.isMultiDrag) return;
80-
for (let i in multiDragElements) {
80+
for (let i = 0; i < multiDragElements.length; i++) {
8181
multiDragClones.push(clone(multiDragElements[i]));
8282

8383
multiDragClones[i].sortableIndex = multiDragElements[i].sortableIndex;
@@ -108,9 +108,9 @@ function MultiDragPlugin() {
108108
showClone({ cloneNowShown, rootEl }) {
109109
if (!this.isMultiDrag) return;
110110
insertMultiDragClones(false, rootEl);
111-
for (let i in multiDragClones) {
112-
css(multiDragClones[i], 'display', '');
113-
}
111+
multiDragClones.forEach(clone => {
112+
css(clone, 'display', '');
113+
});
114114

115115
cloneNowShown();
116116
clonesHidden = false;
@@ -119,12 +119,13 @@ function MultiDragPlugin() {
119119

120120
hideClone({ sortable, cloneNowHidden }) {
121121
if (!this.isMultiDrag) return;
122-
for (let i in multiDragClones) {
123-
css(multiDragClones[i], 'display', 'none');
124-
if (sortable.options.removeCloneOnHide && multiDragClones[i].parentNode) {
125-
multiDragClones[i].parentNode.removeChild(multiDragClones[i]);
122+
multiDragClones.forEach(clone => {
123+
css(clone, 'display', 'none');
124+
if (sortable.options.removeCloneOnHide && clone.parentNode) {
125+
clone.parentNode.removeChild(clone);
126126
}
127-
}
127+
});
128+
128129
cloneNowHidden();
129130
clonesHidden = true;
130131
return true;
@@ -135,9 +136,9 @@ function MultiDragPlugin() {
135136
multiDragSortable.multiDrag._deselectMultiDrag();
136137
}
137138

138-
for (let i in multiDragElements) {
139-
multiDragElements[i].sortableIndex = index(multiDragElements[i]);
140-
}
139+
multiDragElements.forEach(multiDragElement => {
140+
multiDragElement.sortableIndex = index(multiDragElement);
141+
});
141142

142143
// Sort multi-drag elements
143144
multiDragElements = multiDragElements.sort(function(a, b) {
@@ -159,17 +160,17 @@ function MultiDragPlugin() {
159160
sortable.captureAnimationState();
160161

161162
if (sortable.options.animation) {
162-
for (let i in multiDragElements) {
163-
if (multiDragElements[i] === dragEl) continue;
164-
css(multiDragElements[i], 'position', 'absolute');
165-
}
163+
multiDragElements.forEach(multiDragElement => {
164+
if (multiDragElement === dragEl) return;
165+
css(multiDragElement, 'position', 'absolute');
166+
});
166167

167168
let dragRect = getRect(dragEl, false, true, true);
168169

169-
for (let i in multiDragElements) {
170-
if (multiDragElements[i] === dragEl) continue;
171-
setRect(multiDragElements[i], dragRect);
172-
}
170+
multiDragElements.forEach(multiDragElement => {
171+
if (multiDragElement === dragEl) return;
172+
setRect(multiDragElement, dragRect);
173+
});
173174

174175
folding = true;
175176
initialFolding = true;
@@ -181,9 +182,9 @@ function MultiDragPlugin() {
181182
initialFolding = false;
182183

183184
if (sortable.options.animation) {
184-
for (let i in multiDragElements) {
185-
unsetRect(multiDragElements[i]);
186-
}
185+
multiDragElements.forEach(multiDragElement => {
186+
unsetRect(multiDragElement);
187+
});
187188
}
188189

189190
// Remove all auxiliary multidrag items from el, if sorting enabled
@@ -202,18 +203,18 @@ function MultiDragPlugin() {
202203
revert({ fromSortable, rootEl, sortable, dragRect }) {
203204
if (multiDragElements.length > 1) {
204205
// Setup unfold animation
205-
for (let i in multiDragElements) {
206+
multiDragElements.forEach(multiDragElement => {
206207
sortable.addAnimationState({
207-
target: multiDragElements[i],
208-
rect: folding ? getRect(multiDragElements[i]) : dragRect
208+
target: multiDragElement,
209+
rect: folding ? getRect(multiDragElement) : dragRect
209210
});
210211

211-
unsetRect(multiDragElements[i]);
212+
unsetRect(multiDragElement);
212213

213-
multiDragElements[i].fromRect = dragRect;
214+
multiDragElement.fromRect = dragRect;
214215

215-
fromSortable.removeAnimationState(multiDragElements[i]);
216-
}
216+
fromSortable.removeAnimationState(multiDragElement);
217+
});
217218
folding = false;
218219
insertMultiDragElements(!sortable.options.removeCloneOnHide, rootEl);
219220
}
@@ -233,14 +234,14 @@ function MultiDragPlugin() {
233234
// Fold: Set all multi drag elements's rects to dragEl's rect when multi-drag elements are invisible
234235
let dragRectAbsolute = getRect(dragEl, false, true, true);
235236

236-
for (let i in multiDragElements) {
237-
if (multiDragElements[i] === dragEl) continue;
238-
setRect(multiDragElements[i], dragRectAbsolute);
237+
multiDragElements.forEach(multiDragElement => {
238+
if (multiDragElement === dragEl) return;
239+
setRect(multiDragElement, dragRectAbsolute);
239240

240241
// Move element(s) to end of parentEl so that it does not interfere with multi-drag clones insertion if they are inserted
241242
// while folding, and so that we can capture them again because old sortable will no longer be fromSortable
242-
parentEl.appendChild(multiDragElements[i]);
243-
}
243+
parentEl.appendChild(multiDragElement);
244+
});
244245

245246
folding = true;
246247
}
@@ -258,15 +259,15 @@ function MultiDragPlugin() {
258259

259260
// Unfold animation for clones if showing from hidden
260261
if (activeSortable.options.animation && !clonesHidden && clonesHiddenBefore) {
261-
for (let i in multiDragClones) {
262+
multiDragClones.forEach(clone => {
262263
activeSortable.addAnimationState({
263-
target: multiDragClones[i],
264+
target: clone,
264265
rect: clonesFromRect
265266
});
266267

267-
multiDragClones[i].fromRect = clonesFromRect;
268-
multiDragClones[i].thisAnimationDuration = null;
269-
}
268+
clone.fromRect = clonesFromRect;
269+
clone.thisAnimationDuration = null;
270+
});
270271
}
271272
} else {
272273
activeSortable._showClone(sortable);
@@ -276,9 +277,9 @@ function MultiDragPlugin() {
276277
},
277278

278279
dragOverAnimationCapture({ dragRect, isOwner, activeSortable }) {
279-
for (let i in multiDragElements) {
280-
multiDragElements[i].thisAnimationDuration = null;
281-
}
280+
multiDragElements.forEach(multiDragElement => {
281+
multiDragElement.thisAnimationDuration = null;
282+
});
282283

283284
if (activeSortable.options.animation && !isOwner && activeSortable.multiDrag.isMultiDrag) {
284285
clonesFromRect = Object.assign({}, dragRect);
@@ -383,45 +384,45 @@ function MultiDragPlugin() {
383384
if (!initialFolding) {
384385
if (options.animation) {
385386
dragEl.fromRect = dragRect;
386-
for (let i in multiDragElements) {
387-
multiDragElements[i].thisAnimationDuration = null;
388-
if (multiDragElements[i] !== dragEl) {
389-
let rect = folding ? getRect(multiDragElements[i]) : dragRect;
390-
multiDragElements[i].fromRect = rect;
387+
multiDragElements.forEach(multiDragElement => {
388+
multiDragElement.thisAnimationDuration = null;
389+
if (multiDragElement !== dragEl) {
390+
let rect = folding ? getRect(multiDragElement) : dragRect;
391+
multiDragElement.fromRect = rect;
391392

392393
// Prepare unfold animation
393394
toSortable.addAnimationState({
394-
target: multiDragElements[i],
395+
target: multiDragElement,
395396
rect: rect
396397
});
397398
}
398-
}
399+
});
399400
}
400401

401402
// Multi drag elements are not necessarily removed from the DOM on drop, so to reinsert
402403
// properly they must all be removed
403404
removeMultiDragElements();
404405

405-
for (let i in multiDragElements) {
406+
multiDragElements.forEach(multiDragElement => {
406407
if (children[multiDragIndex]) {
407-
parentEl.insertBefore(multiDragElements[i], children[multiDragIndex]);
408+
parentEl.insertBefore(multiDragElement, children[multiDragIndex]);
408409
} else {
409-
parentEl.appendChild(multiDragElements[i]);
410+
parentEl.appendChild(multiDragElement);
410411
}
411412
multiDragIndex++;
412-
}
413+
});
413414

414415
// If initial folding is done, the elements may have changed position because they are now
415416
// unfolding around dragEl, even though dragEl may not have his index changed, so update event
416417
// must be fired here as Sortable will not.
417418
if (oldIndex === index(dragEl)) {
418419
let update = false;
419-
for (let i in multiDragElements) {
420-
if (multiDragElements[i].sortableIndex !== index(multiDragElements[i])) {
420+
multiDragElements.forEach(multiDragElement => {
421+
if (multiDragElement.sortableIndex !== index(multiDragElement)) {
421422
update = true;
422-
break;
423+
return;
423424
}
424-
}
425+
});
425426

426427
if (update) {
427428
dispatchSortableEvent('update');
@@ -430,9 +431,9 @@ function MultiDragPlugin() {
430431
}
431432

432433
// Must be done after capturing individual rects (scroll bar)
433-
for (let i in multiDragElements) {
434-
unsetRect(multiDragElements[i]);
435-
}
434+
multiDragElements.forEach(multiDragElement => {
435+
unsetRect(multiDragElement);
436+
});
436437

437438
toSortable.animateAll();
438439
}
@@ -442,9 +443,9 @@ function MultiDragPlugin() {
442443

443444
// Remove clones if necessary
444445
if (rootEl === parentEl || (putSortable && putSortable.lastPutMode !== 'clone')) {
445-
for (let i in multiDragClones) {
446-
multiDragClones[i].parentNode && multiDragClones[i].parentNode.removeChild(multiDragClones[i]);
447-
}
446+
multiDragClones.forEach(clone => {
447+
clone.parentNode && clone.parentNode.removeChild(clone);
448+
});
448449
}
449450
},
450451

@@ -537,23 +538,23 @@ function MultiDragPlugin() {
537538
const oldIndicies = [],
538539
newIndicies = [];
539540

540-
multiDragElements.forEach((element) => {
541+
multiDragElements.forEach(multiDragElement => {
541542
oldIndicies.push({
542-
element,
543-
index: element.sortableIndex
543+
multiDragElement,
544+
index: multiDragElement.sortableIndex
544545
});
545546

546547
// multiDragElements will already be sorted if folding
547548
let newIndex;
548-
if (folding && element !== dragEl) {
549+
if (folding && multiDragElement !== dragEl) {
549550
newIndex = -1;
550551
} else if (folding) {
551-
newIndex = index(element, ':not(.' + this.options.selectedClass + ')');
552+
newIndex = index(multiDragElement, ':not(.' + this.options.selectedClass + ')');
552553
} else {
553-
newIndex = index(element);
554+
newIndex = index(multiDragElement);
554555
}
555556
newIndicies.push({
556-
element,
557+
multiDragElement,
557558
index: newIndex
558559
});
559560
});
@@ -579,14 +580,14 @@ function MultiDragPlugin() {
579580
}
580581

581582
function insertMultiDragElements(clonesInserted, rootEl) {
582-
for (let i in multiDragElements) {
583-
let target = rootEl.children[multiDragElements[i].sortableIndex + (clonesInserted ? Number(i) : 0)];
583+
multiDragElements.forEach(multiDragElement => {
584+
let target = rootEl.children[multiDragElement.sortableIndex + (clonesInserted ? Number(i) : 0)];
584585
if (target) {
585-
rootEl.insertBefore(multiDragElements[i], target);
586+
rootEl.insertBefore(multiDragElement, target);
586587
} else {
587-
rootEl.appendChild(multiDragElements[i]);
588+
rootEl.appendChild(multiDragElement);
588589
}
589-
}
590+
});
590591
}
591592

592593
/**
@@ -595,21 +596,21 @@ function insertMultiDragElements(clonesInserted, rootEl) {
595596
* @param {HTMLElement} rootEl
596597
*/
597598
function insertMultiDragClones(elementsInserted, rootEl) {
598-
for (let i in multiDragClones) {
599-
let target = rootEl.children[multiDragClones[i].sortableIndex + (elementsInserted ? Number(i) : 0)];
599+
multiDragClones.forEach(clone => {
600+
let target = rootEl.children[clone.sortableIndex + (elementsInserted ? Number(i) : 0)];
600601
if (target) {
601-
rootEl.insertBefore(multiDragClones[i], target);
602+
rootEl.insertBefore(clone, target);
602603
} else {
603-
rootEl.appendChild(multiDragClones[i]);
604+
rootEl.appendChild(clone);
604605
}
605-
}
606+
});
606607
}
607608

608609
function removeMultiDragElements() {
609-
for (let i in multiDragElements) {
610-
if (multiDragElements[i] === dragEl) continue;
611-
multiDragElements[i].parentNode && multiDragElements[i].parentNode.removeChild(multiDragElements[i]);
612-
}
610+
multiDragElements.forEach(multiDragElement => {
611+
if (multiDragElement === dragEl) return;
612+
multiDragElement.parentNode && multiDragElement.parentNode.removeChild(multiDragElement);
613+
});
613614
}
614615

615616
export default MultiDragPlugin;

0 commit comments

Comments
 (0)