Skip to content

Commit

Permalink
Fix memory leaks from orphaned event listeners
Browse files Browse the repository at this point in the history
  • Loading branch information
@msssk
msssk committed Apr 17, 2020
1 parent 56aaf32 commit 569c8ed
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 31 deletions.
58 changes: 30 additions & 28 deletions ColumnSet.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
});

var colsetidAttr = "data-dgrid-column-set-id";

hasClass("safari", "ie-7");

function adjustScrollLeft(grid, row){
var scrollLefts = grid._columnSetScrollLefts;
function doAdjustScrollLeft(){
Expand All @@ -24,7 +24,7 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
doAdjustScrollLeft();
}
}

function getColumnSetSubRows(subRows, columnSetId){
// Builds a subRow collection that only contains columns that correspond to
// a given column set id.
Expand Down Expand Up @@ -64,7 +64,7 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
return;
}
}

// Normalize reported delta value:
// wheelDeltaX (webkit, mousewheel) needs to be negated and divided by 3
// deltaX (FF17+, wheel) can be used exactly as-is
Expand All @@ -85,22 +85,22 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
});
};
};

return declare(null, {
// summary:
// Provides column sets to isolate horizontal scroll of sets of
// Provides column sets to isolate horizontal scroll of sets of
// columns from each other. This mainly serves the purpose of allowing for
// column locking.

postCreate: function(){
var self = this;
this.inherited(arguments);

this.on(horizMouseWheel(this), function(grid, colsetNode, amount){
var id = colsetNode.getAttribute(colsetidAttr),
scroller = grid._columnSetScrollers[id],
scrollLeft = scroller.scrollLeft + amount;

scroller.scrollLeft = scrollLeft < 0 ? 0 : scrollLeft;
});
this.on('.dgrid-column-set:dgrid-cellfocusin', function (event) {
Expand Down Expand Up @@ -136,24 +136,24 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
adjustScrollLeft(this, row);
return row;
},

renderHeader: function(){
// summary:
// Setup the headers for the grid
this.inherited(arguments);

var columnSets = this.columnSets,
domNode = this.domNode,
scrollers = this._columnSetScrollers,
scrollerContents = this._columnSetScrollerContents = {},
scrollLefts = this._columnSetScrollLefts = {},
grid = this,
i, l;

function reposition(){
grid._positionScrollers();
}

if (scrollers) {
// this isn't the first time; destroy existing scroller nodes first
for(i in scrollers){
Expand All @@ -165,27 +165,27 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
aspect.after(this, "styleColumn", reposition, true);
this._columnSetScrollerNode = put(this.footerNode, "+div.dgrid-column-set-scroller-container");
}

// reset to new object to be populated in loop below
scrollers = this._columnSetScrollers = {};

for(i = 0, l = columnSets.length; i < l; i++){
this._putScroller(columnSets[i], i);
}

this._positionScrollers();
},

styleColumnSet: function(colsetId, css){
// summary:
// Dynamically creates a stylesheet rule to alter a columnset's style.

var rule = this.addCssRule("#" + miscUtil.escapeCssIdentifier(this.domNode.id) +
" .dgrid-column-set-" + miscUtil.escapeCssIdentifier(colsetId, "-"), css);
this._positionScrollers();
return rule;
},

_destroyColumns: function(){
var columnSetsLength = this.columnSets.length,
i, j, k, subRowsLength, len, columnSet, subRow, column;
Expand Down Expand Up @@ -224,26 +224,26 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
scrollerWidth = 0,
numScrollers = 0, // tracks number of visible scrollers (sets w/ overflow)
i, l, columnSetElement, contentWidth;

for(i = 0, l = columnSets.length; i < l; i++){
// iterate through the columnSets
columnSetElement = query('.dgrid-column-set[' + colsetidAttr + '="' + i +'"]', domNode)[0];
scrollerWidth = columnSetElement.offsetWidth;
contentWidth = columnSetElement.firstChild.offsetWidth;
scrollerContents[i].style.width = contentWidth + "px";
scrollers[i].style.width = scrollerWidth + "px";

if(has("ie") < 9){
// IE seems to need scroll to be set explicitly
scrollers[i].style.overflowX = contentWidth > scrollerWidth ? "scroll" : "auto";
}

// Keep track of how many scrollbars we're showing
if(contentWidth > scrollerWidth){ numScrollers++; }
}

this._columnSetScrollerNode.style.bottom = this.showFooter ? this.footerNode.offsetHeight + "px" : "0";

// Align bottom of body node depending on whether there are scrollbars
this.bodyNode.style.bottom = numScrollers ?
(has("dom-scrollbar-height") + (has("ie") ? 1 : 0) + "px") :
Expand All @@ -260,7 +260,9 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
".dgrid-column-set-scroller.dgrid-column-set-scroller-" + i +
"[" + colsetidAttr + "=" + i +"]");
this._columnSetScrollerContents[i] = put(scroller, "div.dgrid-column-set-scroller-content");
listen(scroller, "scroll", lang.hitch(this, '_onColumnSetScroll'));
this._listeners.push(
listen(scroller, "scroll", lang.hitch(this, '_onColumnSetScroll'))
);
},

_onColumnSetScroll: function (evt){
Expand All @@ -285,7 +287,7 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
this._columnSetScrollLefts[colSetId] = newScrollLeft;
}
},

_setColumnSets: function(columnSets){
this._destroyColumns();
this.columnSets = columnSets;
Expand All @@ -295,13 +297,13 @@ function(kernel, declare, lang, Deferred, listen, aspect, query, has, miscUtil,
kernel.deprecated("setColumnSets(...)", 'use set("columnSets", ...) instead', "dgrid 0.4");
this.set("columnSets", columnSets);
},

_scrollColumnSet: function(nodeOrId, offsetLeft){
var id = nodeOrId.tagName ? nodeOrId.getAttribute(colsetidAttr) : nodeOrId;
var scroller = this._columnSetScrollers[id];
scroller.scrollLeft = offsetLeft < 0 ? 0 : offsetLeft;
},

_onColumnSetCellFocus: function(event, columnSetNode){
var focusedNode = event.target;
var columnSetId = columnSetNode.getAttribute(colsetidAttr);
Expand Down
8 changes: 5 additions & 3 deletions Keyboard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,11 @@ var Keyboard = declare(null, {

if(this.tabableHeader){
enableNavigation(this.headerNode);
on(this.headerNode, "dgrid-cellfocusin", function(){
grid.scrollTo({ x: this.scrollLeft });
});
this._listeners.push(
on(this.headerNode, "dgrid-cellfocusin", function(){
grid.scrollTo({ x: this.scrollLeft });
})
);
}
enableNavigation(this.contentNode);

Expand Down

0 comments on commit 569c8ed

Please sign in to comment.