Skip to content

Commit

Permalink
Fix #1157: Tree: avoid duplicate rendering in _configureTreeColumn
Browse files Browse the repository at this point in the history
This addresses cases where the same column definition object is passed
to one of the structure-related setters, as is the case when using
ColumnReorder.

Thanks to @dmartinzar for the bug report and original example + fix, and
@maier49 for the initial unit test.

Resolves #1185.

(Manually ported from 0f67fa1)
  • Loading branch information
Kenneth G. Franqueiro committed Dec 17, 2015
1 parent a2629ac commit 28c4786
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 26 deletions.
55 changes: 29 additions & 26 deletions Tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,35 @@ define([
// summary:
// Adds tree navigation capability to a column.

var originalRenderCell = column.renderCell || this._defaultRenderCell;
if (!column._isConfiguredTreeColumn) {
var originalRenderCell = column.renderCell || this._defaultRenderCell;
column._isConfiguredTreeColumn = true;
column.renderCell = function (object, value, td, options) {
// summary:
// Renders a cell that can be expanded, creating more rows

var grid = column.grid,
level = Number(options && options.queryLevel) + 1,
mayHaveChildren = !grid.collection.mayHaveChildren || grid.collection.mayHaveChildren(object),
expando, node;

level = grid._currentLevel = isNaN(level) ? 0 : level;
expando = column.renderExpando(level, mayHaveChildren,
grid._expanded[grid.collection.getIdentity(object)], object);
expando.level = level;
expando.mayHaveChildren = mayHaveChildren;

node = originalRenderCell.call(column, object, value, td, options);
if (node && node.nodeType) {
put(td, expando);
put(td, node);
}
else {
td.insertBefore(expando, td.firstChild);
}
};
}

var clicked; // tracks row that was clicked (for expand dblclick event handling)

this._treeColumn = column;
Expand Down Expand Up @@ -339,31 +367,6 @@ define([
grid.expand(this);
}));
}

column.renderCell = function (object, value, td, options) {
// summary:
// Renders a cell that can be expanded, creating more rows

var grid = column.grid,
level = Number(options && options.queryLevel) + 1,
mayHaveChildren = !grid.collection.mayHaveChildren || grid.collection.mayHaveChildren(object),
expando, node;

level = grid._currentLevel = isNaN(level) ? 0 : level;
expando = column.renderExpando(level, mayHaveChildren,
grid._expanded[grid.collection.getIdentity(object)], object);
expando.level = level;
expando.mayHaveChildren = mayHaveChildren;

node = originalRenderCell.call(column, object, value, td, options);
if (node && node.nodeType) {
put(td, expando);
put(td, node);
}
else {
td.insertBefore(expando, td.firstChild);
}
};
},

_defaultRenderExpando: function (level, hasChildren, expanded) {
Expand Down
12 changes: 12 additions & 0 deletions test/intern/mixins/Tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,18 @@ define([
assert.isDefined(inputNode, 'Row ' + i + ' should have an input node');
}
});

// Test goal: ensure renderCell does not get re-wrapped over itself if the same column definition object
// is received during a column reset (e.g. via ColumnReorder). See #1157
test.test('renderCell after same structure is recomputed', function () {
function countRowExpandos() {
return grid.row('0').element.querySelectorAll('.dgrid-expando-icon').length;
}

assert.strictEqual(countRowExpandos(), 1, 'Each parent row should have one expando icon');
grid.set('columns', grid.get('columns'));
assert.strictEqual(countRowExpandos(), 1,'Each parent row should still have only one expando icon');
});
});

test.suite('large family expansion without sort', function () {
Expand Down

0 comments on commit 28c4786

Please sign in to comment.