From edae9fc3edce9c9443d0d8c6e3f8225513db14b2 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 14 Sep 2016 16:16:58 -0400 Subject: [PATCH 1/3] Mirror events on an internal event handler --- src/lib/events.js | 45 ++++++++++++++++++++++++++++++- test/jasmine/tests/events_test.js | 41 ++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/lib/events.js b/src/lib/events.js index 8b8d63d5614..d48d8624cab 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -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 @@ -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 @@ -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 @@ -58,6 +78,7 @@ var Events = { } ev.emit(event, data); + internalEv.emit(event, data); }; return plotObj; @@ -70,6 +91,7 @@ var Events = { * triggerHandler for backwards compatibility. */ triggerHandler: function(plotObj, event, data) { + var i; var jQueryHandlerValue; var nodeEventHandlerValue; /* @@ -98,10 +120,24 @@ var Events = { /* * Call all the handlers except the last one. */ - for(var i = 0; i < handlers.length; i++) { + for(i = 0; i < handlers.length; i++) { handlers[i](data); } + /* Do the same as for external-facing events, except trigger the same + * events on the internal handlers. This does *not* affect the return + * value. It simply mirrors triggers internally so that there's no + * conflict with external user-defined events when plotly manages + * events internally. + */ + var internalHandlers = plotObj._internalEv._events[event]; + if (internalHandlers) { + if(typeof internalHandlers === 'function') internalHandlers = [internalHandlers]; + for(i = 0; i < internalHandlers.length; i++) { + internalHandlers[i](data); + } + } + /* * Now call the final handler and collect its value */ @@ -124,6 +160,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; } diff --git a/test/jasmine/tests/events_test.js b/test/jasmine/tests/events_test.js index ecf3c0ba4fe..c60f5f1f6ab 100644 --- a/test/jasmine/tests/events_test.js +++ b/test/jasmine/tests/events_test.js @@ -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() { @@ -100,6 +113,34 @@ describe('Events', function() { expect(result).toBe('pong'); }); + it('mirrors 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(1); + expect(result).toBe('pong'); + }); + it('triggers jQuery handlers when no matching node events bound', function() { var eventBaton = 0; From 66346bc610cdad196c3ec09f448aa599ff6fa03b Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 14 Sep 2016 16:41:33 -0400 Subject: [PATCH 2/3] Fix plots test to work with internal event handler --- src/lib/events.js | 4 ++-- test/jasmine/tests/plots_test.js | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index d48d8624cab..2283cde863b 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -131,7 +131,7 @@ var Events = { * events internally. */ var internalHandlers = plotObj._internalEv._events[event]; - if (internalHandlers) { + if(internalHandlers) { if(typeof internalHandlers === 'function') internalHandlers = [internalHandlers]; for(i = 0; i < internalHandlers.length; i++) { internalHandlers[i](data); @@ -165,7 +165,7 @@ var Events = { delete plotObj._internalOn; delete plotObj._internalOnce; delete plotObj._removeInternalListener; - delete plotObj._removeAllInternalListeners;; + delete plotObj._removeAllInternalListeners; return plotObj; } diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 4f9284b3753..dad7cf6e8e0 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -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); From dc9f58de92f9144d31916fa43711fb1b237640b7 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 14 Sep 2016 17:22:25 -0400 Subject: [PATCH 3/3] Disconnect triggerHandler from internal events --- src/lib/events.js | 21 +++++---------------- test/jasmine/tests/events_test.js | 4 ++-- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index 2283cde863b..2d7cbd65908 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -89,9 +89,12 @@ 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 i; var jQueryHandlerValue; var nodeEventHandlerValue; /* @@ -120,24 +123,10 @@ var Events = { /* * Call all the handlers except the last one. */ - for(i = 0; i < handlers.length; i++) { + for(var i = 0; i < handlers.length; i++) { handlers[i](data); } - /* Do the same as for external-facing events, except trigger the same - * events on the internal handlers. This does *not* affect the return - * value. It simply mirrors triggers internally so that there's no - * conflict with external user-defined events when plotly manages - * events internally. - */ - var internalHandlers = plotObj._internalEv._events[event]; - if(internalHandlers) { - if(typeof internalHandlers === 'function') internalHandlers = [internalHandlers]; - for(i = 0; i < internalHandlers.length; i++) { - internalHandlers[i](data); - } - } - /* * Now call the final handler and collect its value */ diff --git a/test/jasmine/tests/events_test.js b/test/jasmine/tests/events_test.js index c60f5f1f6ab..d4cc5bf423a 100644 --- a/test/jasmine/tests/events_test.js +++ b/test/jasmine/tests/events_test.js @@ -113,7 +113,7 @@ describe('Events', function() { expect(result).toBe('pong'); }); - it('mirrors events on the internal handler', function() { + it('does *not* mirror triggerHandler events on the internal handler', function() { var eventBaton = 0; var internalEventBaton = 0; @@ -137,7 +137,7 @@ describe('Events', function() { var result = Events.triggerHandler(plotDiv, 'ping'); expect(eventBaton).toBe(2); - expect(internalEventBaton).toBe(1); + expect(internalEventBaton).toBe(0); expect(result).toBe('pong'); });