From 0426b61b9a6034b73adca10970923d8ced6cc71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 19 Jul 2018 17:13:39 -0400 Subject: [PATCH 1/4] lint in legend.draw --- src/components/legend/draw.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 3e9b455f612..69b54355104 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -341,12 +341,13 @@ module.exports = function draw(gd) { } }, clickFn: function(numClicks, e) { - var clickedTrace = - fullLayout._infolayer.selectAll('g.traces').filter(function() { - var bbox = this.getBoundingClientRect(); - return (e.clientX >= bbox.left && e.clientX <= bbox.right && - e.clientY >= bbox.top && e.clientY <= bbox.bottom); - }); + var clickedTrace = fullLayout._infolayer.selectAll('g.traces').filter(function() { + var bbox = this.getBoundingClientRect(); + return ( + e.clientX >= bbox.left && e.clientX <= bbox.right && + e.clientY >= bbox.top && e.clientY <= bbox.bottom + ); + }); if(clickedTrace.size() > 0) { clickOrDoubleClick(gd, legend, clickedTrace, numClicks, e); } @@ -673,8 +674,8 @@ function computeLegendDimensions(gd, groups, traces) { opts._height = Math.ceil(opts._height); traces.each(function(d) { - var legendItem = d[0], - bg = d3.select(this).select('.legendtoggle'); + var legendItem = d[0]; + var bg = d3.select(this).select('.legendtoggle'); Drawing.setRect(bg, 0, From 7b3b97106217649f223e8c85fe2c9318992a324a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 19 Jul 2018 17:15:48 -0400 Subject: [PATCH 2/4] fix #2819 - don't use computed width when edits.legendPosition:true - just like we do currently for edits.legendText, that way the legend dragElement instance can correctly retrieve the legend item bounding box --- src/components/legend/draw.js | 7 ++- test/jasmine/tests/legend_test.js | 75 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 69b54355104..4de2e679429 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -673,6 +673,11 @@ function computeLegendDimensions(gd, groups, traces) { opts._width = Math.ceil(opts._width); opts._height = Math.ceil(opts._height); + var isEditable = ( + gd._context.edits.legendText || + gd._context.edits.legendPosition + ); + traces.each(function(d) { var legendItem = d[0]; var bg = d3.select(this).select('.legendtoggle'); @@ -680,7 +685,7 @@ function computeLegendDimensions(gd, groups, traces) { Drawing.setRect(bg, 0, -legendItem.height / 2, - (gd._context.edits.legendText ? 0 : opts._width) + extraWidth, + (isEditable ? 0 : opts._width) + extraWidth, legendItem.height ); }); diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 871e01381c1..13091e31edc 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -10,6 +10,7 @@ var anchorUtils = require('@src/components/legend/anchor_utils'); var d3 = require('d3'); var failTest = require('../assets/fail_test'); +var mouseEvent = require('../assets/mouse_event'); var delay = require('../assets/delay'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); @@ -929,6 +930,7 @@ describe('legend interaction', function() { describe('editable mode interactions', function() { var gd; + var mock = { data: [{ x: [1, 2, 3], @@ -1027,6 +1029,79 @@ describe('legend interaction', function() { }); }); + describe('visible toggle', function() { + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + var data = [ + {y: [1, 2, 1]}, + {y: [2, 1, 2]}, + {y: [2, 3, 4]} + ]; + + // we need to click on the drag cover to truly test this, + function clickAt(p) { + return function() { + return new Promise(function(resolve) { + mouseEvent('mousedown', p[0], p[1]); + mouseEvent('mouseup', p[0], p[1]); + setTimeout(resolve, DBLCLICKDELAY + 20); + }); + }; + } + + function assertVisible(expectation) { + return function() { + var actual = gd._fullData.map(function(t) { return t.visible; }); + expect(actual).toEqual(expectation); + }; + } + + var specs = [{ + orientation: 'h', + edits: {legendPosition: true}, + clickPos: [[118, 469], [212, 469], [295, 469]] + }, { + orientation: 'h', + edits: {legendPosition: true, legendText: true}, + clickPos: [[118, 469], [212, 469], [295, 469]] + }, { + orientation: 'v', + edits: {legendPosition: true}, + clickPos: [[430, 114], [430, 131], [430, 153]] + }, { + orientation: 'v', + edits: {legendPosition: true, legendText: true}, + clickPos: [[430, 114], [430, 131], [430, 153]] + }]; + + specs.forEach(function(s) { + var msg = s.orientation + ' - ' + JSON.stringify(s.edits); + + it('should find correct bounding box (case ' + msg + ')', function(done) { + Plotly.plot(gd, + Lib.extendDeep([], data), + {legend: {orientation: s.orientation}, width: 500, height: 500}, + {edits: s.edits} + ) + .then(assertVisible([true, true, true])) + .then(clickAt(s.clickPos[0])) + .then(assertVisible(['legendonly', true, true])) + .then(clickAt(s.clickPos[1])) + .then(assertVisible(['legendonly', 'legendonly', true])) + .then(clickAt(s.clickPos[2])) + .then(assertVisible(['legendonly', 'legendonly', 'legendonly'])) + .catch(failTest) + .then(done); + }); + }); + }); + describe('legend visibility interactions', function() { var gd; From 05089a3c5f7a0f52c42a429c7368932f2a2d35d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 09:30:19 -0400 Subject: [PATCH 3/4] add flaky tag --- test/jasmine/tests/legend_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 13091e31edc..91c6f159a7b 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1029,7 +1029,7 @@ describe('legend interaction', function() { }); }); - describe('visible toggle', function() { + describe('@flaky visible toggle', function() { var gd; beforeEach(function() { From 5449f304ab0021562134c9a60a7d89a3d5d8606d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 20 Jul 2018 17:37:53 -0400 Subject: [PATCH 4/4] try targeting to rm CI flakiness --- test/jasmine/tests/legend_test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 91c6f159a7b..686f7b24da6 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1029,7 +1029,7 @@ describe('legend interaction', function() { }); }); - describe('@flaky visible toggle', function() { + describe('visible toggle', function() { var gd; beforeEach(function() { @@ -1048,8 +1048,10 @@ describe('legend interaction', function() { function clickAt(p) { return function() { return new Promise(function(resolve) { - mouseEvent('mousedown', p[0], p[1]); - mouseEvent('mouseup', p[0], p[1]); + var el = d3.select('g.legend').node(); + var opts = {element: el}; + mouseEvent('mousedown', p[0], p[1], opts); + mouseEvent('mouseup', p[0], p[1], opts); setTimeout(resolve, DBLCLICKDELAY + 20); }); };