Skip to content

Commit 634cefd

Browse files
committed
Cleanup event handlers in d3 specturm chart.
Moved to name-scoping the handlers, and added a `destroy` function to clean them up. Move no-select on spectrum chart from JS, to just using CSS. This solves some issues was seeing when creating multiple charts in the document at once, and then removing some.
1 parent 9779513 commit 634cefd

File tree

2 files changed

+41
-11
lines changed

2 files changed

+41
-11
lines changed

d3_resources/SpectrumChartD3.css

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ CSS variables InterSpec will define for colors, and line widths:
2222
--d3spec-second-line-color
2323
*/
2424

25+
/* Text selection control class - replaces global document.onselectstart */
26+
.spectrum-chart-no-select {
27+
user-select: none;
28+
-webkit-user-select: none;
29+
-moz-user-select: none;
30+
-ms-user-select: none;
31+
}
32+
2533
/*******************/
2634
svg.SpectrumChartD3 {
2735
/* This is the <svg> element. */
@@ -194,7 +202,7 @@ svg.SpectrumChartD3 {
194202
/* fill: var(--d3spec-text-color, black); */
195203
}
196204

197-
/* This is the box that gives the other kinetic reference candidates */
205+
/* This is the box that gives the other dynamic reference candidates */
198206
.refLineCandidatesBackground {
199207
/* It would be great to use colors defined by global variables, but for the moment we'll
200208
just use the same colors as the legend, since they are okay in both light and dark theme.

d3_resources/SpectrumChartD3.js

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ SpectrumChartD3 = function(elem, options) {
4444

4545
this.chart = typeof elem === 'string' ? document.getElementById(elem) : elem;
4646

47+
// Apply no-select class to prevent text selection during chart interactions
48+
d3.select(this.chart).classed("spectrum-chart-no-select", true);
49+
4750
this.cx = this.chart.clientWidth;
4851
this.cy = this.chart.clientHeight;
4952

@@ -390,10 +393,10 @@ SpectrumChartD3 = function(elem, options) {
390393
;
391394
/// @TODO triggering the cancel events on document.body and window is probably a bit agressive; could probably do this for just this.vis + on leave events
392395
d3.select(document.body)
393-
.on("mouseup", self.handleCancelAllMouseEvents() )
396+
.on("mouseup.chart" + this.chart.id, self.handleCancelAllMouseEvents() )
394397
d3.select(window)
395-
.on("mouseup", self.handleCancelAllMouseEvents())
396-
.on("mousemove", function() {
398+
.on("mouseup.chart" + this.chart.id, self.handleCancelAllMouseEvents())
399+
.on("mousemove.chart" + this.chart.id, function() {
397400
if( d3.event && (self.sliderBoxDown || self.leftDragRegionDown || self.rightDragRegionDown || self.currentlyAdjustingSpectrumScale) ) {
398401
//d3.event.preventDefault();
399402
//d3.event.stopPropagation();
@@ -560,8 +563,31 @@ SpectrumChartD3 = function(elem, options) {
560563
this.kineticRefLineCycleTimer = null;
561564

562565
// Register global keyboard handler
563-
d3.select(window).on("keydown", this.keydown());
564-
}
566+
d3.select(window).on("keydown.chart" + this.chart.id, this.keydown());
567+
}
568+
569+
/** Destructor method to properly clean up global event handlers. Call this before removing/destroying a chart. */
570+
SpectrumChartD3.prototype.destroy = function() {
571+
// Remove global event handlers using the namespaced IDs
572+
d3.select(window).on("keydown.chart" + this.chart.id, null);
573+
d3.select(window).on("mouseup.chart" + this.chart.id, null);
574+
d3.select(window).on("mousemove.chart" + this.chart.id, null);
575+
d3.select(window).on("blur.chart" + this.chart.id, null);
576+
d3.select(document.body).on("mouseup.chart" + this.chart.id, null);
577+
578+
// Clear any active timers - we probably dont actually have to do this, probabilistically, but we will JIC
579+
window.clearTimeout(this.mousewait);
580+
window.clearTimeout(this.touchHold);
581+
window.clearTimeout(this.wheeltimer);
582+
window.clearTimeout(this.roiDragRequestTimeout);
583+
window.clearTimeout(this.kineticRefLineCycleTimer);
584+
window.cancelAnimationFrame(this.zoomAnimationID);
585+
this.mousewait = this.touchHold = this.wheeltimer = this.roiDragRequestTimeout = this.kineticRefLineCycleTimer = this.zoomAnimationID = null;
586+
587+
d3.select(document.body).style("cursor", "default"); // Reset global cursor style
588+
589+
console.log("SpectrumChartD3 instance destroyed: " + this.chart.id);
590+
};
565591

566592

567593

@@ -732,7 +758,6 @@ SpectrumChartD3.prototype.getStaticSvg = function(){
732758
SpectrumChartD3.prototype.dataPointDrag = function() {
733759
var self = this;
734760
return function(d) {
735-
document.onselectstart = function() { return false; };
736761
self.selected = self.dragged = d;
737762
self.update(false); /* boolean set to false to indicate no animation needed */
738763

@@ -3455,7 +3480,6 @@ SpectrumChartD3.prototype.mousemove = function () {
34553480
SpectrumChartD3.prototype.mouseup = function () {
34563481
var self = this;
34573482
return function() {
3458-
document.onselectstart = function() { return true; };
34593483
d3.select('body').style("cursor", "auto");
34603484
d3.select('body').style("cursor", "auto");
34613485
if( self.xaxisdown ) {
@@ -5317,7 +5341,6 @@ SpectrumChartD3.prototype.yaxisDrag = function(d) {
53175341
var self = this;
53185342
return function(d) {
53195343
console.log('yaxisDrag work');
5320-
document.onselectstart = function() { return false; };
53215344
var p = d3.mouse(self.vis[0][0]);
53225345
self.yaxisdown = self.yScale.invert(p[1]);
53235346
}
@@ -5608,7 +5631,6 @@ SpectrumChartD3.prototype.xaxisDrag = function() {
56085631
/*This function is called once when you click on an x-axis label (which you can then start dragging it) */
56095632
/* And NOT when you click on the chart and drag it to pan */
56105633

5611-
document.onselectstart = function() { return false; };
56125634
var p = d3.mouse(self.vis[0][0]);
56135635

56145636
if (self.xScale.invert(p[0]) > 0){ /* set self.xaxisdown equal to value of your mouse pos */
@@ -8260,7 +8282,7 @@ SpectrumChartD3.prototype.updateFeatureMarkers = function( mouseDownEnergy, over
82608282
/* If mouse edge could has been deleted, do not update the mouse edge */
82618283
if (deleteMouseEdge())
82628284
return;
8263-
8285+
console.log( "updateMouseEdge" );
82648286
/* Update the mouse edge and corresponding text position */
82658287
if ( self.mouseEdge ) {
82668288
self.mouseEdge

0 commit comments

Comments
 (0)