From 45e7a3d728e30fd6b9f9490c285ffe4296853e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 24 Sep 2018 18:13:31 -0400 Subject: [PATCH 1/2] contrast common hover label font color w/ bgcolor ... and fix tests that were asserting white on white hover label text. --- src/components/fx/hover.js | 3 ++- test/jasmine/tests/hover_label_test.js | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 4c187ae1fa9..3ffd75cdb2b 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -703,6 +703,7 @@ function createHoverText(hoverData, opts, gd) { var commonBgColor = commonLabelOpts.bgcolor || Color.defaultLine; var commonStroke = commonLabelOpts.bordercolor || Color.contrast(commonBgColor); + var contrastColor = Color.contrast(commonBgColor); lpath.style({ fill: commonBgColor, @@ -713,7 +714,7 @@ function createHoverText(hoverData, opts, gd) { .call(Drawing.font, commonLabelOpts.font.family || fontFamily, commonLabelOpts.font.size || fontSize, - commonLabelOpts.font.color || Color.background + commonLabelOpts.font.color || contrastColor ) .call(svgTextUtils.positionText, 0, 0) .call(svgTextUtils.convertToTspans, gd); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 9469a876f37..c7eff43c40d 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -2062,7 +2062,7 @@ describe('Test hover label custom styling:', function() { }); assertCommonLabel({ path: ['rgb(255, 255, 255)', 'rgb(68, 68, 68)'], - text: [13, 'Arial', 'rgb(255, 255, 255)'] + text: [13, 'Arial', 'rgb(68, 68, 68)'] }); }) .then(function() { @@ -2074,7 +2074,7 @@ describe('Test hover label custom styling:', function() { }); assertCommonLabel({ path: ['rgb(255, 255, 255)', 'rgb(68, 68, 68)'], - text: [13, 'Arial', 'rgb(255, 255, 255)'] + text: [13, 'Arial', 'rgb(68, 68, 68)'] }); }) .then(function() { @@ -2086,7 +2086,7 @@ describe('Test hover label custom styling:', function() { }); assertCommonLabel({ path: ['rgb(255, 255, 255)', 'rgb(68, 68, 68)'], - text: [13, 'Arial', 'rgb(255, 255, 255)'] + text: [13, 'Arial', 'rgb(68, 68, 68)'] }); // test arrayOk case @@ -2113,7 +2113,7 @@ describe('Test hover label custom styling:', function() { assertPtLabel(null); assertCommonLabel({ path: ['rgb(255, 255, 255)', 'rgb(68, 68, 68)'], - text: [13, 'Arial', 'rgb(255, 255, 255)'] + text: [13, 'Arial', 'rgb(68, 68, 68)'] }); // test base case From 864f94126062c88da57854e82bd6e2cd902d89c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 24 Sep 2018 18:20:20 -0400 Subject: [PATCH 2/2] do not use hoverlabel.bgcolor for "name" part of hover label - this makes the trace name visible with e.g. hoverlabel.bgcolor: 'white' --- src/components/fx/hover.js | 24 ++++++---- test/jasmine/tests/hover_label_test.js | 66 ++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 3ffd75cdb2b..674e8f3a31a 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -796,12 +796,20 @@ function createHoverText(hoverData, opts, gd) { var name = ''; var text = ''; - // combine possible non-opaque trace color with bgColor - var baseColor = Color.opacity(d.color) ? d.color : Color.defaultLine; - var traceColor = Color.combine(baseColor, bgColor); - + // combine possible non-opaque trace color with bgColor + var color0 = d.bgcolor || d.color; + // color for 'nums' part of the label + var numsColor = Color.combine( + Color.opacity(color0) ? color0 : Color.defaultLine, + bgColor + ); + // color for 'name' part of the label + var nameColor = Color.combine( + Color.opacity(d.color) ? d.color : Color.defaultLine, + bgColor + ); // find a contrasting color for border and text - var contrastColor = d.borderColor || Color.contrast(traceColor); + var contrastColor = d.borderColor || Color.contrast(numsColor); // to get custom 'name' labels pass cleanPoint if(d.nameOverride !== undefined) d.name = d.nameOverride; @@ -870,7 +878,7 @@ function createHoverText(hoverData, opts, gd) { tx2.call(Drawing.font, d.fontFamily || fontFamily, d.fontSize || fontSize, - traceColor) + nameColor) .text(name) .attr('data-notex', 1) .call(svgTextUtils.positionText, 0, 0) @@ -884,7 +892,7 @@ function createHoverText(hoverData, opts, gd) { g.select('path') .style({ - fill: traceColor, + fill: numsColor, stroke: contrastColor }); var tbb = tx.node().getBoundingClientRect(); @@ -1190,7 +1198,7 @@ function cleanPoint(d, hovermode) { } fill('hoverinfo', 'hi', 'hoverinfo'); - fill('color', 'hbg', 'hoverlabel.bgcolor'); + fill('bgcolor', 'hbg', 'hoverlabel.bgcolor'); fill('borderColor', 'hbc', 'hoverlabel.bordercolor'); fill('fontFamily', 'htf', 'hoverlabel.font.family'); fill('fontSize', 'hts', 'hoverlabel.font.size'); diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index c7eff43c40d..b181cca4e1d 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -2203,6 +2203,72 @@ describe('Test hover label custom styling:', function() { .then(done); }); + it('should work for x/y cartesian traces (multi-trace case)', function(done) { + var gd = createGraphDiv(); + + function assertNameLabel(expectation) { + var g = d3.selectAll('g.hovertext > text.name'); + + if(expectation === null) { + expect(g.size()).toBe(0); + } else { + g.each(function(_, i) { + var textStyle = window.getComputedStyle(this); + expect(textStyle.fill).toBe(expectation.color[i]); + }); + } + } + + Plotly.plot(gd, [{ + x: [1, 2, 3], + y: [1, 2, 1], + }, { + x: [1, 2, 3], + y: [4, 5, 4], + }], { + hovermode: 'x', + }) + .then(function() { + _hover(gd, { xval: gd._fullData[0].x[0] }); + assertNameLabel({ + color: ['rgb(31, 119, 180)', 'rgb(255, 127, 14)'] + }); + return Plotly.restyle(gd, 'marker.color', ['red', 'blue']); + }) + .then(function() { + _hover(gd, { xval: gd._fullData[0].x[0] }); + assertNameLabel({ + color: ['rgb(255, 0, 0)', 'rgb(0, 0, 255)'] + }); + return Plotly.relayout(gd, 'hoverlabel.bgcolor', 'white'); + }) + .then(function() { + _hover(gd, { xval: gd._fullData[0].x[0] }); + // should not affect the name font color + assertNameLabel({ + color: ['rgb(255, 0, 0)', 'rgb(0, 0, 255)'] + }); + return Plotly.restyle(gd, 'marker.color', ['rgba(255,0,0,0.1)', 'rgba(0,0,255,0.1)']); + }) + .then(function() { + _hover(gd, { xval: gd._fullData[0].x[0] }); + // should blend with plot_bgcolor + assertNameLabel({ + color: ['rgb(255, 179, 179)', 'rgb(179, 179, 255)'] + }); + return Plotly.restyle(gd, 'marker.color', ['rgba(255,0,0,0)', 'rgba(0,0,255,0)']); + }) + .then(function() { + _hover(gd, { xval: gd._fullData[0].x[0] }); + // uses default line color when opacity=0 + assertNameLabel({ + color: ['rgb(68, 68, 68)', 'rgb(68, 68, 68)'] + }); + }) + .catch(failTest) + .then(done); + }); + it('should work for 2d z cartesian traces', function(done) { var gd = createGraphDiv();