Skip to content

unified hover: honor layout.hoverlabel #4687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 26, 2020
Merged

unified hover: honor layout.hoverlabel #4687

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9faea16
unified hover: use hoverlabel.bgcolor or legend.bgcolor or paperbg_color
antoinerg Mar 25, 2020
576ac60
unified hover: honor hoverlabel.font, then legend.font, then layout.font
antoinerg Mar 25, 2020
76a0488
unified hover: fix for unspecified layout.paperbg_color
antoinerg Mar 25, 2020
531fe79
unified hover: honor hoverlabel.bordercolor, then legend.bordercolor
antoinerg Mar 25, 2020
4ef8205
unified hover: remove extendFlat to handle falsy value for hoverlabel
antoinerg Mar 25, 2020
6483ac9
unified hover: fix inheritance of font attributes
antoinerg Mar 25, 2020
dfd73b8
set Fx defaults after legend defaults so unified hover label can inherit
antoinerg Mar 26, 2020
cc8ca1c
default unified hoverlabel.bgcolor combines legend.bgcolor and paper_…
antoinerg Mar 26, 2020
0d406a0
add comment stating that fx defaults should be set after legend defaults
antoinerg Mar 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,9 +989,10 @@ function createHoverText(hoverData, opts, gd) {
var mockLayoutIn = {
showlegend: true,
legend: {
title: {text: t0, font: fullLayout.font},
font: fullLayout.font,
bgcolor: fullLayout.paper_bgcolor,
title: {text: t0, font: fullLayout.hoverlabel.font},
font: fullLayout.hoverlabel.font,
bgcolor: fullLayout.hoverlabel.bgcolor || fullLayout.paper_bgcolor,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the paper_bgcolor fallback here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If hoverlabel.bgcolor is undefined, we need a fallback. We can either specify it here or specify paper_bgcolor in mockLayoutOut in order for legendSupplyDefaults to use it as a fallback. Let me know what you prefer!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test for this?
Should we update the descriptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the background color is well tested to make sure that the color is set by layout attribute hoverlabel.bgcolor > legend.bgcolor > paper_bgcolor.

We should definitely update the description.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just closing the loop based on our off-GH conversation: layout.hoverlabel.bgcolor is empty by default, so that each hoverlabel knows to take its color from its trace. It only has a value if you explicitly set one, that will become the common hover label color for all traces.

bordercolor: fullLayout.hoverlabel.bordercolor,
borderwidth: 1,
tracegroupgap: 7,
traceorder: fullLayout.legend ? fullLayout.legend.traceorder : undefined,
Expand Down
22 changes: 22 additions & 0 deletions src/components/fx/hoverlabel_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,32 @@
'use strict';

var Lib = require('../../lib');
var isUnifiedHover = require('./helpers').isUnifiedHover;

module.exports = function handleHoverLabelDefaults(contIn, contOut, coerce, opts) {
opts = opts || {};

function inheritFontAttr(attr) {
if(!opts.font[attr]) {
if(contIn.legend && contIn.legend.font && contIn.legend.font[attr]) {
opts.font[attr] = contIn.legend.font[attr];
} else if(contIn.font && contIn.font[attr]) {
opts.font[attr] = contIn.font[attr];
}
}
}

// In unified hover, inherit from legend if available
if(contIn && isUnifiedHover(contIn.hovermode)) {
if(!opts.font) opts.font = {};
inheritFontAttr('size');
inheritFontAttr('family');
inheritFontAttr('color');

if(!opts.bgcolor && contIn.legend && contIn.legend.bgcolor) opts.bgcolor = contIn.legend.bgcolor;
if(!opts.bordercolor && contIn.legend && contIn.legend.bordercolor) opts.bordercolor = contIn.legend.bordercolor;
}

coerce('hoverlabel.bgcolor', opts.bgcolor);
coerce('hoverlabel.bordercolor', opts.bordercolor);
coerce('hoverlabel.namelength', opts.namelength);
Expand Down
144 changes: 132 additions & 12 deletions test/jasmine/tests/hover_label_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3988,14 +3988,18 @@ describe('hovermode: (x|y)unified', function() {
Lib.clearThrottle();
}

function getHoverLabel() {
var hoverLayer = d3.select('g.hoverlayer');
return hoverLayer.select('g.legend');
}

function assertElementCount(selector, size) {
var g = d3.selectAll(selector);
expect(g.size()).toBe(size);
}

function assertLabel(expectation) {
var hoverLayer = d3.select('g.hoverlayer');
var hover = hoverLayer.select('g.legend');
var hover = getHoverLabel();
var title = hover.select('text.legendtitletext');
var traces = hover.selectAll('g.traces');

Expand All @@ -4010,16 +4014,15 @@ describe('hovermode: (x|y)unified', function() {
});
}

function assertBgcolor(color) {
var hoverLayer = d3.select('g.hoverlayer');
var hover = hoverLayer.select('g.legend');
function assertRectColor(color, bordercolor) {
var hover = getHoverLabel();
var bg = hover.select('rect.bg');
expect(bg.node().style.fill).toBe(color);
if(color) expect(bg.node().style.fill).toBe(color);
if(bordercolor) expect(bg.node().style.stroke).toBe(bordercolor);
}

function assertSymbol(exp) {
var hoverLayer = d3.select('g.hoverlayer');
var hover = hoverLayer.select('g.legend');
var hover = getHoverLabel();
var traces = hover.selectAll('g.traces');
expect(traces.size()).toBe(exp.length);

Expand All @@ -4034,6 +4037,17 @@ describe('hovermode: (x|y)unified', function() {
});
}

function assertFont(fontFamily, fontSize, fontColor) {
var hover = getHoverLabel();
var text = hover.select('text.legendtext');
var node = text.node();

var textStyle = window.getComputedStyle(node);
expect(textStyle.fontFamily.split(',')[0]).toBe(fontFamily, 'wrong font family');
expect(textStyle.fontSize).toBe(fontSize, 'wrong font size');
expect(textStyle.fill).toBe(fontColor, 'wrong font color');
}

it('set smart defaults for spikeline in x unified', function(done) {
Plotly.newPlot(gd, [{y: [4, 6, 5]}], {'hovermode': 'x unified', 'xaxis': {'color': 'red'}})
.then(function(gd) {
Expand Down Expand Up @@ -4316,15 +4330,121 @@ describe('hovermode: (x|y)unified', function() {
.then(done);
});

it('label should have color of paper_bgcolor', function(done) {
it('label should have bgcolor/bordercolor from hoverlabel or legend or paper_bgcolor', function(done) {
var mockCopy = Lib.extendDeep({}, mock);
var bgcolor = [
'rgb(10, 10, 10)',
'rgb(20, 20, 20)',
'rgb(30, 30, 30)',
'rgb(40, 40, 40)'
];

Plotly.newPlot(gd, mockCopy)
.then(function(gd) {
_hover(gd, { xval: 3 });

assertRectColor('rgb(255, 255, 255)', 'rgb(68, 68, 68)');

// Set paper_bgcolor
return Plotly.relayout(gd, 'paper_bgcolor', bgcolor[0]);
})
.then(function(gd) {
_hover(gd, { xval: 3 });

assertRectColor(bgcolor[0]);

// Set legend.bgcolor which should win over paper_bgcolor
return Plotly.relayout(gd, {
'showlegend': true,
'legend.bgcolor': bgcolor[1],
'legend.bordercolor': bgcolor[1]
});
})
.then(function(gd) {
_hover(gd, { xval: 3 });

assertRectColor(bgcolor[1], bgcolor[1]);

// Set hoverlabel.bgcolor which should win over legend.bgcolor
return Plotly.relayout(gd, {
'hoverlabel.bgcolor': bgcolor[2],
'hoverlabel.bordercolor': bgcolor[2]
});
})
.then(function(gd) {
_hover(gd, { xval: 3 });

assertRectColor(bgcolor[2], bgcolor[2]);

// Finally, check that a hoverlabel.bgcolor defined in template wins
delete mockCopy.layout;
mockCopy.layout = {
hovermode: 'x unified',
template: { layout: { hoverlabel: { bgcolor: bgcolor[3], bordercolor: bgcolor[3] } } },
legend: { bgcolor: bgcolor[1], bordercolor: bgcolor[1] }
};

return Plotly.newPlot(gd, mockCopy);
})
.then(function(gd) {
_hover(gd, { xval: 3 });

assertRectColor(bgcolor[3], bgcolor[3]);
})
.catch(failTest)
.then(done);
});

it('should use hoverlabel.font or legend.font or layout.font', function(done) {
var mockCopy = Lib.extendDeep({}, mock);
var bgcolor = 'rgb(15, 200, 85)';
mockCopy.layout.paper_bgcolor = bgcolor;

// Set layout.font
mockCopy.layout.font = {size: 20, family: 'Mono', color: 'rgb(10, 10, 10)'};
Plotly.newPlot(gd, mockCopy)
.then(function(gd) {
_hover(gd, { xval: 3});

assertFont('Mono', '20px', 'rgb(10, 10, 10)');

// Set legend.font which should win over layout font
return Plotly.relayout(gd, {
'showlegend': true,
'legend.font.size': 15,
'legend.font.family': 'Helvetica',
'legend.font.color': 'rgb(20, 20, 20)'
});
})
.then(function(gd) {
_hover(gd, { xval: 3 });

assertFont('Helvetica', '15px', 'rgb(20, 20, 20)');

// Set hoverlabel.font which should win over legend.font
return Plotly.relayout(gd, {
'hoverlabel.font.size': 22,
'hoverlabel.font.family': 'Arial',
'hoverlabel.font.color': 'rgb(30, 30, 30)'
});
})
.then(function() {
_hover(gd, { xval: 3 });

assertFont('Arial', '22px', 'rgb(30, 30, 30)');

// Finally, check that a hoverlabel.font defined in template wins
delete mockCopy.layout;
mockCopy.layout = {
hovermode: 'x unified',
template: { layout: { hoverlabel: { font: {family: 'Mono', size: 30, color: 'red'}}}},
legend: {font: {size: 20, family: 'Mono', color: 'rgb(10, 10, 10)'}}
};

return Plotly.newPlot(gd, mockCopy);
})
.then(function() {
_hover(gd, { xval: 3 });

assertBgcolor(bgcolor);
assertFont('Mono', '30px', 'rgb(255, 0, 0)');
})
.catch(failTest)
.then(done);
Expand Down