From d2db27d5a6d0220657cee8320bd7f053ad20bb5a Mon Sep 17 00:00:00 2001 From: Domino987 Date: Mon, 26 Jul 2021 16:54:08 +0200 Subject: [PATCH 01/17] Skip "hoverinfo": "none" --- src/components/fx/hover.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 7773787fc73..2833499bf04 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -772,7 +772,10 @@ function _hover(gd, evt, subplot, noHoverEvent) { hoverdistance: fullLayout.hoverdistance }; - var hoverLabels = createHoverText(hoverData, labelOpts, gd); + var actualHoverData = hoverData.filter(function(d) { + return d.hoverinfo !== 'none'; + }); + var hoverLabels = createHoverText(actualHoverData, labelOpts, gd); if(!helpers.isUnifiedHover(hovermode)) { hoverAvoidOverlaps(hoverLabels, rotateLabels ? 'xa' : 'ya', fullLayout); From aee42e06bfc796256a066eef122f97b18b429d21 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Mon, 26 Jul 2021 17:16:20 +0200 Subject: [PATCH 02/17] add draft logs --- draftlogs/5854_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/5854_fix.md diff --git a/draftlogs/5854_fix.md b/draftlogs/5854_fix.md new file mode 100644 index 00000000000..c49f3a33488 --- /dev/null +++ b/draftlogs/5854_fix.md @@ -0,0 +1 @@ +- Skip `"hoverinfo": "none"` traces for `x/y unified` [[#5854](https://github.com/plotly/plotly.js/pull/5854)] From 0e897b50721ae590b7ca382bb9a2d78a0b819149 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Mon, 26 Jul 2021 18:41:29 +0200 Subject: [PATCH 03/17] improve fitlering of hover data --- src/components/fx/hover.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 2833499bf04..eee0020fb76 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -813,7 +813,10 @@ function hoverDataKey(d) { var EXTRA_STRING_REGEX = /([\s\S]*)<\/extra>/; -function createHoverText(hoverData, opts, gd) { +function createHoverText(allHoverData, opts, gd) { + var hoverData = allHoverData.filter(function(d) { + return d.hoverinfo !== 'none'; + }); var fullLayout = gd._fullLayout; var hovermode = opts.hovermode; var rotateLabels = opts.rotateLabels; From 6c181f6248283b067fcfd1c7bf0331c190cf15c8 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Mon, 26 Jul 2021 18:41:35 +0200 Subject: [PATCH 04/17] Update log --- draftlogs/5854_fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draftlogs/5854_fix.md b/draftlogs/5854_fix.md index c49f3a33488..6e7336c7f4c 100644 --- a/draftlogs/5854_fix.md +++ b/draftlogs/5854_fix.md @@ -1 +1 @@ -- Skip `"hoverinfo": "none"` traces for `x/y unified` [[#5854](https://github.com/plotly/plotly.js/pull/5854)] +- Skip `"hoverinfo": "none"` trace display for hover modes [[#5854](https://github.com/plotly/plotly.js/pull/5854)] From b116dc10d5d522ecd9a1d57d3014272515e0cb64 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Mon, 26 Jul 2021 22:37:19 +0200 Subject: [PATCH 05/17] move filter to unified only --- src/components/fx/hover.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index eee0020fb76..65d84c26dd9 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -813,10 +813,7 @@ function hoverDataKey(d) { var EXTRA_STRING_REGEX = /([\s\S]*)<\/extra>/; -function createHoverText(allHoverData, opts, gd) { - var hoverData = allHoverData.filter(function(d) { - return d.hoverinfo !== 'none'; - }); +function createHoverText(hoverData, opts, gd) { var fullLayout = gd._fullLayout; var hovermode = opts.hovermode; var rotateLabels = opts.rotateLabels; @@ -1028,11 +1025,14 @@ function createHoverText(allHoverData, opts, gd) { // Show a single hover label if(helpers.isUnifiedHover(hovermode)) { + var unifiedHoverData = hoverData.filter(function(d) { + return d.hoverinfo !== 'none'; + }); // Delete leftover hover labels from other hovermodes container.selectAll('g.hovertext').remove(); // Return early if nothing is hovered on - if(hoverData.length === 0) return; + if(unifiedHoverData.length === 0) return; // mock legend var mockLayoutIn = { @@ -1054,11 +1054,11 @@ function createHoverText(allHoverData, opts, gd) { // prepare items for the legend mockLegend.entries = []; - for(var j = 0; j < hoverData.length; j++) { - var texts = getHoverLabelText(hoverData[j], true, hovermode, fullLayout, t0); + for(var j = 0; j < unifiedHoverData.length; j++) { + var texts = getHoverLabelText(unifiedHoverData[j], true, hovermode, fullLayout, t0); var text = texts[0]; var name = texts[1]; - var pt = hoverData[j]; + var pt = unifiedHoverData[j]; pt.name = name; if(name !== '') { pt.text = name + ' : ' + text; @@ -1093,7 +1093,7 @@ function createHoverText(allHoverData, opts, gd) { var tbb = legendContainer.node().getBoundingClientRect(); var tWidth = tbb.width + 2 * HOVERTEXTPAD; var tHeight = tbb.height + 2 * HOVERTEXTPAD; - var winningPoint = hoverData[0]; + var winningPoint = unifiedHoverData[0]; var avgX = (winningPoint.x0 + winningPoint.x1) / 2; var avgY = (winningPoint.y0 + winningPoint.y1) / 2; // When a scatter (or e.g. heatmap) point wins, it's OK for the hovelabel to occlude the bar and other points. @@ -1108,11 +1108,11 @@ function createHoverText(allHoverData, opts, gd) { lyTop = avgY - HOVERTEXTPAD; lyBottom = avgY + HOVERTEXTPAD; } else { - lyTop = Math.min.apply(null, hoverData.map(function(c) { return Math.min(c.y0, c.y1); })); - lyBottom = Math.max.apply(null, hoverData.map(function(c) { return Math.max(c.y0, c.y1); })); + lyTop = Math.min.apply(null, unifiedHoverData.map(function(c) { return Math.min(c.y0, c.y1); })); + lyBottom = Math.max.apply(null, unifiedHoverData.map(function(c) { return Math.max(c.y0, c.y1); })); } } else { - lyTop = lyBottom = Lib.mean(hoverData.map(function(c) { return (c.y0 + c.y1) / 2; })) - tHeight / 2; + lyTop = lyBottom = Lib.mean(unifiedHoverData.map(function(c) { return (c.y0 + c.y1) / 2; })) - tHeight / 2; } var lxRight, lxLeft; @@ -1121,11 +1121,11 @@ function createHoverText(allHoverData, opts, gd) { lxRight = avgX + HOVERTEXTPAD; lxLeft = avgX - HOVERTEXTPAD; } else { - lxRight = Math.max.apply(null, hoverData.map(function(c) { return Math.max(c.x0, c.x1); })); - lxLeft = Math.min.apply(null, hoverData.map(function(c) { return Math.min(c.x0, c.x1); })); + lxRight = Math.max.apply(null, unifiedHoverData.map(function(c) { return Math.max(c.x0, c.x1); })); + lxLeft = Math.min.apply(null, unifiedHoverData.map(function(c) { return Math.min(c.x0, c.x1); })); } } else { - lxRight = lxLeft = Lib.mean(hoverData.map(function(c) { return (c.x0 + c.x1) / 2; })) - tWidth / 2; + lxRight = lxLeft = Lib.mean(unifiedHoverData.map(function(c) { return (c.x0 + c.x1) / 2; })) - tWidth / 2; } var xOffset = xa._offset; From 8f5f3d51214b159e0bfadb4869e672cf8e618cd4 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Thu, 5 Aug 2021 00:06:05 +0200 Subject: [PATCH 06/17] guard for no hover labels --- src/components/fx/hover.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 65d84c26dd9..cc5148b1970 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -775,9 +775,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { var actualHoverData = hoverData.filter(function(d) { return d.hoverinfo !== 'none'; }); - var hoverLabels = createHoverText(actualHoverData, labelOpts, gd); + var hoverLabels = actualHoverData.length && createHoverText(actualHoverData, labelOpts, gd); - if(!helpers.isUnifiedHover(hovermode)) { + if(hoverLabels && !helpers.isUnifiedHover(hovermode)) { hoverAvoidOverlaps(hoverLabels, rotateLabels ? 'xa' : 'ya', fullLayout); alignHoverText(hoverLabels, rotateLabels, fullLayout._invScaleX, fullLayout._invScaleY); } // TODO: tagName hack is needed to appease geo.js's hack of using evt.target=true From b0eb6dbdee15a75c5838ea6bfefa570361a12c39 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 5 Aug 2021 09:59:22 -0400 Subject: [PATCH 07/17] revert changes to hover.js --- src/components/fx/hover.js | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 0d7915aebda..e9e92749801 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -772,12 +772,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { hoverdistance: fullLayout.hoverdistance }; - var actualHoverData = hoverData.filter(function(d) { - return d.hoverinfo !== 'none'; - }); - var hoverLabels = actualHoverData.length && createHoverText(actualHoverData, labelOpts, gd); + var hoverLabels = createHoverText(hoverData, labelOpts, gd); - if(hoverLabels && !helpers.isUnifiedHover(hovermode)) { + if(!helpers.isUnifiedHover(hovermode)) { hoverAvoidOverlaps(hoverLabels, rotateLabels ? 'xa' : 'ya', fullLayout); alignHoverText(hoverLabels, rotateLabels, fullLayout._invScaleX, fullLayout._invScaleY); } // TODO: tagName hack is needed to appease geo.js's hack of using evt.target=true @@ -1025,14 +1022,11 @@ function createHoverText(hoverData, opts, gd) { // Show a single hover label if(helpers.isUnifiedHover(hovermode)) { - var unifiedHoverData = hoverData.filter(function(d) { - return d.hoverinfo !== 'none'; - }); // Delete leftover hover labels from other hovermodes container.selectAll('g.hovertext').remove(); // Return early if nothing is hovered on - if(unifiedHoverData.length === 0) return; + if(hoverData.length === 0) return; // mock legend var mockLayoutIn = { @@ -1054,11 +1048,11 @@ function createHoverText(hoverData, opts, gd) { // prepare items for the legend mockLegend.entries = []; - for(var j = 0; j < unifiedHoverData.length; j++) { - var texts = getHoverLabelText(unifiedHoverData[j], true, hovermode, fullLayout, t0); + for(var j = 0; j < hoverData.length; j++) { + var texts = getHoverLabelText(hoverData[j], true, hovermode, fullLayout, t0); var text = texts[0]; var name = texts[1]; - var pt = unifiedHoverData[j]; + var pt = hoverData[j]; pt.name = name; if(name !== '') { pt.text = name + ' : ' + text; @@ -1093,7 +1087,7 @@ function createHoverText(hoverData, opts, gd) { var tbb = legendContainer.node().getBoundingClientRect(); var tWidth = tbb.width + 2 * HOVERTEXTPAD; var tHeight = tbb.height + 2 * HOVERTEXTPAD; - var winningPoint = unifiedHoverData[0]; + var winningPoint = hoverData[0]; var avgX = (winningPoint.x0 + winningPoint.x1) / 2; var avgY = (winningPoint.y0 + winningPoint.y1) / 2; // When a scatter (or e.g. heatmap) point wins, it's OK for the hovelabel to occlude the bar and other points. @@ -1108,11 +1102,11 @@ function createHoverText(hoverData, opts, gd) { lyTop = avgY - HOVERTEXTPAD; lyBottom = avgY + HOVERTEXTPAD; } else { - lyTop = Math.min.apply(null, unifiedHoverData.map(function(c) { return Math.min(c.y0, c.y1); })); - lyBottom = Math.max.apply(null, unifiedHoverData.map(function(c) { return Math.max(c.y0, c.y1); })); + lyTop = Math.min.apply(null, hoverData.map(function(c) { return Math.min(c.y0, c.y1); })); + lyBottom = Math.max.apply(null, hoverData.map(function(c) { return Math.max(c.y0, c.y1); })); } } else { - lyTop = lyBottom = Lib.mean(unifiedHoverData.map(function(c) { return (c.y0 + c.y1) / 2; })) - tHeight / 2; + lyTop = lyBottom = Lib.mean(hoverData.map(function(c) { return (c.y0 + c.y1) / 2; })) - tHeight / 2; } var lxRight, lxLeft; @@ -1121,11 +1115,11 @@ function createHoverText(hoverData, opts, gd) { lxRight = avgX + HOVERTEXTPAD; lxLeft = avgX - HOVERTEXTPAD; } else { - lxRight = Math.max.apply(null, unifiedHoverData.map(function(c) { return Math.max(c.x0, c.x1); })); - lxLeft = Math.min.apply(null, unifiedHoverData.map(function(c) { return Math.min(c.x0, c.x1); })); + lxRight = Math.max.apply(null, hoverData.map(function(c) { return Math.max(c.x0, c.x1); })); + lxLeft = Math.min.apply(null, hoverData.map(function(c) { return Math.min(c.x0, c.x1); })); } } else { - lxRight = lxLeft = Lib.mean(unifiedHoverData.map(function(c) { return (c.x0 + c.x1) / 2; })) - tWidth / 2; + lxRight = lxLeft = Lib.mean(hoverData.map(function(c) { return (c.x0 + c.x1) / 2; })) - tWidth / 2; } var xOffset = xa._offset; From c52953dd8c652a423667e6c82d3bce572e7bc5da Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 5 Aug 2021 10:18:31 -0400 Subject: [PATCH 08/17] do not display points with none hoverinfo in unified modes --- src/components/fx/hover.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index e9e92749801..1190a7f1a95 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -1049,10 +1049,13 @@ function createHoverText(hoverData, opts, gd) { // prepare items for the legend mockLegend.entries = []; for(var j = 0; j < hoverData.length; j++) { - var texts = getHoverLabelText(hoverData[j], true, hovermode, fullLayout, t0); + var pt = hoverData[j]; + if(pt.hoverinfo === 'none') continue; + + var texts = getHoverLabelText(pt, true, hovermode, fullLayout, t0); var text = texts[0]; var name = texts[1]; - var pt = hoverData[j]; + pt.name = name; if(name !== '') { pt.text = name + ' : ' + text; From 3fe967f13fef600b529e927085d1568893ec032a Mon Sep 17 00:00:00 2001 From: Domino987 Date: Thu, 5 Aug 2021 17:04:44 +0200 Subject: [PATCH 09/17] Add tribute text --- draftlogs/5854_fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draftlogs/5854_fix.md b/draftlogs/5854_fix.md index 6e7336c7f4c..27de7da558a 100644 --- a/draftlogs/5854_fix.md +++ b/draftlogs/5854_fix.md @@ -1 +1 @@ -- Skip `"hoverinfo": "none"` trace display for hover modes [[#5854](https://github.com/plotly/plotly.js/pull/5854)] +- Skip `"hoverinfo": "none"` trace display for hover modes [[#5854](https://github.com/plotly/plotly.js/pull/5854)], with thanks to @Domino987 for the contribution! From ccd86f0df8027211449c87047fb48b31b0d74cf8 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Tue, 17 Aug 2021 00:18:11 +0200 Subject: [PATCH 10/17] add test for hoverinfo: none --- src/components/fx/hover.js | 5 +- test/jasmine/tests/hover_label_test.js | 68 ++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 1190a7f1a95..5e778b1f04e 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -810,7 +810,8 @@ function hoverDataKey(d) { var EXTRA_STRING_REGEX = /([\s\S]*)<\/extra>/; -function createHoverText(hoverData, opts, gd) { +function createHoverText(allHoverData, opts, gd) { + var hoverData = allHoverData.filter(data => data.hoverinfo !== 'none') var fullLayout = gd._fullLayout; var hovermode = opts.hovermode; var rotateLabels = opts.rotateLabels; @@ -1051,7 +1052,7 @@ function createHoverText(hoverData, opts, gd) { for(var j = 0; j < hoverData.length; j++) { var pt = hoverData[j]; if(pt.hoverinfo === 'none') continue; - + console.log(t0) var texts = getHoverLabelText(pt, true, hovermode, fullLayout, t0); var text = texts[0]; var name = texts[1]; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 9ceeba1bcb2..4058ba70926 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -4600,6 +4600,74 @@ describe('hovermode: (x|y)unified', function() { }) .then(done, done.fail); }); + + it('should not display hover for display: none', function(done) { + var x1 = []; + var y1 = []; + var x2 = []; + var y2 = []; + var i, t; + + function r100(v) { + return Math.round(v * 100); + } + + for(i = 0; i <= 1800; i++) { + t = i / 180 * Math.PI; + x1.push(r100(t / 5)); + y1.push(r100(Math.sin(t))); + } + + for(i = 0; i <= 360; i++) { + t = i / 180 * Math.PI; + x2.push(r100(t)); + y2.push(r100(Math.sin(t))); + } + + Plotly.newPlot(gd, { + data: [{ + name: 'high', + x: x1, + y: y1, + hoverinfo: 'none' + }, { + name: 'low', + x: x2, + y: y2 + }], + layout: { + hovermode: 'x unified', + showlegend: false, + width: 500, + height: 500, + margin: { + t: 50, + b: 50, + l: 50, + r: 50 + } + } + }) + .then(function() { + _hover(gd, { xpx: 100, ypx: 200 }); + assertLabel({title: '157', items: [ + 'low : 100' + ]}); + }) + .then(function() { + _hover(gd, { xpx: 175, ypx: 200 }); + assertLabel({title: '274', items: [ + 'low : 39' + ]}); + }) + .then(function() { + _hover(gd, { xpx: 350, ypx: 200 }); + assertLabel({title: '550', items: [ + 'low : −71' + ]}); + }) + .then(done, done.fail); + }); it('y unified should work for x/y cartesian traces', function(done) { var mockCopy = Lib.extendDeep({}, mock); From 27d1e1236a70b05d826925411595372e38d66ed3 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Tue, 17 Aug 2021 00:32:41 +0200 Subject: [PATCH 11/17] add early eject for no labels to be drawn --- src/components/fx/hover.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 5e778b1f04e..e9baf063d90 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -819,13 +819,15 @@ function createHoverText(allHoverData, opts, gd) { var container = opts.container; var outerContainer = opts.outerContainer; var commonLabelOpts = opts.commonLabelOpts || {}; + // Early exit if no labels are drawn + if(hoverData.length === 0) return container.selectAll('g.hovertext'); // opts.fontFamily/Size are used for the common label // and as defaults for each hover label, though the individual labels // can override this. var fontFamily = opts.fontFamily || constants.HOVERFONT; var fontSize = opts.fontSize || constants.HOVERFONTSIZE; - + var c0 = hoverData[0]; var xa = c0.xa; var ya = c0.ya; From 558a61b20305c39ea70d511a343f8f2e7ca8fa10 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Tue, 17 Aug 2021 00:49:34 +0200 Subject: [PATCH 12/17] return empty array for early eject --- package.json | 2 +- src/components/fx/hover.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index dcf5ad8cd7d..8b86b5330de 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "test-bundle": "node tasks/test_bundle.js", "test-requirejs": "node tasks/test_requirejs.js", "test-plain-obj": "node tasks/test_plain_obj.js", - "test": "npm run test-jasmine -- --nowatch && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint", + "test": "npm run test-jasmine hover_label && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint", "strict": "node devtools/test_dashboard/server.js --strict", "start": "node devtools/test_dashboard/server.js", "baseline": "node test/image/make_baseline.js", diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index e9baf063d90..48a89616449 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -819,8 +819,9 @@ function createHoverText(allHoverData, opts, gd) { var container = opts.container; var outerContainer = opts.outerContainer; var commonLabelOpts = opts.commonLabelOpts || {}; + console.log( container.selectAll('g.hovertext')) // Early exit if no labels are drawn - if(hoverData.length === 0) return container.selectAll('g.hovertext'); + if(hoverData.length === 0) return [[]]; // opts.fontFamily/Size are used for the common label // and as defaults for each hover label, though the individual labels From 897d1f7596b564741356c7ae001ad8aa226ebfc8 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Tue, 17 Aug 2021 00:49:49 +0200 Subject: [PATCH 13/17] fix test of package json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8b86b5330de..dcf5ad8cd7d 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "test-bundle": "node tasks/test_bundle.js", "test-requirejs": "node tasks/test_requirejs.js", "test-plain-obj": "node tasks/test_plain_obj.js", - "test": "npm run test-jasmine hover_label && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint", + "test": "npm run test-jasmine -- --nowatch && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint", "strict": "node devtools/test_dashboard/server.js --strict", "start": "node devtools/test_dashboard/server.js", "baseline": "node test/image/make_baseline.js", From 285da63d260ae53be57c8648f3601c20248c826c Mon Sep 17 00:00:00 2001 From: Domino987 Date: Tue, 17 Aug 2021 00:56:07 +0200 Subject: [PATCH 14/17] fix linting --- src/components/fx/hover.js | 7 +++---- test/jasmine/tests/hover_label_test.js | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 48a89616449..7de608a275e 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -811,7 +811,7 @@ function hoverDataKey(d) { var EXTRA_STRING_REGEX = /([\s\S]*)<\/extra>/; function createHoverText(allHoverData, opts, gd) { - var hoverData = allHoverData.filter(data => data.hoverinfo !== 'none') + var hoverData = allHoverData.filter(function(data) {return data.hoverinfo !== 'none';}); var fullLayout = gd._fullLayout; var hovermode = opts.hovermode; var rotateLabels = opts.rotateLabels; @@ -819,7 +819,6 @@ function createHoverText(allHoverData, opts, gd) { var container = opts.container; var outerContainer = opts.outerContainer; var commonLabelOpts = opts.commonLabelOpts || {}; - console.log( container.selectAll('g.hovertext')) // Early exit if no labels are drawn if(hoverData.length === 0) return [[]]; @@ -828,7 +827,7 @@ function createHoverText(allHoverData, opts, gd) { // can override this. var fontFamily = opts.fontFamily || constants.HOVERFONT; var fontSize = opts.fontSize || constants.HOVERFONTSIZE; - + var c0 = hoverData[0]; var xa = c0.xa; var ya = c0.ya; @@ -1055,7 +1054,7 @@ function createHoverText(allHoverData, opts, gd) { for(var j = 0; j < hoverData.length; j++) { var pt = hoverData[j]; if(pt.hoverinfo === 'none') continue; - console.log(t0) + var texts = getHoverLabelText(pt, true, hovermode, fullLayout, t0); var text = texts[0]; var name = texts[1]; diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 4058ba70926..8bc20d646f9 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -4600,7 +4600,7 @@ describe('hovermode: (x|y)unified', function() { }) .then(done, done.fail); }); - + it('should not display hover for display: none', function(done) { var x1 = []; var y1 = []; From e0cf3a77567058764fad9a452a054b678fec8298 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Wed, 25 Aug 2021 11:36:32 +0200 Subject: [PATCH 15/17] Only filter hovermode none for grouped label --- src/components/fx/hover.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 7de608a275e..6a8c8e9ea54 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -810,8 +810,7 @@ function hoverDataKey(d) { var EXTRA_STRING_REGEX = /([\s\S]*)<\/extra>/; -function createHoverText(allHoverData, opts, gd) { - var hoverData = allHoverData.filter(function(data) {return data.hoverinfo !== 'none';}); +function createHoverText(hoverData, opts, gd) { var fullLayout = gd._fullLayout; var hovermode = opts.hovermode; var rotateLabels = opts.rotateLabels; @@ -1027,9 +1026,9 @@ function createHoverText(allHoverData, opts, gd) { if(helpers.isUnifiedHover(hovermode)) { // Delete leftover hover labels from other hovermodes container.selectAll('g.hovertext').remove(); - + var groupedHoverData = hoverData.filter(function(data) {return data.hoverinfo !== 'none';}); // Return early if nothing is hovered on - if(hoverData.length === 0) return; + if(groupedHoverData.length === 0) return; // mock legend var mockLayoutIn = { @@ -1051,8 +1050,8 @@ function createHoverText(allHoverData, opts, gd) { // prepare items for the legend mockLegend.entries = []; - for(var j = 0; j < hoverData.length; j++) { - var pt = hoverData[j]; + for(var j = 0; j < groupedHoverData.length; j++) { + var pt = groupedHoverData[j]; if(pt.hoverinfo === 'none') continue; var texts = getHoverLabelText(pt, true, hovermode, fullLayout, t0); @@ -1093,7 +1092,7 @@ function createHoverText(allHoverData, opts, gd) { var tbb = legendContainer.node().getBoundingClientRect(); var tWidth = tbb.width + 2 * HOVERTEXTPAD; var tHeight = tbb.height + 2 * HOVERTEXTPAD; - var winningPoint = hoverData[0]; + var winningPoint = groupedHoverData[0]; var avgX = (winningPoint.x0 + winningPoint.x1) / 2; var avgY = (winningPoint.y0 + winningPoint.y1) / 2; // When a scatter (or e.g. heatmap) point wins, it's OK for the hovelabel to occlude the bar and other points. @@ -1108,11 +1107,11 @@ function createHoverText(allHoverData, opts, gd) { lyTop = avgY - HOVERTEXTPAD; lyBottom = avgY + HOVERTEXTPAD; } else { - lyTop = Math.min.apply(null, hoverData.map(function(c) { return Math.min(c.y0, c.y1); })); - lyBottom = Math.max.apply(null, hoverData.map(function(c) { return Math.max(c.y0, c.y1); })); + lyTop = Math.min.apply(null, groupedHoverData.map(function(c) { return Math.min(c.y0, c.y1); })); + lyBottom = Math.max.apply(null, groupedHoverData.map(function(c) { return Math.max(c.y0, c.y1); })); } } else { - lyTop = lyBottom = Lib.mean(hoverData.map(function(c) { return (c.y0 + c.y1) / 2; })) - tHeight / 2; + lyTop = lyBottom = Lib.mean(groupedHoverData.map(function(c) { return (c.y0 + c.y1) / 2; })) - tHeight / 2; } var lxRight, lxLeft; @@ -1121,11 +1120,11 @@ function createHoverText(allHoverData, opts, gd) { lxRight = avgX + HOVERTEXTPAD; lxLeft = avgX - HOVERTEXTPAD; } else { - lxRight = Math.max.apply(null, hoverData.map(function(c) { return Math.max(c.x0, c.x1); })); - lxLeft = Math.min.apply(null, hoverData.map(function(c) { return Math.min(c.x0, c.x1); })); + lxRight = Math.max.apply(null, groupedHoverData.map(function(c) { return Math.max(c.x0, c.x1); })); + lxLeft = Math.min.apply(null, groupedHoverData.map(function(c) { return Math.min(c.x0, c.x1); })); } } else { - lxRight = lxLeft = Lib.mean(hoverData.map(function(c) { return (c.x0 + c.x1) / 2; })) - tWidth / 2; + lxRight = lxLeft = Lib.mean(groupedHoverData.map(function(c) { return (c.x0 + c.x1) / 2; })) - tWidth / 2; } var xOffset = xa._offset; From 5ce39770366487b36fd7a196bbf4381e628983e4 Mon Sep 17 00:00:00 2001 From: Domino987 Date: Tue, 5 Oct 2021 11:23:16 +0200 Subject: [PATCH 16/17] move hoverinfo: 'none' down in test --- 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 8bc20d646f9..c09ff63e63d 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -4628,12 +4628,12 @@ describe('hovermode: (x|y)unified', function() { data: [{ name: 'high', x: x1, - y: y1, - hoverinfo: 'none' + y: y1 }, { name: 'low', x: x2, - y: y2 + y: y2, + hoverinfo: 'none' }], layout: { hovermode: 'x unified', From 0d9a1bedcd4db5ef5b15935ed2413a0d3488716e Mon Sep 17 00:00:00 2001 From: Domino987 Date: Thu, 7 Oct 2021 14:28:02 +0200 Subject: [PATCH 17/17] fix should not display hover for display: none --- test/jasmine/tests/hover_label_test.js | 50 ++++---------------------- 1 file changed, 7 insertions(+), 43 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index c09ff63e63d..69db628b1b6 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -4602,37 +4602,13 @@ describe('hovermode: (x|y)unified', function() { }); it('should not display hover for display: none', function(done) { - var x1 = []; - var y1 = []; - var x2 = []; - var y2 = []; - var i, t; - - function r100(v) { - return Math.round(v * 100); - } - - for(i = 0; i <= 1800; i++) { - t = i / 180 * Math.PI; - x1.push(r100(t / 5)); - y1.push(r100(Math.sin(t))); - } - - for(i = 0; i <= 360; i++) { - t = i / 180 * Math.PI; - x2.push(r100(t)); - y2.push(r100(Math.sin(t))); - } - Plotly.newPlot(gd, { data: [{ - name: 'high', - x: x1, - y: y1 + name: 'A', + y: [1] }, { - name: 'low', - x: x2, - y: y2, + name: 'B', + y: [2], hoverinfo: 'none' }], layout: { @@ -4649,21 +4625,9 @@ describe('hovermode: (x|y)unified', function() { } }) .then(function() { - _hover(gd, { xpx: 100, ypx: 200 }); - assertLabel({title: '157', items: [ - 'low : 100' - ]}); - }) - .then(function() { - _hover(gd, { xpx: 175, ypx: 200 }); - assertLabel({title: '274', items: [ - 'low : 39' - ]}); - }) - .then(function() { - _hover(gd, { xpx: 350, ypx: 200 }); - assertLabel({title: '550', items: [ - 'low : −71' + _hover(gd, { xpx: 200, ypx: 200 }); + assertLabel({title: '0', items: [ + 'A : 1' ]}); }) .then(done, done.fail);