Permalink
Browse files

Arrays: drag-and-drop switched items. Should have moved dragged item.

A drag-and-drop within arrays should move the item being dragged
to the drop position, but JSON Form actually switched the item with
the item at the drop position. That's a possibility but definitely
not what the user would expect from a drag-and-drop.

This commit fixes the bug, both for arrays and tabarrays.
  • Loading branch information...
1 parent ddb412f commit e02774b5726e790fd558cc271457affd42679cb6 François Daoust committed Aug 7, 2012
Showing with 72 additions and 15 deletions.
  1. +34 −13 lib/jsonform.js
  2. +36 −0 tests/array/t.js
  3. +1 −1 tests/index.html
  4. +1 −1 tests/runner.js
View
@@ -572,16 +572,26 @@ jsonform.elementTypes = {
},
'onInsert': function (evt, node) {
// Switch two nodes in an array
- var switchNodes = function (fromIdx, toIdx) {
- var parentEl = $('#' + escapeSelector(node.id) + ' ul');
+ var moveNodeTo = function (fromIdx, toIdx) {
+ // Note "switchValuesWith" extracts values from the DOM since field
+ // values are not synchronized with the tree data structure, so calls
+ // to render are needed at each step to force values down to the DOM
+ // before next move.
+ // TODO: synchronize field values and data structure completely and
+ // call render only once to improve efficiency.
if (fromIdx === toIdx) return;
- node.children[fromIdx].switchValuesWith(node.children[toIdx]);
- node.children[fromIdx].render(parentEl.get(0));
- node.children[toIdx].render(parentEl.get(0));
+ var incr = (fromIdx < toIdx) ? 1: -1;
+ var i = 0;
+ var parentEl = $('#' + escapeSelector(node.id) + ' ul');
+ for (i = fromIdx; i !== toIdx; i += incr) {
+ node.children[i].switchValuesWith(node.children[i + incr]);
+ node.children[i].render(parentEl.get(0));
+ node.children[i + incr].render(parentEl.get(0));
+ }
// No simple way to prevent DOM reordering with jQuery UI Sortable,
// so we're going to need to move sorted DOM elements back to their
- // origin position in the DOM ourselves (we swtich values but not
+ // origin position in the DOM ourselves (we switched values but not
// DOM elements)
var fromEl = $(node.children[fromIdx].el);
var toEl = $(node.children[toIdx].el);
@@ -618,7 +628,7 @@ jsonform.elementTypes = {
$(node.el).find('ul').bind('sortstop', function (event, ui) {
var idx = $(ui.item).data('idx');
var newIdx = $(ui.item).index();
- switchNodes(idx, newIdx);
+ moveNodeTo(idx, newIdx);
});
}
}
@@ -657,11 +667,22 @@ jsonform.elementTypes = {
data.tabs = tabs;
},
'onInsert': function (evt, node) {
- // Switch two nodes in an array
- var switchNodes = function (fromIdx, toIdx) {
- node.children[fromIdx].switchValuesWith(node.children[toIdx]);
- node.children[fromIdx].render($('.tab-content', $('#' + escapeSelector(node.id))).get(0));
- node.children[toIdx].render($('.tab-content', $('#' + escapeSelector(node.id))).get(0));
+ var moveNodeTo = function (fromIdx, toIdx) {
+ // Note "switchValuesWith" extracts values from the DOM since field
+ // values are not synchronized with the tree data structure, so calls
+ // to render are needed at each step to force values down to the DOM
+ // before next move.
+ // TODO: synchronize field values and data structure completely and
+ // call render only once to improve efficiency.
+ if (fromIdx === toIdx) return;
+ var incr = (fromIdx < toIdx) ? 1: -1;
+ var i = 0;
+ var tabEl = $('.tab-content', $('#' + escapeSelector(node.id))).get(0);
+ for (i = fromIdx; i !== toIdx; i += incr) {
+ node.children[i].switchValuesWith(node.children[i + incr]);
+ node.children[i].render(tabEl);
+ node.children[i + incr].render(tabEl);
+ }
};
// Refreshes the list of tabs
@@ -720,7 +741,7 @@ jsonform.elementTypes = {
$(node.el).find('.nav-tabs').bind('sortstop', function (event, ui) {
var idx = $(ui.item).data('idx');
var newIdx = $(ui.item).index();
- switchNodes(idx, newIdx);
+ moveNodeTo(idx, newIdx);
updateTabs(newIdx);
});
}
View
@@ -126,5 +126,41 @@ var tests = [
}
}
}
+ },
+ {
+ name: 'Value as legend',
+ jsonform: {
+ schema: {
+ arr: {
+ type: 'array',
+ title: 'An array',
+ items: {
+ type: 'string',
+ title: 'Array item',
+ maxLength: 15
+ }
+ }
+ },
+ form: [
+ {
+ type: 'array',
+ items: [
+ {
+ type: 'fieldset',
+ title: 'Number {{idx}}',
+ legend: '{{idx}}. {{value}}',
+ items: [
+ {
+ key: 'arr[]',
+ title: 'Item {{idx}}',
+ value: 'Hello number {{idx}}',
+ valueInLegend: true
+ }
+ ]
+ }
+ ]
+ }
+ ]
+ }
}
];
View
@@ -64,7 +64,7 @@
<!-- <script src="value/t.js"></script> -->
<!-- <script src="wysihtml5/t.js"></script> -->
- <script src="checkbox/t.js"></script>
+ <script src="array/t.js"></script>
<script src="runner.js"></script>
</body>
View
@@ -1,4 +1,4 @@
-var idx = 2;
+var idx = 5;
if (!tests[idx].jsonform.form) {
tests[idx].jsonform.form = ['*'];
}

0 comments on commit e02774b

Please sign in to comment.