From 8d1a49eb7020becbd0bb91864f043c5603b6b08c Mon Sep 17 00:00:00 2001 From: Marco Banterle Date: Wed, 10 Apr 2024 14:37:08 +0200 Subject: [PATCH 1/5] fix: do not ovelap hoverlabels for different tracetypes --- src/components/fx/hover.js | 2 +- test/jasmine/tests/hover_label_test.js | 57 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 883bf8c9194..ebf333a1e71 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -1757,7 +1757,7 @@ function hoverAvoidOverlaps(hoverLabels, rotateLabels, fullLayout, commonLabelBo topOverlap = p0.pos + p0.dp + p0.size - p1.pos - p1.dp + p1.size; // Only group points that lie on the same axes - if(topOverlap > 0.01 && (p0.pmin === p1.pmin) && (p0.pmax === p1.pmax)) { + if(topOverlap > 0.01 && p0.crossAxKey === p1.crossAxKey) { // push the new point(s) added to this group out of the way for(j = g1.length - 1; j >= 0; j--) g1[j].dp += topOverlap; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index e666526651e..5dd8cf76675 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1668,6 +1668,10 @@ describe('hover info', function() { return Math.max(0, overlap); } + function labelCount() { + return d3Select(gd).selectAll('g.hovertext').size(); + } + it('centered-aligned, should render labels inside boxes', function(done) { var trace1 = { x: ['giraffes'], @@ -1787,6 +1791,59 @@ describe('hover info', function() { }) .then(done, done.fail); }); + + it("does not overlap lebels for different trace types", function (done) { + function trace(name, type, delta) { + return { + name: name, + type: type, + y: [0 + delta, 1 + delta, 2 + delta], + x: ["CAT 1", "CAT 2", "CAT 3"], + }; + } + + var scatterName = "scatter_"; + var barName = "bar_"; + var data = []; + for(let i = 0; i<3; i++) { + data.push(trace(barName + i, "bar", 0.0)); + data.push(trace(scatterName + i, "scatter", 0.1)); + } + var layout = { + width: 600, + height: 400, + hovermode: "x", + }; + + Plotly.newPlot(gd, data, layout) + .then(function () { + _hoverNatural(gd, 200, 200); + }) + .then(function () { + expect(labelCount()).toBe(6); + }) + .then(function () { + + var nodes = []; + for(let i = 0; i<3; i++) { + nodes.push(hoverInfoNodes(barName + i).secondaryBox.getBoundingClientRect()); + nodes.push(hoverInfoNodes(scatterName + i).secondaryBox.getBoundingClientRect()); + } + nodes.sort(function(a,b) { return a.top - b.top; } ); + + for(let i = 0; i<5; i++) { + expect( + calcLineOverlap( + nodes[i].top, + nodes[i].bottom, + nodes[i+1].top, + nodes[i+1].bottom, + ) + ).toBeWithin(2, 1); + } + }) + .then(done, done.fail); + }); }); describe('constraints info graph viewport', function() { From 74dc83d01090c91ae6c5e2f0695c2ca71ae3f92e Mon Sep 17 00:00:00 2001 From: Marco Banterle Date: Wed, 10 Apr 2024 22:14:53 +0200 Subject: [PATCH 2/5] chore: draftlog --- draftlogs/6954_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/6954_fix.md diff --git a/draftlogs/6954_fix.md b/draftlogs/6954_fix.md new file mode 100644 index 00000000000..8e8f167df05 --- /dev/null +++ b/draftlogs/6954_fix.md @@ -0,0 +1 @@ +- fix for excessive hoverlabel removal and overlap for plots with both `scatter` and `bar` traces which reported in [#6917](https://github.com/plotly/plotly.js/issues/6917). \ No newline at end of file From 6cfc094954212972cccf615081384c148ca2b2d6 Mon Sep 17 00:00:00 2001 From: Marco Banterle Date: Wed, 10 Apr 2024 22:22:58 +0200 Subject: [PATCH 3/5] fix: var instead of let --- test/jasmine/tests/hover_label_test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 5dd8cf76675..ed4285fb04e 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1805,7 +1805,7 @@ describe('hover info', function() { var scatterName = "scatter_"; var barName = "bar_"; var data = []; - for(let i = 0; i<3; i++) { + for(var i = 0; i<3; i++) { data.push(trace(barName + i, "bar", 0.0)); data.push(trace(scatterName + i, "scatter", 0.1)); } @@ -1825,13 +1825,13 @@ describe('hover info', function() { .then(function () { var nodes = []; - for(let i = 0; i<3; i++) { + for(var i = 0; i<3; i++) { nodes.push(hoverInfoNodes(barName + i).secondaryBox.getBoundingClientRect()); nodes.push(hoverInfoNodes(scatterName + i).secondaryBox.getBoundingClientRect()); } nodes.sort(function(a,b) { return a.top - b.top; } ); - for(let i = 0; i<5; i++) { + for(var i = 0; i<5; i++) { expect( calcLineOverlap( nodes[i].top, From 33b28ecbebee3aada802abf19cd1625f98449815 Mon Sep 17 00:00:00 2001 From: mbant Date: Wed, 10 Apr 2024 22:46:33 +0200 Subject: [PATCH 4/5] fix: fix eslint mismatch --- test/jasmine/tests/hover_label_test.js | 84 +++++++++++++------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index ed4285fb04e..c8450737fba 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1792,58 +1792,58 @@ describe('hover info', function() { .then(done, done.fail); }); - it("does not overlap lebels for different trace types", function (done) { + it('does not overlap lebels for different trace types', function(done) { function trace(name, type, delta) { - return { - name: name, - type: type, - y: [0 + delta, 1 + delta, 2 + delta], - x: ["CAT 1", "CAT 2", "CAT 3"], - }; + return { + name: name, + type: type, + y: [0 + delta, 1 + delta, 2 + delta], + x: ['CAT 1', 'CAT 2', 'CAT 3'], + }; } - - var scatterName = "scatter_"; - var barName = "bar_"; + + var scatterName = 'scatter_'; + var barName = 'bar_'; var data = []; - for(var i = 0; i<3; i++) { - data.push(trace(barName + i, "bar", 0.0)); - data.push(trace(scatterName + i, "scatter", 0.1)); + var i; + for(i = 0; i < 3; i++) { + data.push(trace(barName + i, 'bar', 0.0)); + data.push(trace(scatterName + i, 'scatter', 0.1)); } var layout = { - width: 600, - height: 400, - hovermode: "x", + width: 600, + height: 400, + hovermode: 'x', }; - + Plotly.newPlot(gd, data, layout) - .then(function () { - _hoverNatural(gd, 200, 200); - }) - .then(function () { - expect(labelCount()).toBe(6); - }) - .then(function () { - - var nodes = []; - for(var i = 0; i<3; i++) { - nodes.push(hoverInfoNodes(barName + i).secondaryBox.getBoundingClientRect()); - nodes.push(hoverInfoNodes(scatterName + i).secondaryBox.getBoundingClientRect()); - } - nodes.sort(function(a,b) { return a.top - b.top; } ); - - for(var i = 0; i<5; i++) { + .then(function() { + _hoverNatural(gd, 200, 200); + }) + .then(function() { + expect(labelCount()).toBe(6); + }) + .then(function() { + var nodes = []; + for(i = 0; i < 3; i++) { + nodes.push(hoverInfoNodes(barName + i).secondaryBox.getBoundingClientRect()); + nodes.push(hoverInfoNodes(scatterName + i).secondaryBox.getBoundingClientRect()); + } + nodes.sort(function(a, b) { return a.top - b.top; }); + + for(i = 0; i < 5; i++) { expect( - calcLineOverlap( - nodes[i].top, - nodes[i].bottom, - nodes[i+1].top, - nodes[i+1].bottom, - ) - ).toBeWithin(2, 1); + calcLineOverlap( + nodes[i].top, + nodes[i].bottom, + nodes[i + 1].top, + nodes[i + 1].bottom + ) + ).toBeWithin(2, 1); } - }) + }) .then(done, done.fail); - }); + }); }); describe('constraints info graph viewport', function() { From 4321451abacc89fb7f4056d3818af9f0507086e0 Mon Sep 17 00:00:00 2001 From: mbant Date: Thu, 30 May 2024 13:37:38 +0200 Subject: [PATCH 5/5] fix: simplify label grouping condition --- src/components/fx/hover.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index ebf333a1e71..87e130d823d 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -1756,8 +1756,7 @@ function hoverAvoidOverlaps(hoverLabels, rotateLabels, fullLayout, commonLabelBo var p1 = g1[0]; topOverlap = p0.pos + p0.dp + p0.size - p1.pos - p1.dp + p1.size; - // Only group points that lie on the same axes - if(topOverlap > 0.01 && p0.crossAxKey === p1.crossAxKey) { + if(topOverlap > 0.01) { // push the new point(s) added to this group out of the way for(j = g1.length - 1; j >= 0; j--) g1[j].dp += topOverlap;