Skip to content

Commit

Permalink
Fix #1211: Track and clean up editor listeners on a per-row/cell basis
Browse files Browse the repository at this point in the history
  • Loading branch information
maier49 authored and Kenneth G. Franqueiro committed Jan 6, 2016
1 parent 2b8c8f9 commit 8c3d0c6
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 12 deletions.
89 changes: 77 additions & 12 deletions Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ define([
return declare(null, {
constructor: function () {
this._editorInstances = {};
// Tracks shared editor dismissal listeners, and editor click/change listeners for old IE
this._editorColumnListeners = [];
// Tracks always-on editor listeners for old IE, or listeners for triggering shared editors
this._editorCellListeners = {};
this._editorsPendingStartup = [];
},

Expand All @@ -33,8 +36,19 @@ define([
},

insertRow: function () {
this._editorRowListeners = {};
var rowElement = this.inherited(arguments);
var row = this.row(rowElement);
var rowListeners = this._editorCellListeners[rowElement.id] =
this._editorCellListeners[rowElement.id] || {};

for (var key in this._editorRowListeners) {
rowListeners[key] = this._editorRowListeners[key];
}
// Null this out so that _createEditor can tell whether the editor being created is
// an individual cell editor at insertion time, vs. a cell being refreshed
this._editorRowListeners = null;

var previouslyFocusedCell = this._previouslyFocusedEditorCell;

if (previouslyFocusedCell && previouslyFocusedCell.row.id === row.id) {
Expand Down Expand Up @@ -74,6 +88,13 @@ define([
}, 0);
}

if (this._editorCellListeners[rowElement.id]) {
for (var columnId in this._editorCellListeners[rowElement.id]) {
this._editorCellListeners[rowElement.id][columnId].remove();
}
delete this._editorCellListeners[rowElement.id];
}

for (var i = this._alwaysOnWidgetColumns.length; i--;) {
// Destroy always-on editor widgets during the row removal operation,
// but don't trip over loading nodes from incomplete requests
Expand Down Expand Up @@ -130,6 +151,18 @@ define([
for (var i = listeners.length; i--;) {
listeners[i].remove();
}

for (var rowId in this._editorCellListeners) {
for (columnId in this._editorCellListeners[rowId]) {
this._editorCellListeners[rowId][columnId].remove();
}
}

for (i = 0; i < this._editorColumnListeners.length; i++) {
this._editorColumnListeners[i].remove();
}

this._editorCellListeners = {};
this._editorColumnListeners = [];
this._editorsPendingStartup = [];
},
Expand Down Expand Up @@ -171,12 +204,19 @@ define([
// which we already advocate in docs for optimal use)

if (!options || !options.alreadyHooked) {
self._editorColumnListeners.push(
on(cell, editOn, function () {
self._activeOptions = options;
self.edit(this);
})
);
var listener = on(cell, editOn, function () {
self._activeOptions = options;
self.edit(this);
});
if (self._editorRowListeners) {
self._editorRowListeners[column.id] = listener;
}
else {
// We're in refreshCell since _editorRowListeners doesn't exist,
// so the row should exist
var row = self.row(object);
self._editorCellListeners[row.element.id][column.id] = listener;
}
}

// initially render content in non-edit mode
Expand All @@ -185,7 +225,8 @@ define([
} : function (object, value, cell, options) {
// always-on: create editor immediately upon rendering each cell
if (!column.canEdit || column.canEdit(object, value)) {
var cmp = self._createEditor(column);
// _createEditor also needs the object for when this is invoked via refreshCell, to get the row
var cmp = self._createEditor(column, object);
self._showEditor(cmp, column, cell, value);
// Maintain reference for later use.
cell[isWidget ? 'widget' : 'input'] = cmp;
Expand Down Expand Up @@ -293,6 +334,16 @@ define([
return null;
},

refreshCell: function(cell) {
var rowElementId = cell.row.element.id;
var columnId = cell.column.id;
if (this._editorCellListeners[rowElementId] && this._editorCellListeners[rowElementId][columnId]) {
this._editorCellListeners[rowElementId][columnId].remove();
this._editorCellListeners[rowElementId][columnId] = null;
}
return this.inherited(arguments);
},

_showEditor: function (cmp, column, cellElement, value) {
// Places a shared editor into the newly-active cell in the column.
// Also called when rendering an editor in an "always-on" editor column.
Expand Down Expand Up @@ -376,7 +427,7 @@ define([
}
},

_createEditor: function (column) {
_createEditor: function (column, object) {
// Creates an editor instance based on column definition properties,
// and hooks up events.
var editor = column.editor,
Expand Down Expand Up @@ -436,16 +487,30 @@ define([
if (has('ie') < 9) {
// IE<9 doesn't fire change events for all the right things,
// and it doesn't bubble.
var listener;
if (editor === 'radio' || editor === 'checkbox') {
// listen for clicks since IE doesn't fire change events properly for checks/radios
this._editorColumnListeners.push(on(cmp, 'click', function (evt) {
listener = on(cmp, 'click', function (evt) {
self._handleEditorChange(evt, column);
}));
});
}
else {
this._editorColumnListeners.push(on(cmp, 'change', function (evt) {
listener = on(cmp, 'change', function (evt) {
self._handleEditorChange(evt, column);
}));
});
}

if (editOn) {
// Shared editor handlers are maintained in _editorColumnListeners, since they're not per-row
this._editorColumnListeners.push(listener);
}
else if (this._editorRowListeners) {
this._editorRowListeners[column.id] = listener;
}
// If editRowListeners doesn't exist and this is an always-on editor,
// then we're here from renderCell via refreshCell, and the row should exist
else {
this._editorCellListeners[this.row(object).element.id][column.id] = listener;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions _StoreMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ define([
},

refreshCell: function (cell) {
this.inherited(arguments);
var row = cell.row;
var self = this;

Expand Down

0 comments on commit 8c3d0c6

Please sign in to comment.