From 26eb06b6000a6eb36533113bc99ffe0399d5cd2b Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Fri, 15 Sep 2023 15:38:34 -0400 Subject: [PATCH 1/7] Fixing issue #5350 by moving the 'false' check to affect only the single click --- src/components/legend/draw.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 741bb089895..2d72f5b094c 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -465,10 +465,10 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { evtData.label = legendItem.datum()[0].label; } - var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); - if(clickVal === false) return; - if(numClicks === 1) { + var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); + if(clickVal === false) return; + legend._clickTimeout = setTimeout(function() { if(!gd._fullLayout) return; handleClick(legendItem, gd, numClicks); From cf2ef69e5a85d412c5624ab085af222f94337623 Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Fri, 15 Sep 2023 16:27:46 -0400 Subject: [PATCH 2/7] Modified 1 legend unit test --- src/components/legend/draw.js | 4 ++-- test/jasmine/tests/legend_test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 2d72f5b094c..e3d5f8f8ea9 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -464,11 +464,11 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { if(Registry.traceIs(trace, 'pie-like')) { evtData.label = legendItem.datum()[0].label; } - + if(numClicks === 1) { var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); if(clickVal === false) return; - + legend._clickTimeout = setTimeout(function() { if(!gd._fullLayout) return; handleClick(legendItem, gd, numClicks); diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 3dd140ffe95..590c208d3e5 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -2720,7 +2720,7 @@ describe('legend with custom doubleClickDelay', function() { .then(_assert('[long] after click + (t/2) delay', 1, 0)) .then(delay(tLong + 10)) .then(click(0)).then(delay(DBLCLICKDELAY + 1)).then(click(0)) - .then(_assert('[long] after click + (DBLCLICKDELAY+1) delay + click', 2, 1)) + .then(_assert('[long] after click + (DBLCLICKDELAY+1) delay + click', 1, 1)) .then(delay(tLong + 10)) .then(click(0)).then(delay(1.1 * tLong)).then(click(0)) .then(_assert('[long] after click + (1.1*t) delay + click', 2, 0)) From 6477e1a59aae11cd7b35988b383ed0d384a98984 Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Fri, 15 Sep 2023 16:43:46 -0400 Subject: [PATCH 3/7] removing trailing spaces --- src/components/legend/draw.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index e3d5f8f8ea9..cbd32bd1aee 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -464,7 +464,7 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { if(Registry.traceIs(trace, 'pie-like')) { evtData.label = legendItem.datum()[0].label; } - + if(numClicks === 1) { var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); if(clickVal === false) return; From 07f2b7059fae507a4ed7ec11c9444bd087f167d1 Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Tue, 19 Sep 2023 11:51:01 -0400 Subject: [PATCH 4/7] small tweak to always call handler --- src/components/legend/draw.js | 4 +--- test/jasmine/tests/legend_test.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index cbd32bd1aee..64bd3c2ef27 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -464,11 +464,9 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { if(Registry.traceIs(trace, 'pie-like')) { evtData.label = legendItem.datum()[0].label; } - + var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); if(numClicks === 1) { - var clickVal = Events.triggerHandler(gd, 'plotly_legendclick', evtData); if(clickVal === false) return; - legend._clickTimeout = setTimeout(function() { if(!gd._fullLayout) return; handleClick(legendItem, gd, numClicks); diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 590c208d3e5..3dd140ffe95 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -2720,7 +2720,7 @@ describe('legend with custom doubleClickDelay', function() { .then(_assert('[long] after click + (t/2) delay', 1, 0)) .then(delay(tLong + 10)) .then(click(0)).then(delay(DBLCLICKDELAY + 1)).then(click(0)) - .then(_assert('[long] after click + (DBLCLICKDELAY+1) delay + click', 1, 1)) + .then(_assert('[long] after click + (DBLCLICKDELAY+1) delay + click', 2, 1)) .then(delay(tLong + 10)) .then(click(0)).then(delay(1.1 * tLong)).then(click(0)) .then(_assert('[long] after click + (1.1*t) delay + click', 2, 0)) From 0757d3d2e8a1e557a1a9a0907bde85f4e9374418 Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Thu, 21 Sep 2023 13:16:19 -0400 Subject: [PATCH 5/7] dblclick default handler only when single click and dbl click are not false --- src/components/legend/draw.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 64bd3c2ef27..967f55ea629 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -476,7 +476,8 @@ function clickOrDoubleClick(gd, legend, legendItem, numClicks, evt) { gd._legendMouseDownTime = 0; var dblClickVal = Events.triggerHandler(gd, 'plotly_legenddoubleclick', evtData); - if(dblClickVal !== false) handleClick(legendItem, gd, numClicks); + // Activate default double click behaviour only when both single click and double click values are not false + if(dblClickVal !== false && clickVal !== false) handleClick(legendItem, gd, numClicks); } } From c25bc9b88b435baf55622b274761116fb3daa339 Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Fri, 22 Sep 2023 11:15:17 -0400 Subject: [PATCH 6/7] added draftlog file --- draftlogs/6727_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/6727_fix.md diff --git a/draftlogs/6727_fix.md b/draftlogs/6727_fix.md new file mode 100644 index 00000000000..7e8805335da --- /dev/null +++ b/draftlogs/6727_fix.md @@ -0,0 +1 @@ + - Addressing issue [[#5350](https://github.com/plotly/plotly.js/issues/5350)] via pull request [[#6727](https://github.com/plotly/plotly.js/pull/6727)]. This small change allows custom plotly_legenddoubleclick handlers to execute even when the default plotly_legendclick event is cancelled (returns false). From 6b72540c182ce29330d8340d3d523d8e94cec331 Mon Sep 17 00:00:00 2001 From: Andrej Vasilj Date: Fri, 22 Sep 2023 11:42:15 -0400 Subject: [PATCH 7/7] Added unit test for bug fix --- test/jasmine/tests/legend_test.js | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 3dd140ffe95..ded26277656 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -2742,6 +2742,38 @@ describe('legend with custom doubleClickDelay', function() { .then(_assert('[short] after click + (1.1*t) delay + click', 2, 0)) .then(done, done.fail); }, 3 * jasmine.DEFAULT_TIMEOUT_INTERVAL); + + it('custom plotly_legenddoubleclick handler should fire even when plotly_legendclick has been cancelled', function(done) { + var tShort = 0.75 * DBLCLICKDELAY; + var dblClickCnt = 0; + var newPlot = function(fig) { + return Plotly.newPlot(gd, fig).then(function() { + gd.on('plotly_legendclick', function() { return false; }); + gd.on('plotly_legenddoubleclick', function() { dblClickCnt++; }); + }); + }; + + function _assert(msg, _dblClickCnt) { + return function() { + expect(dblClickCnt).toBe(_dblClickCnt, msg + '| dblClickCnt'); + dblClickCnt = 0; + }; + } + + newPlot({ + data: [ + {y: [1, 2, 1]}, + {y: [2, 1, 2]} + ], + layout: {}, + config: {} + }) + .then(click(0)) + .then(delay(tShort)) + .then(click(0)) + .then(_assert('Double click increases count', 1)) + .then(done); + }, 3 * jasmine.DEFAULT_TIMEOUT_INTERVAL); }); describe('legend with custom legendwidth', function() {