Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Disable listeners on hide #1232

Merged
merged 3 commits into from

2 participants

@mattpardee

This doesn't solve other problems recently noticed with mm, just solves the listener-removal-when-disabled issues

Fixes #1224

@sergi sergi was assigned
@sergi sergi commented on the diff
client/ext/minimap/map.js
((5 lines not shown))
this.ace = ace;
this.c = c;
this.visor = visor;
- this.visibleLines = this.ace.$getVisibleRowCount();
- this.visorHeight = Map.toHeight(this.visibleLines);
@sergi
sergi added a note

Why do these lines go away?

Ah, I neglected to add them to the updateSource - they're there now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sergi sergi commented on the diff
client/ext/minimap/minimap.js
@@ -162,12 +163,17 @@ return module.exports = ext.register("ext/minimap/minimap", {
* @see this.disable()
*/
hide : function(noSetMapEnabled) {
+ if (this.$changeEvent)
+ this.editorSession.removeEventListener("change", this.$changeEvent);
+ this.map.disableListeners();
@sergi
sergi added a note

Shouldn't the destroy method also have these lines above?

destroy now calls hide()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sergi sergi merged commit 932c36a into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 34 additions and 17 deletions.
  1. +26 −15 client/ext/minimap/map.js
  2. +8 −2 client/ext/minimap/minimap.js
View
41 client/ext/minimap/map.js
@@ -93,33 +93,32 @@ var Map = (function() {
};
function Map(ace, c, visor) {
- var _self = this;
this.ace = ace;
this.c = c;
this.visor = visor;
- this.visibleLines = this.ace.$getVisibleRowCount();
- this.visorHeight = Map.toHeight(this.visibleLines);
@sergi
sergi added a note

Why do these lines go away?

Ah, I neglected to add them to the updateSource - they're there now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
this.ctx = c.getContext("2d");
this.ctx.fillStyle = "#000";
this.ctx.fillRect(0, 0, this.c.width, this.c.height);
this.visorTop = 0;
this.inVisor = false;
this.mousedown = false;
+ }
- this.ace.renderer.scrollBar.addEventListener("scroll", function() {
+ Map.prototype.enableListeners = function() {
+ var _self = this;
+
+ this.ace.renderer.scrollBar.addEventListener("scroll", this.$scrollListener = function() {
_self.afterScroll();
});
- var session = this.ace.getSession();
-
- visor.addEventListener("mousedown", function(e) {
+ this.visor.addEventListener("mousedown", this.$visorMouseDown = function(e) {
_self.inVisor = true;
_self.visorDiff = e.offsetY || e.layerY;
_self.mousedown = _self.visorDiff + _self.visorTop;
_self.containerTop = apf.getAbsolutePosition(_self.c)[1];
}, false);
- document.addEventListener("mousemove", function(e) {
+ document.addEventListener("mousemove", this.$docMouseMove = function(e) {
if (_self.inVisor && _self.mousedown !== false) {
_self.visorTop = (e.pageY - _self.containerTop) - _self.visorDiff;
_self.normal = _self.getNormal();
@@ -127,19 +126,33 @@ var Map = (function() {
}
}, false);
- document.addEventListener("mouseup", function(e) {
- if (!_self.inVisor && e.target === c) {
+ document.addEventListener("mouseup", this.$docMouseUp = function(e) {
+ if (!_self.inVisor && e.target === _self.c) {
_self.visorTop = (e.offsetY || e.layerY) - (_self.visorHeight / 2);
_self.normal = _self.getNormal();
_self.render(true);
}
_self.mousedown = _self.inVisor = false;
}, false);
+ };
- this.updateSource(session);
- }
+ Map.prototype.disableListeners = function() {
+ if (this.$scrollListener)
+ this.ace.renderer.scrollBar.removeEventListener("scroll", this.$scrollListener);
+
+ if (this.$docMouseUp)
+ document.removeEventListener("mouseup", this.$docMouseUp);
+
+ if (this.$docMouseMove)
+ document.removeEventListener("mousemove", this.$docMouseMove);
+
+ if (this.$visorMouseDown)
+ this.visor.removeEventListener("mousedown", this.$visorMouseDown);
+ };
Map.prototype.updateSource = function(session) {
+ this.visibleLines = this.ace.$getVisibleRowCount();
+ this.visorHeight = Map.toHeight(this.visibleLines);
this.lines = session.getLines(0, session.getLength() - 1);
this.actualHeight = Map.toHeight(this.lines.length);
this.codeCanvas = Map.storeCanvas(this.c.width, this.actualHeight, this.lines);
@@ -188,10 +201,8 @@ var Map = (function() {
Map.prototype.destroy = function() {
this.lines = null;
this.pixelData = null;
+ this.disableListeners();
this.ctx.clearRect(0, 0, this.c.width, this.c.height);
- this.c.removeEventListener("mousedown");
- this.c.removeEventListener("mousemove");
- this.c.removeEventListener("mouseup");
this.c = this.ctx = this.ace = this.codeCanvas = null;
};
View
10 client/ext/minimap/minimap.js
@@ -110,8 +110,6 @@ return module.exports = ext.register("ext/minimap/minimap", {
_self.show();
});
}
-
- this.setupChangeListener();
},
setupChangeListener : function() {
@@ -144,6 +142,9 @@ return module.exports = ext.register("ext/minimap/minimap", {
},
show : function() {
+ this.setupChangeListener();
+ this.map.enableListeners();
+
this.editor.container.style.right = this.map_width + "px";
this.panel.show();
this.updateMap();
@@ -162,6 +163,10 @@ return module.exports = ext.register("ext/minimap/minimap", {
* @see this.disable()
*/
hide : function(noSetMapEnabled) {
+ if (this.$changeEvent)
+ this.editorSession.removeEventListener("change", this.$changeEvent);
+ this.map.disableListeners();
@sergi
sergi added a note

Shouldn't the destroy method also have these lines above?

destroy now calls hide()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
this.panel.hide();
this.editor.container.style.right = "0";
@@ -198,6 +203,7 @@ return module.exports = ext.register("ext/minimap/minimap", {
this.nodes.each(function(item) {
return item.destroy();
});
+ this.hide();
this.nodes = [];
this.map.destroy();
this.map = null;
Something went wrong with that request. Please try again.