Skip to content

Commit

Permalink
Merge pull request #941 from plotly/internal-event-handler
Browse files Browse the repository at this point in the history
Mirror events on an internal event handler
  • Loading branch information
rreusser committed Sep 14, 2016
2 parents 668428d + dc9f58d commit 920a333
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 3 deletions.
32 changes: 32 additions & 0 deletions src/lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ var Events = {
if(plotObj._ev instanceof EventEmitter) return plotObj;

var ev = new EventEmitter();
var internalEv = new EventEmitter();

/*
* Assign to plot._ev while we still live in a land
Expand All @@ -32,6 +33,16 @@ var Events = {
*/
plotObj._ev = ev;

/*
* Create a second event handler that will manage events *internally*.
* This allows parts of plotly to respond to thing like relayout without
* having to use the user-facing event handler. They cannot peacefully
* coexist on the same handler because a user invoking
* plotObj.removeAllListeners() would detach internal events, breaking
* plotly.
*/
plotObj._internalEv = internalEv;

/*
* Assign bound methods from the ev to the plot object. These methods
* will reference the 'this' of plot._ev even though they are methods
Expand All @@ -46,6 +57,15 @@ var Events = {
plotObj.removeListener = ev.removeListener.bind(ev);
plotObj.removeAllListeners = ev.removeAllListeners.bind(ev);

/*
* Create funtions for managing internal events. These are *only* triggered
* by the mirroring of external events via the emit function.
*/
plotObj._internalOn = internalEv.on.bind(internalEv);
plotObj._internalOnce = internalEv.once.bind(internalEv);
plotObj._removeInternalListener = internalEv.removeListener.bind(internalEv);
plotObj._removeAllInternalListeners = internalEv.removeAllListeners.bind(internalEv);

/*
* We must wrap emit to continue to support JQuery events. The idea
* is to check to see if the user is using JQuery events, if they are
Expand All @@ -58,6 +78,7 @@ var Events = {
}

ev.emit(event, data);
internalEv.emit(event, data);
};

return plotObj;
Expand All @@ -68,6 +89,10 @@ var Events = {
* all handlers for a particular event and returns the return value
* of the LAST handler. This function also triggers jQuery's
* triggerHandler for backwards compatibility.
*
* Note: triggerHandler has been recommended for deprecation in v2.0.0,
* so the additional behavior of triggerHandler triggering internal events
* is deliberate excluded in order to avoid reinforcing more usage.
*/
triggerHandler: function(plotObj, event, data) {
var jQueryHandlerValue;
Expand Down Expand Up @@ -124,6 +149,13 @@ var Events = {
delete plotObj.removeAllListeners;
delete plotObj.emit;

delete plotObj._ev;
delete plotObj._internalEv;
delete plotObj._internalOn;
delete plotObj._internalOnce;
delete plotObj._removeInternalListener;
delete plotObj._removeAllInternalListeners;

return plotObj;
}

Expand Down
41 changes: 41 additions & 0 deletions test/jasmine/tests/events_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ describe('Events', function() {
$(plotDiv).trigger('ping', 'pong');
});
});

it('mirrors events on an internal handler', function(done) {
Events.init(plotDiv);

plotDiv._internalOn('ping', function(data) {
expect(data).toBe('pong');
done();
});

setTimeout(function() {
plotDiv.emit('ping', 'pong');
});
});
});

describe('triggerHandler', function() {
Expand Down Expand Up @@ -100,6 +113,34 @@ describe('Events', function() {
expect(result).toBe('pong');
});

it('does *not* mirror triggerHandler events on the internal handler', function() {
var eventBaton = 0;
var internalEventBaton = 0;

Events.init(plotDiv);

plotDiv.on('ping', function() {
eventBaton++;
return 'ping';
});

plotDiv._internalOn('ping', function() {
internalEventBaton++;
return 'foo';
});

plotDiv.on('ping', function() {
eventBaton++;
return 'pong';
});

var result = Events.triggerHandler(plotDiv, 'ping');

expect(eventBaton).toBe(2);
expect(internalEventBaton).toBe(0);
expect(result).toBe('pong');
});

it('triggers jQuery handlers when no matching node events bound', function() {
var eventBaton = 0;

Expand Down
7 changes: 4 additions & 3 deletions test/jasmine/tests/plots_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,10 @@ describe('Test Plots', function() {

it('should unset everything in the gd except _context', function() {
var expectedKeys = [
'_ev', 'on', 'once', 'removeListener', 'removeAllListeners',
'emit', '_context', '_replotPending', '_mouseDownTime',
'_hmpixcount', '_hmlumcount'
'_ev', '_internalEv', 'on', 'once', 'removeListener', 'removeAllListeners',
'_internalOn', '_internalOnce', '_removeInternalListener',
'_removeAllInternalListeners', 'emit', '_context', '_replotPending',
'_mouseDownTime', '_hmpixcount', '_hmlumcount'
];

Plots.purge(gd);
Expand Down

0 comments on commit 920a333

Please sign in to comment.