From 8e2d1fe7f5539a78cdba2223c20ee512ca4677f5 Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 25 Apr 2018 14:44:12 -0400 Subject: [PATCH 1/8] rename fail -> failTest --- test/jasmine/tests/legend_test.js | 35 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index a216412c658..39d02394279 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -9,12 +9,11 @@ var helpers = require('@src/components/legend/helpers'); var anchorUtils = require('@src/components/legend/anchor_utils'); var d3 = require('d3'); -var fail = require('../assets/fail_test'); +var failTest = require('../assets/fail_test'); var delay = require('../assets/delay'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); - describe('legend defaults', function() { 'use strict'; @@ -538,7 +537,7 @@ describe('legend relayout update', function() { .then(function() { expect(d3.selectAll('g.legend').size()).toBe(1); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -575,7 +574,7 @@ describe('legend relayout update', function() { }).then(function() { assertLegendStyle('rgb(0, 0, 255)', 'rgb(255, 0, 0)', 10); }) - .catch(fail) + .catch(failTest) .then(done); }); }); @@ -907,7 +906,7 @@ describe('legend interaction', function() { .then(function() { assertVisible(gd, [true, true, true, true]); }) - .catch(fail) + .catch(failTest) .then(done); }); }); @@ -976,7 +975,7 @@ describe('legend interaction', function() { {target: 3}, {target: 4} ]); - }).catch(fail).then(done); + }).catch(failTest).then(done); }); }); @@ -1023,7 +1022,7 @@ describe('legend interaction', function() { .then(assertVisible([false, 'legendonly', true])) .then(click(0)) .then(assertVisible([false, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('clicking once toggles true -> legendonly', function(done) { @@ -1031,7 +1030,7 @@ describe('legend interaction', function() { .then(assertVisible([false, 'legendonly', true])) .then(click(1)) .then(assertVisible([false, 'legendonly', 'legendonly'])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('double-clicking isolates a visible trace ', function(done) { @@ -1040,14 +1039,14 @@ describe('legend interaction', function() { .then(assertVisible([false, true, true])) .then(click(0, 2)) .then(assertVisible([false, true, 'legendonly'])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('double-clicking an isolated trace shows all non-hidden traces', function(done) { Promise.resolve() .then(click(0, 2)) .then(assertVisible([false, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); }); @@ -1077,7 +1076,7 @@ describe('legend interaction', function() { .then(assertVisible([false, 'legendonly', true, 'legendonly'])) .then(click(1)) .then(assertVisible([false, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('isolates legendgroups as a whole', function(done) { @@ -1086,7 +1085,7 @@ describe('legend interaction', function() { .then(assertVisible([false, true, 'legendonly', true])) .then(click(1, 2)) .then(assertVisible([false, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); }); @@ -1119,7 +1118,7 @@ describe('legend interaction', function() { it('computes the initial visibility correctly', function(done) { Promise.resolve() .then(assertVisible([false, true, true, true, true, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('toggles the visibility of a non-groupby trace in the presence of groupby traces', function(done) { @@ -1128,7 +1127,7 @@ describe('legend interaction', function() { .then(assertVisible([false, true, 'legendonly', true, true, true, true, true])) .then(click(1)) .then(assertVisible([false, true, true, true, true, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('toggles the visibility of the first group in a groupby trace', function(done) { @@ -1137,7 +1136,7 @@ describe('legend interaction', function() { .then(assertVisible([false, 'legendonly', true, true, true, true, true, true])) .then(click(0)) .then(assertVisible([false, true, true, true, true, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('toggles the visibility of the third group in a groupby trace', function(done) { @@ -1146,7 +1145,7 @@ describe('legend interaction', function() { .then(assertVisible([false, true, true, true, 'legendonly', true, true, true])) .then(click(3)) .then(assertVisible([false, true, true, true, true, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('double-clicking isolates a non-groupby trace', function(done) { @@ -1155,7 +1154,7 @@ describe('legend interaction', function() { .then(assertVisible([false, true, 'legendonly', 'legendonly', 'legendonly', 'legendonly', 'legendonly', 'legendonly'])) .then(click(0, 2)) .then(assertVisible([false, true, true, true, true, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); it('double-clicking isolates a groupby trace', function(done) { @@ -1164,7 +1163,7 @@ describe('legend interaction', function() { .then(assertVisible([false, 'legendonly', true, 'legendonly', 'legendonly', 'legendonly', 'legendonly', 'legendonly'])) .then(click(1, 2)) .then(assertVisible([false, true, true, true, true, true, true, true])) - .catch(fail).then(done); + .catch(failTest).then(done); }); }); }); From 1f3163a3185bed44dbd017764b31cee11a663759 Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 25 Apr 2018 15:31:28 -0400 Subject: [PATCH 2/8] fix typos in lib/events.js :books: --- src/lib/events.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index 61b06b4b5cc..a90d3ed0a6b 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -85,7 +85,7 @@ var Events = { }, /* - * This function behaves like jQueries triggerHandler. It calls + * This function behaves like jQuery's triggerHandler. It calls * 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. @@ -98,7 +98,7 @@ var Events = { var jQueryHandlerValue; var nodeEventHandlerValue; /* - * If Jquery exists run all its handlers for this event and + * If jQuery exists run all its handlers for this event and * collect the return value of the LAST handler function */ if(typeof jQuery !== 'undefined') { @@ -133,9 +133,9 @@ var Events = { nodeEventHandlerValue = lastHandler(data); /* - * Return either the jquery handler value if it exists or the - * nodeEventHandler value. Jquery event value superceeds nodejs - * events for backwards compatability reasons. + * Return either the jQuery handler value if it exists or the + * nodeEventHandler value. jQuery event value supersedes nodejs + * events for backwards compatibility reasons. */ return jQueryHandlerValue !== undefined ? jQueryHandlerValue : nodeEventHandlerValue; From 92748601753517a171faea82725bffaca1cf9adc Mon Sep 17 00:00:00 2001 From: etienne Date: Wed, 25 Apr 2018 15:51:17 -0400 Subject: [PATCH 3/8] add 'plotly_legendclick' and 'plotly_doubleclick' events ... and use their return val to determine whether or not the default handler is called after them. This allows users to disable legend trace toggling and/or augment it! --- src/components/legend/draw.js | 55 ++++++++++------ test/jasmine/tests/legend_test.js | 102 +++++++++++++++++++++++++++++- 2 files changed, 137 insertions(+), 20 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index fbedb04763f..d67ea99255b 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -13,6 +13,7 @@ var d3 = require('d3'); var Lib = require('../../lib'); var Plots = require('../../plots/plots'); var Registry = require('../../registry'); +var Events = require('../../lib/events'); var dragElement = require('../dragelement'); var Drawing = require('../drawing'); var Color = require('../color'); @@ -336,22 +337,46 @@ module.exports = function draw(gd) { e.clientY >= bbox.top && e.clientY <= bbox.bottom); }); if(clickedTrace.size() > 0) { - if(numClicks === 1) { - legend._clickTimeout = setTimeout(function() { - handleClick(clickedTrace, gd, numClicks); - }, DBLCLICKDELAY); - } else if(numClicks === 2) { - if(legend._clickTimeout) { - clearTimeout(legend._clickTimeout); - } - handleClick(clickedTrace, gd, numClicks); - } + clickOrDoubleClick(gd, legend, clickedTrace, numClicks, e); } } }); } }; +function clickOrDoubleClick(gd, legend, clickedTrace, numClicks, evt) { + var datum = clickedTrace.data()[0][0]; + + var evtData = { + event: evt, + node: clickedTrace.node(), + itemNumber: datum.i, + curveNumber: datum.trace.index, + data: gd.data, + layout: gd.layout, + frames: gd._transitionData._frames, + config: gd._context, + fullData: gd._fullData, + fullLayout: gd._fullLayout + }; + + var returnVal; + + if(numClicks === 1) { + legend._clickTimeout = setTimeout(function() { + returnVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); + if(returnVal !== false) handleClick(clickedTrace, gd, numClicks); + }, DBLCLICKDELAY); + } + else if(numClicks === 2) { + if(legend._clickTimeout) clearTimeout(legend._clickTimeout); + gd._legendMouseDownTime = 0; + + returnVal = Events.triggerHandler(gd, 'plotly_legenddoubleclick', evtData); + if(returnVal !== false) handleClick(clickedTrace, gd, numClicks); + } +} + function drawTexts(g, gd) { var legendItem = g.data()[0][0], fullLayout = gd._fullLayout, @@ -441,15 +466,7 @@ function setupTraceToggle(g, gd) { numClicks = Math.max(numClicks - 1, 1); } - if(numClicks === 1) { - legend._clickTimeout = setTimeout(function() { handleClick(g, gd, numClicks); }, DBLCLICKDELAY); - } else if(numClicks === 2) { - if(legend._clickTimeout) { - clearTimeout(legend._clickTimeout); - } - gd._legendMouseDownTime = 0; - handleClick(g, gd, numClicks); - } + clickOrDoubleClick(gd, legend, g, numClicks, d3.event); }); } diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 39d02394279..ee920c39afa 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1001,9 +1001,13 @@ describe('legend interaction', function() { }; } + function extractVisibilities(data) { + return data.map(function(trace) { return trace.visible; }); + } + function assertVisible(expectation) { return function() { - var actual = gd._fullData.map(function(trace) { return trace.visible; }); + var actual = extractVisibilities(gd._fullData); expect(actual).toEqual(expectation); }; } @@ -1166,5 +1170,101 @@ describe('legend interaction', function() { .catch(failTest).then(done); }); }); + + describe('custom legend click/doubleclick handlers', function() { + var fig, to; + + beforeEach(function() { + fig = Lib.extendDeep({}, require('@mocks/0.json')); + }); + + afterEach(function() { + clearTimeout(to); + }); + + function setupFail() { + to = setTimeout(function() { + fail('did not trigger plotly_legendclick'); + }, 2 * DBLCLICKDELAY); + } + + it('should call custom click handler before default handler', function(done) { + Plotly.newPlot(gd, fig).then(function() { + var gotCalled = false; + + gd.on('plotly_legendclick', function(d) { + gotCalled = true; + expect(extractVisibilities(d.fullData)).toEqual([true, true, true]); + expect(extractVisibilities(gd._fullData)).toEqual([true, true, true]); + }); + gd.on('plotly_restyle', function() { + expect(extractVisibilities(gd._fullData)).toEqual([true, 'legendonly', true]); + if(gotCalled) done(); + }); + setupFail(); + }) + .then(click(1, 1)) + .catch(failTest); + }); + + it('should call custom doubleclick handler before default handler', function(done) { + Plotly.newPlot(gd, fig).then(function() { + var gotCalled = false; + + gd.on('plotly_legenddoubleclick', function(d) { + gotCalled = true; + expect(extractVisibilities(d.fullData)).toEqual([true, true, true]); + expect(extractVisibilities(gd._fullData)).toEqual([true, true, true]); + }); + gd.on('plotly_restyle', function() { + expect(extractVisibilities(gd._fullData)).toEqual(['legendonly', true, 'legendonly']); + if(gotCalled) done(); + }); + setupFail(); + }) + .then(click(1, 2)) + .catch(failTest); + }); + + it('should not call default click handler if custom handler return *false*', function(done) { + Plotly.newPlot(gd, fig).then(function() { + gd.on('plotly_legendclick', function(d) { + Plotly.relayout(gd, 'title', 'just clicked on trace #' + d.curveNumber); + return false; + }); + gd.on('plotly_relayout', function(d) { + expect(typeof d).toBe('object'); + expect(d.title).toBe('just clicked on trace #2'); + done(); + }); + gd.on('plotly_restyle', function() { + fail('should not have triggered plotly_restyle'); + }); + setupFail(); + }) + .then(click(2, 1)) + .catch(failTest); + }); + + it('should not call default doubleclick handle if custom handler return *false*', function(done) { + Plotly.newPlot(gd, fig).then(function() { + gd.on('plotly_legenddoubleclick', function(d) { + Plotly.relayout(gd, 'title', 'just double clicked on trace #' + d.curveNumber); + return false; + }); + gd.on('plotly_relayout', function(d) { + expect(typeof d).toBe('object'); + expect(d.title).toBe('just double clicked on trace #0'); + done(); + }); + gd.on('plotly_restyle', function() { + fail('should not have triggered plotly_restyle'); + }); + setupFail(); + }) + .then(click(0, 2)) + .catch(failTest); + }); + }); }); }); From 76b4b10096074605d36aa907b59eb750c77193dd Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 26 Apr 2018 14:15:05 -0400 Subject: [PATCH 4/8] :hocho: obsolete deprecation warning for Events.triggerHandler --- src/lib/events.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index a90d3ed0a6b..613b984c3ae 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -89,10 +89,6 @@ 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; From 02a85bf609bddfad3d6e9d80ee44b3bed40478e4 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 30 Apr 2018 14:45:16 -0400 Subject: [PATCH 5/8] improve and :lock: plotly_legend(double)click event data - add 'expandIndex' for all traces - add 'group' for traces with enable groupby transforms - :hocho: itemNumber (which was undefined on double click, can add it back in future if someone asks for it. --- src/components/legend/draw.js | 18 +++-- test/jasmine/tests/legend_test.js | 108 ++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index f85bdb451df..76ad7118876 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -355,14 +355,14 @@ module.exports = function draw(gd) { } }; -function clickOrDoubleClick(gd, legend, clickedTrace, numClicks, evt) { - var datum = clickedTrace.data()[0][0]; +function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { + var trace = legendItem.data()[0][0].trace; var evtData = { event: evt, - node: clickedTrace.node(), - itemNumber: datum.i, - curveNumber: datum.trace.index, + node: legendItem.node(), + curveNumber: trace.index, + expandedIndex: trace._expandedIndex, data: gd.data, layout: gd.layout, frames: gd._transitionData._frames, @@ -371,12 +371,16 @@ function clickOrDoubleClick(gd, legend, clickedTrace, numClicks, evt) { fullLayout: gd._fullLayout }; + if(trace._group) { + evtData.group = trace._group; + } + var returnVal; if(numClicks === 1) { legend._clickTimeout = setTimeout(function() { returnVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); - if(returnVal !== false) handleClick(clickedTrace, gd, numClicks); + if(returnVal !== false) handleClick(legendItem, gd, numClicks); }, DBLCLICKDELAY); } else if(numClicks === 2) { @@ -384,7 +388,7 @@ function clickOrDoubleClick(gd, legend, clickedTrace, numClicks, evt) { gd._legendMouseDownTime = 0; returnVal = Events.triggerHandler(gd, 'plotly_legenddoubleclick', evtData); - if(returnVal !== false) handleClick(clickedTrace, gd, numClicks); + if(returnVal !== false) handleClick(legendItem, gd, numClicks); } } diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 5c9e0bffd4a..ba426207f2a 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1299,5 +1299,113 @@ describe('legend interaction', function() { .catch(failTest); }); }); + + describe('legend click/doubleclick event data', function() { + function _assert(act, exp) { + for(var k in exp) { + if(k === 'event' || k === 'node') { + expect(act[k]).toBeDefined(); + } else if(k === 'group') { + expect(act[k]).toEqual(exp[k]); + } else { + expect(act[k]).toBe(exp[k], 'key ' + k); + } + } + + expect(Object.keys(act).length) + .toBe(Object.keys(exp).length, '# of keys'); + } + + function clickAndCheck(clickArg, exp) { + Lib.extendFlat(exp, { + event: true, + node: true, + data: gd.data, + layout: gd.layout, + frames: gd._transitionData._frames, + config: gd._context, + fullData: gd._fullData, + fullLayout: gd._fullLayout + }); + + var evtName = { + 1: 'plotly_legendclick', + 2: 'plotly_legenddoubleclick' + }[clickArg[1]]; + + return new Promise(function(resolve, reject) { + var hasBeenCalled = false; + + var to = setTimeout(function() { + reject('did not trigger ' + evtName); + }, 2 * DBLCLICKDELAY); + + gd.on(evtName, function(d) { + hasBeenCalled = true; + _assert(d, exp); + }); + gd.on('plotly_restyle', function() { + if(hasBeenCalled) { + clearTimeout(to); + resolve(); + } + }); + + click(clickArg[0], clickArg[1])(); + }); + } + + it('should have correct keys (base case)', function(done) { + Plotly.newPlot(gd, [{ + x: [1, 2, 3, 4, 5], + y: [1, 2, 1, 2, 3] + }], { + showlegend: true + }) + .then(function() { + return clickAndCheck([0, 1], { + curveNumber: 0, + expandedIndex: 0 + }); + }) + .then(function() { + return clickAndCheck([0, 2], { + curveNumber: 0, + expandedIndex: 0 + }); + }) + .catch(failTest) + .then(done); + }); + + it('should have correct keys (groupby case)', function(done) { + Plotly.newPlot(gd, [{ + x: [1, 2, 3, 4, 5], + y: [1, 2, 1, 2, 3], + transforms: [{ + type: 'groupby', + groups: ['a', 'b', 'b', 'a', 'b'] + }] + }, { + x: [1, 2, 3, 4, 5], + y: [1, 2, 1, 2, 3], + }]) + .then(function() { + return clickAndCheck([1, 1], { + curveNumber: 0, + expandedIndex: 1, + group: 'b' + }); + }) + .then(function() { + return clickAndCheck([2, 2], { + curveNumber: 1, + expandedIndex: 2 + }); + }) + .catch(failTest) + .then(done); + }); + }); }); }); From 9a7cc143ffffd8a7c1c117fd2174a1532fd2f521 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 1 May 2018 10:58:59 -0400 Subject: [PATCH 6/8] fix and :lock: triggerHandlers for ".once" handlers --- src/lib/events.js | 46 +++++++++++++++++++------------ test/jasmine/tests/events_test.js | 19 +++++++++++++ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index 613b984c3ae..df47b3f676b 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -58,7 +58,7 @@ var Events = { plotObj.removeAllListeners = ev.removeAllListeners.bind(ev); /* - * Create funtions for managing internal events. These are *only* triggered + * Create functions for managing internal events. These are *only* triggered * by the mirroring of external events via the emit function. */ plotObj._internalOn = internalEv.on.bind(internalEv); @@ -93,6 +93,7 @@ var Events = { triggerHandler: function(plotObj, event, data) { var jQueryHandlerValue; var nodeEventHandlerValue; + /* * If jQuery exists run all its handlers for this event and * collect the return value of the LAST handler function @@ -110,30 +111,41 @@ var Events = { var handlers = ev._events[event]; if(!handlers) return jQueryHandlerValue; - /* - * handlers can be function or an array of functions - */ - if(typeof handlers === 'function') handlers = [handlers]; - var lastHandler = handlers.pop(); - - /* - * Call all the handlers except the last one. - */ - for(var i = 0; i < handlers.length; i++) { - handlers[i](data); + // making sure 'this' is the EventEmitter instance + function apply(handler) { + // The 'once' case, we can't just call handler() as we need + // the return value here. So, + // - remove handler + // - call listener and grab return value! + // - stash 'fired' key to not call handler twice + if(handler.listener) { + ev.removeListener(event, handler.listener); + if(!handler.fired) { + handler.fired = true; + return handler.listener.apply(ev, [data]); + } + } else { + return handler.apply(ev, [data]); + } } - /* - * Now call the final handler and collect its value - */ - nodeEventHandlerValue = lastHandler(data); + // handlers can be function or an array of functions + handlers = Array.isArray(handlers) ? handlers : [handlers]; + + var i; + for(i = 0; i < handlers.length - 1; i++) { + apply(handlers[i]); + } + // now call the final handler and collect its value + nodeEventHandlerValue = apply(handlers[i]); /* * Return either the jQuery handler value if it exists or the * nodeEventHandler value. jQuery event value supersedes nodejs * events for backwards compatibility reasons. */ - return jQueryHandlerValue !== undefined ? jQueryHandlerValue : + return jQueryHandlerValue !== undefined ? + jQueryHandlerValue : nodeEventHandlerValue; }, diff --git a/test/jasmine/tests/events_test.js b/test/jasmine/tests/events_test.js index d4cc5bf423a..9cb0bfe19b7 100644 --- a/test/jasmine/tests/events_test.js +++ b/test/jasmine/tests/events_test.js @@ -220,6 +220,25 @@ describe('Events', function() { expect(eventBaton).toBe(3); expect(result).toBe('pong'); }); + + it('works with *once* event handlers', function() { + var eventBaton = 0; + + Events.init(plotDiv); + + plotDiv.once('ping', function() { + eventBaton++; + return 'pong'; + }); + + var result = Events.triggerHandler(plotDiv, 'ping'); + expect(result).toBe('pong'); + expect(eventBaton).toBe(1); + + var nop = Events.triggerHandler(plotDiv, 'ping'); + expect(nop).toBeUndefined(); + expect(eventBaton).toBe(1); + }); }); describe('purge', function() { From fde209f007ec2dc90aa22992c4a09fddedab417a Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 1 May 2018 11:19:29 -0400 Subject: [PATCH 7/8] trigger plotly_legendclick out of single-click timeout --- src/components/legend/draw.js | 10 +++++----- test/jasmine/tests/legend_test.js | 12 +++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 76ad7118876..96c09100b3a 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -375,20 +375,20 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { evtData.group = trace._group; } - var returnVal; + var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); + if(clickVal === false) return; if(numClicks === 1) { legend._clickTimeout = setTimeout(function() { - returnVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); - if(returnVal !== false) handleClick(legendItem, gd, numClicks); + handleClick(legendItem, gd, numClicks); }, DBLCLICKDELAY); } else if(numClicks === 2) { if(legend._clickTimeout) clearTimeout(legend._clickTimeout); gd._legendMouseDownTime = 0; - returnVal = Events.triggerHandler(gd, 'plotly_legenddoubleclick', evtData); - if(returnVal !== false) handleClick(legendItem, gd, numClicks); + var dblClickVal = Events.triggerHandler(gd, 'plotly_legenddoubleclick', evtData); + if(dblClickVal !== false) handleClick(legendItem, gd, numClicks); } } diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index ba426207f2a..15def938ace 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1340,17 +1340,19 @@ describe('legend interaction', function() { reject('did not trigger ' + evtName); }, 2 * DBLCLICKDELAY); - gd.on(evtName, function(d) { - hasBeenCalled = true; - _assert(d, exp); - }); - gd.on('plotly_restyle', function() { + function done() { if(hasBeenCalled) { clearTimeout(to); resolve(); } + } + + gd.once(evtName, function(d) { + hasBeenCalled = true; + _assert(d, exp); }); + gd.once('plotly_restyle', done); click(clickArg[0], clickArg[1])(); }); } From 05e1b6b8c21258dc619b3732f9b59bdb827d1f1c Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 1 May 2018 11:19:47 -0400 Subject: [PATCH 8/8] add 'label' in legend event data for pie traces --- src/components/legend/draw.js | 3 +++ test/jasmine/tests/legend_test.js | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 96c09100b3a..3e9b455f612 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -374,6 +374,9 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { if(trace._group) { evtData.group = trace._group; } + if(trace.type === 'pie') { + evtData.label = legendItem.datum()[0].label; + } var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); if(clickVal === false) return; diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 15def938ace..b92df5bc529 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1353,6 +1353,8 @@ describe('legend interaction', function() { }); gd.once('plotly_restyle', done); + gd.once('plotly_relayout', done); + click(clickArg[0], clickArg[1])(); }); } @@ -1408,6 +1410,30 @@ describe('legend interaction', function() { .catch(failTest) .then(done); }); + + it('should have correct keys (pie case)', function(done) { + Plotly.newPlot(gd, [{ + type: 'pie', + labels: ['A', 'B', 'C', 'D'], + values: [1, 2, 1, 3] + }]) + .then(function() { + return clickAndCheck([0, 1], { + curveNumber: 0, + expandedIndex: 0, + label: 'D' + }); + }) + .then(function() { + return clickAndCheck([2, 2], { + curveNumber: 0, + expandedIndex: 0, + label: 'A' + }); + }) + .catch(failTest) + .then(done); + }); }); }); });