From 3f42bd43b3e6aa61a2b4ac1f9b69e475d6b0e17e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 24 Sep 2018 13:30:18 -0400 Subject: [PATCH 1/4] rotate hover labels under a few more circumstances - when hovermode:'y' and one trace with multiple hover labels - when hovermode:'closest' and at leat one trace is 'horizontal' on the subplot. - lock behavior in violin suite. --- src/components/fx/hover.js | 14 ++++++-- test/jasmine/assets/custom_assertions.js | 2 ++ test/jasmine/tests/violin_test.js | 42 ++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 4c187ae1fa9..0de3f4eae6d 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -237,6 +237,10 @@ function _hover(gd, evt, subplot, noHoverEvent) { vLinePoint: null }; + // does subplot have one (or more) horizontal traces, + // (to determine whether we rotate the labels or not) + var hasOneHorizontalTrace = false; + // Figure out what we're hovering on: // mouse location or user-supplied data @@ -256,6 +260,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { trace = cd[0].trace; if(trace.hoverinfo !== 'skip' && helpers.isTraceInSubplots(trace, subplots)) { searchData.push(cd); + if(trace.orientation === 'h') { + hasOneHorizontalTrace = true; + } } } @@ -577,9 +584,10 @@ function _hover(gd, evt, subplot, noHoverEvent) { gd._hoverdata = newhoverdata; - // if there's more than one horz bar trace, - // rotate the labels so they don't overlap - var rotateLabels = hovermode === 'y' && searchData.length > 1; + var rotateLabels = ( + (hovermode === 'y' && (searchData.length > 1 || hoverData.length > 1)) || + (hovermode === 'closest' && hasOneHorizontalTrace) + ); var bgColor = Color.combine( fullLayout.plot_bgcolor || Color.background, diff --git a/test/jasmine/assets/custom_assertions.js b/test/jasmine/assets/custom_assertions.js index 4cdee1c84f7..f6a1f47f1b1 100644 --- a/test/jasmine/assets/custom_assertions.js +++ b/test/jasmine/assets/custom_assertions.js @@ -90,6 +90,8 @@ function count(selector) { * - nums {string || array of strings} * - name {string || array of strings} * - axis {string} + * - vOrder {array of number} + * - hOrder {array of number} * @param {string} msg */ exports.assertHoverLabelContent = function(expectation, msg) { diff --git a/test/jasmine/tests/violin_test.js b/test/jasmine/tests/violin_test.js index 3d5acc95435..f3f78562a6a 100644 --- a/test/jasmine/tests/violin_test.js +++ b/test/jasmine/tests/violin_test.js @@ -431,6 +431,48 @@ describe('Test violin hover:', function() { nums: 'x: 42.43046, kde: 0.083', name: '', axis: 'Saturday' + }, { + desc: 'single horizontal violin', + mock: require('@mocks/violin_non-linear.json'), + pos: [310, 160], + nums: ['median: C', 'min: A', 'q1: B', 'q3: D', 'max: G', 'upper fence: D', 'x: C, kde: 1.005'], + name: ['categories', '', '', '', '', '', ''], + axis: 'categories', + hOrder: [4, 5, 3, 6, 0, 2, 1], + }, { + desc: 'multiple horizontal violins', + mock: require('@mocks/box_grouped_horz.json'), + patch: function(fig) { + fig.data.forEach(function(t) { + t.type = 'violin'; + t.hoveron = 'violins'; + }); + fig.layout.violinmode = 'group'; + return fig; + }, + nums: ['median: 0.4', 'min: 0.1', 'q1: 0.2', 'q3: 0.7', 'max: 0.9'], + name: ['kale', '', '', '', ''], + axis: 'day 2', + hOrder: [4, 3, 0, 2, 1], + }, { + desc: 'multiple horizontal violins (under hovermode:closest)', + mock: require('@mocks/box_grouped_horz.json'), + patch: function(fig) { + fig.data.forEach(function(t) { + t.type = 'violin'; + t.hoveron = 'violins'; + }); + fig.layout.violinmode = 'group'; + fig.layout.hovermode = 'closest'; + return fig; + }, + pos: [200, 175], + nums: [ + '(median: 0.7, day 2)', '(min: 0.2, day 2)', '(q1: 0.5, day 2)', + '(q3: 0.8, day 2)', '(max: 0.9, day 2)' + ], + name: ['radishes', '', '', '', ''], + hOrder: [4, 3, 0, 2, 1], }] .forEach(function(specs) { it('should generate correct hover labels ' + specs.desc, function(done) { From a64943beed66538d239e6f1915c1f2efd16f32e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 24 Sep 2018 13:31:06 -0400 Subject: [PATCH 2/4] assert hover label rotation --- test/jasmine/assets/custom_assertions.js | 49 ++++++++++++++---------- test/jasmine/tests/violin_test.js | 3 ++ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/test/jasmine/assets/custom_assertions.js b/test/jasmine/assets/custom_assertions.js index f6a1f47f1b1..9923b23add9 100644 --- a/test/jasmine/assets/custom_assertions.js +++ b/test/jasmine/assets/custom_assertions.js @@ -92,6 +92,7 @@ function count(selector) { * - axis {string} * - vOrder {array of number} * - hOrder {array of number} + * - isRotated {boolean} * @param {string} msg */ exports.assertHoverLabelContent = function(expectation, msg) { @@ -105,17 +106,22 @@ exports.assertHoverLabelContent = function(expectation, msg) { var axMsg = 'common axis hover label'; var axCnt = count(axSelector); + var reRotate = /(\brotate\(.*?\);?)/; + if(ptCnt === 1) { - assertLabelContent( - d3.select(ptSelector + '> text.nums'), - expectation.nums, - ptMsg + ' (nums)' - ); - assertLabelContent( - d3.select(ptSelector + '> text.name'), - expectation.name, - ptMsg + ' (name)' - ); + var g = d3.select(ptSelector); + var numsSel = g.select('text.nums'); + var nameSel = g.select('text.name'); + + assertLabelContent(numsSel, expectation.nums, ptMsg + ' (nums)'); + assertLabelContent(nameSel, expectation.name, ptMsg + ' (name)'); + + if('isRotated' in expectation) { + expect(g.attr('transform').match(reRotate)) + .negateIf(expectation.isRotated) + .toBe(null, ptMsg + ' should be rotated'); + + } } else if(ptCnt > 1) { if(!Array.isArray(expectation.nums) || !Array.isArray(expectation.name)) { fail(ptMsg + ': expecting more than 1 labels.'); @@ -126,16 +132,19 @@ exports.assertHoverLabelContent = function(expectation, msg) { var bboxes = []; d3.selectAll(ptSelector).each(function(_, i) { - assertLabelContent( - d3.select(this).select('text.nums'), - expectation.nums[i], - ptMsg + ' (nums ' + i + ')' - ); - assertLabelContent( - d3.select(this).select('text.name'), - expectation.name[i], - ptMsg + ' (name ' + i + ')' - ); + var g = d3.select(this); + var numsSel = g.select('text.nums'); + var nameSel = g.select('text.name'); + + assertLabelContent(numsSel, expectation.nums[i], ptMsg + ' (nums ' + i + ')'); + assertLabelContent(nameSel, expectation.name[i], ptMsg + ' (name ' + i + ')'); + + if('isRotated' in expectation) { + expect(g.attr('transform').match(reRotate)) + .negateIf(expectation.isRotated) + .toBe(null, ptMsg + ' ' + i + ' should be rotated'); + } + bboxes.push({bbox: this.getBoundingClientRect(), index: i}); }); if(expectation.vOrder) { diff --git a/test/jasmine/tests/violin_test.js b/test/jasmine/tests/violin_test.js index f3f78562a6a..da488b72be7 100644 --- a/test/jasmine/tests/violin_test.js +++ b/test/jasmine/tests/violin_test.js @@ -439,6 +439,7 @@ describe('Test violin hover:', function() { name: ['categories', '', '', '', '', '', ''], axis: 'categories', hOrder: [4, 5, 3, 6, 0, 2, 1], + isRotated: true }, { desc: 'multiple horizontal violins', mock: require('@mocks/box_grouped_horz.json'), @@ -454,6 +455,7 @@ describe('Test violin hover:', function() { name: ['kale', '', '', '', ''], axis: 'day 2', hOrder: [4, 3, 0, 2, 1], + isRotated: true }, { desc: 'multiple horizontal violins (under hovermode:closest)', mock: require('@mocks/box_grouped_horz.json'), @@ -473,6 +475,7 @@ describe('Test violin hover:', function() { ], name: ['radishes', '', '', '', ''], hOrder: [4, 3, 0, 2, 1], + isRotated: true }] .forEach(function(specs) { it('should generate correct hover labels ' + specs.desc, function(done) { From 579bd8d90312d5e5e6d78109b2117ef142559810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 25 Sep 2018 11:02:13 -0400 Subject: [PATCH 3/4] keep track of hasOneHorizontalTrace in user Fx.hover calls --- src/components/fx/hover.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 0de3f4eae6d..181f15be76e 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -249,8 +249,12 @@ function _hover(gd, evt, subplot, noHoverEvent) { hovermode = 'array'; for(itemnum = 0; itemnum < evt.length; itemnum++) { cd = gd.calcdata[evt[itemnum].curveNumber||0]; + trace = cd[0].trace; if(cd[0].trace.hoverinfo !== 'skip') { searchData.push(cd); + if(trace.orientation === 'h') { + hasOneHorizontalTrace = true; + } } } } From d9f3db7b44c3a7ff13ee4cd79c5a99ba87a3e2fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 25 Sep 2018 11:03:03 -0400 Subject: [PATCH 4/4] do not rotate single labels in 'closest' ... and if the corresponding trace is 'horizontal' --- src/components/fx/hover.js | 6 +- test/jasmine/tests/hover_label_test.js | 107 +++++++++++++++++++++++++ test/jasmine/tests/violin_test.js | 13 +++ 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 181f15be76e..b503a7a3978 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -237,8 +237,8 @@ function _hover(gd, evt, subplot, noHoverEvent) { vLinePoint: null }; - // does subplot have one (or more) horizontal traces, - // (to determine whether we rotate the labels or not) + // does subplot have one (or more) horizontal traces? + // This is used to determine whether we rotate the labels or not var hasOneHorizontalTrace = false; // Figure out what we're hovering on: @@ -590,7 +590,7 @@ function _hover(gd, evt, subplot, noHoverEvent) { var rotateLabels = ( (hovermode === 'y' && (searchData.length > 1 || hoverData.length > 1)) || - (hovermode === 'closest' && hasOneHorizontalTrace) + (hovermode === 'closest' && hasOneHorizontalTrace && hoverData.length > 1) ); var bgColor = Color.combine( diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 9469a876f37..63e88e41bb8 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -2470,6 +2470,113 @@ describe('hover distance', function() { }); }); +describe('hover label rotation:', function() { + var gd; + + function _hover(gd, opts) { + Fx.hover(gd, opts); + Lib.clearThrottle(); + } + + describe('when a single pt is picked', function() { + afterAll(destroyGraphDiv); + + beforeAll(function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, [{ + type: 'bar', + orientation: 'h', + y: [0, 1, 2], + x: [1, 2, 1] + }, { + type: 'bar', + orientation: 'h', + y: [3, 4, 5], + x: [1, 2, 1] + }], { + hovermode: 'y' + }) + .catch(failTest) + .then(done); + }); + + it('should rotate labels under *hovermode:y*', function() { + _hover(gd, { xval: 2, yval: 1 }); + assertHoverLabelContent({ + nums: '2', + name: 'trace 0', + axis: '1', + // N.B. could be changed to be made consistent with 'closest' + isRotated: true + }); + }); + + it('should not rotate labels under *hovermode:closest*', function(done) { + Plotly.relayout(gd, 'hovermode', 'closest').then(function() { + _hover(gd, { xval: 1.9, yval: 1 }); + assertHoverLabelContent({ + nums: '(2, 1)', + name: 'trace 0', + axis: '', + isRotated: false + }); + }) + .catch(failTest) + .then(done); + }); + }); + + describe('when mulitple pts are picked', function() { + afterAll(destroyGraphDiv); + + beforeAll(function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, [{ + type: 'bar', + orientation: 'h', + y: [0, 1, 2], + x: [1, 2, 1] + }, { + type: 'bar', + orientation: 'h', + y: [0, 1, 2], + x: [1, 2, 1] + }], { + hovermode: 'y' + }) + .catch(failTest) + .then(done); + }); + + it('should rotate labels under *hovermode:y*', function() { + _hover(gd, { xval: 2, yval: 1 }); + assertHoverLabelContent({ + nums: ['2', '2'], + name: ['trace 0', 'trace 1'], + axis: '1', + isRotated: true + }); + }); + + it('should not rotate labels under *hovermode:closest*', function(done) { + Plotly.relayout(gd, 'hovermode', 'closest').then(function() { + _hover(gd, { xval: 1.9, yval: 1 }); + assertHoverLabelContent({ + nums: '(2, 1)', + // N.B. only showing the 'top' trace + name: 'trace 1', + axis: '', + isRotated: false + }); + }) + .catch(failTest) + .then(done); + }); + }); +}); + describe('hovermode defaults to', function() { var gd; diff --git a/test/jasmine/tests/violin_test.js b/test/jasmine/tests/violin_test.js index da488b72be7..c925b096acb 100644 --- a/test/jasmine/tests/violin_test.js +++ b/test/jasmine/tests/violin_test.js @@ -476,6 +476,19 @@ describe('Test violin hover:', function() { name: ['radishes', '', '', '', ''], hOrder: [4, 3, 0, 2, 1], isRotated: true + }, { + desc: 'hovering over single pt on horizontal violin should not rotate labels', + mock: require('@mocks/violin_old-faithful.json'), + patch: function(fig) { + fig.data[0].x = fig.data[0].y; + delete fig.data[0].y; + fig.layout = {hovermode: 'closest'}; + return fig; + }, + pos: [539, 293], + nums: '(96, Old Faithful)', + name: '', + isRotated: false }] .forEach(function(specs) { it('should generate correct hover labels ' + specs.desc, function(done) {