Skip to content

annotation hover text #1573

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 5 commits into from
Apr 11, 2017
Merged

annotation hover text #1573

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58966ed
annotation hover text
alexcjohnson Apr 7, 2017
384353a
lint annotations_test
alexcjohnson Apr 10, 2017
4ab7986
fix loneHover comment
alexcjohnson Apr 10, 2017
919580e
hover box default dark line -> #444 not black.
alexcjohnson Apr 10, 2017
da1e9d7
annotations[i].captureevents
alexcjohnson Apr 10, 2017
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
1 change: 1 addition & 0 deletions build/plotcss.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var rules = {
"X svg a:hover": "fill:#3c6dc5;",
"X .main-svg": "position:absolute;top:0;left:0;pointer-events:none;",
"X .main-svg .draglayer": "pointer-events:all;",
"X .cursor-default": "cursor:default;",
"X .cursor-pointer": "cursor:pointer;",
"X .cursor-crosshair": "cursor:crosshair;",
"X .cursor-move": "cursor:move;",
Expand Down
15 changes: 14 additions & 1 deletion src/components/annotations/annotation_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
var Lib = require('../../lib');
var Color = require('../color');
var Axes = require('../../plots/cartesian/axes');
var constants = require('../../plots/cartesian/constants');

var attributes = require('./attributes');

Expand All @@ -30,7 +31,7 @@ module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, op
if(!(visible || clickToShow)) return annOut;

coerce('opacity');
coerce('bgcolor');
var bgColor = coerce('bgcolor');

var borderColor = coerce('bordercolor'),
borderOpacity = Color.opacity(borderColor);
Expand Down Expand Up @@ -108,5 +109,17 @@ module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, op
annOut._yclick = (yClick === undefined) ? annOut.y : yClick;
}

var hoverText = coerce('hovertext');
if(hoverText) {
var hoverBG = coerce('hoverlabel.bgcolor',
Color.opacity(bgColor) ? Color.rgb(bgColor) : Color.defaultLine);
var hoverBorder = coerce('hoverlabel.bordercolor', Color.contrast(hoverBG));
Lib.coerceFont(coerce, 'hoverlabel.font', {
family: constants.HOVERFONT,
size: constants.HOVERFONTSIZE,
color: hoverBorder
});
}

return annOut;
};
35 changes: 35 additions & 0 deletions src/components/annotations/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,41 @@ module.exports = {
'is `yclick` rather than the annotation\'s `y` value.'
].join(' ')
},
hovertext: {
valType: 'string',
role: 'info',
description: [
'Sets text to appear when hovering over this annotation.',
'If omitted or blank, no hover label will appear.'
].join(' ')
},
hoverlabel: {
bgcolor: {
valType: 'color',
role: 'style',
description: [
'Sets the background color of the hover label.',
'By default uses the annotation\'s `bgcolor` made opaque,',
'or white if it was transparent.'
].join(' ')
},
bordercolor: {
valType: 'color',
role: 'style',
description: [
'Sets the border color of the hover label.',
'By default uses either dark grey or white, for maximum',
'contrast with `hoverlabel.bgcolor`.'
].join(' ')
},
font: extendFlat({}, fontAttrs, {
description: [
'Sets the hover label text font.',
'By default uses the global hover font and size,',
'with color from `hoverlabel.bordercolor`.'
].join(' ')
})
},

_deprecated: {
ref: {
Expand Down
45 changes: 38 additions & 7 deletions src/components/annotations/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var Plotly = require('../../plotly');
var Plots = require('../../plots/plots');
var Lib = require('../../lib');
var Axes = require('../../plots/cartesian/axes');
var Fx = require('../../plots/cartesian/graph_interact');
var Color = require('../color');
var Drawing = require('../drawing');
var svgTextUtils = require('../../lib/svg_text_utils');
Expand Down Expand Up @@ -96,7 +97,16 @@ function drawOne(gd, index) {
var annGroup = fullLayout._infolayer.append('g')
.classed('annotation', true)
.attr('data-index', String(index))
.style('opacity', options.opacity)
.style('opacity', options.opacity);

// another group for text+background so that they can rotate together
var annTextGroup = annGroup.append('g')
.classed('annotation-text-g', true)
.attr('data-index', String(index));

var annTextGroupInner = annTextGroup.append('g')
.style('pointer-events', 'all')
.call(setCursor, 'default')
.on('click', function() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

plotly_clickannotation was present, but didn't actually work before this, as far as I can tell, because it didn't have pointer-events turned back on. Anyway I had to do this to make hover work. A potential downside to this is that annotations grab mouse interactions even if you don't use them, so if you have a data point that's entirely under an annotation you won't see it in hover. You could imagine only grabbing mouse events if you are using them for hover, but then you won't get click events, and at draw time it's generally not known yet whether click events are going to be captured... so I don't see any way around this.

Anyway, I only attached click and hover to the text box, not to the arrows, and both are included in the new test.

Copy link
Contributor

Choose a reason for hiding this comment

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

I only attached click and hover to the text box, not to the arrows

That's sounds like the desired behavior to me.

gd._dragging = false;
gd.emit('plotly_clickannotation', {
Expand All @@ -106,12 +116,33 @@ function drawOne(gd, index) {
});
});

// another group for text+background so that they can rotate together
var annTextGroup = annGroup.append('g')
.classed('annotation-text-g', true)
.attr('data-index', String(index));

var annTextGroupInner = annTextGroup.append('g');
if(options.hovertext) {
annTextGroupInner
.on('mouseover', function() {
var hoverOptions = options.hoverlabel;
var hoverFont = hoverOptions.font;
var bBox = this.getBoundingClientRect();
var bBoxRef = gd.getBoundingClientRect();

Fx.loneHover({
Copy link
Contributor

@etpinard etpinard Apr 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

peek 2017-04-10 12-07

so this means the annotation hover takes precedence over trace hover. I don't think this a big deal. But perhaps we could make annotation hover pass by Fx.hover so that they don't conflict sort of speak.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added a new attribute captureevents in da1e9d7

This one defaults to false (so data point hover comes through) unless hovertext is provided. But as noted in the description, if you use plotly_clickannotation events you must explicitly turn this on.

@cldougl once this is merged we should update the (currently broken) example in https://plot.ly/javascript/text-and-annotations/#styling-and-formatting-annotations, and add a section about plotly_clickannotation in https://plot.ly/javascript/plotlyjs-events/ that specifically mentions captureevents.

Copy link
Member

Choose a reason for hiding this comment

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

got it-- just made an issue to keep track.
@alexcjohnson was there something specifically broken in https://plot.ly/javascript/text-and-annotations/#styling-and-formatting-annotations (just took a quick look and it seemed to work as expected) or should it just be updated to use plotly_clickannotation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @cldougl - I moved further discussion of this into the docs issue you made.

x0: bBox.left - bBoxRef.left,
x1: bBox.right - bBoxRef.left,
y: (bBox.top + bBox.bottom) / 2 - bBoxRef.top,
text: options.hovertext,
color: hoverOptions.bgcolor,
borderColor: hoverOptions.bordercolor,
fontFamily: hoverFont.family,
fontSize: hoverFont.size,
fontColor: hoverFont.color
}, {
container: fullLayout._hoverlayer.node(),
outerContainer: fullLayout._paper.node()
});
})
.on('mouseout', function() {
Fx.loneUnhover(fullLayout._hoverlayer.node());
});
}

var borderwidth = options.borderwidth,
borderpad = options.borderpad,
Expand Down
21 changes: 18 additions & 3 deletions src/components/color/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ color.defaultLine = colorAttrs.defaultLine;
color.lightLine = colorAttrs.lightLine;
color.background = colorAttrs.background;

/*
* tinyRGB: turn a tinycolor into an rgb string, but
* unlike the built-in tinycolor.toRgbString this never includes alpha
*/
color.tinyRGB = function(tc) {
var c = tc.toRgb();
return 'rgb(' + Math.round(c.r) + ', ' +
Expand Down Expand Up @@ -57,12 +61,23 @@ color.combine = function(front, back) {
return tinycolor(fcflat).toRgbString();
};

/*
* Create a color that contrasts with cstr.
*
* If cstr is a dark color, we lighten it; if it's light, we darken.
*
* If lightAmount / darkAmount are used, we adjust by these percentages,
* otherwise we go all the way to white or black.
* TODO: black is what we've always done for hover, but should it be #444 instead?
*/
color.contrast = function(cstr, lightAmount, darkAmount) {
var tc = tinycolor(cstr);

var newColor = tc.isLight() ?
tc.darken(darkAmount) :
tc.lighten(lightAmount);
if(tc.getAlpha() !== 1) tc = tinycolor(color.combine(cstr, '#fff'));

var newColor = tc.isDark() ?
(lightAmount ? tc.lighten(lightAmount) : '#fff') :
(darkAmount ? tc.darken(darkAmount) : '#000');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I switched from tc.isLight to tc.isDark just because isLight calls isDark anyway 🐎 .

Thoughts about my TODO?

TODO: black is what we've always done for hover, but should it be #444 instead?

(ie Color.defaultLine)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts about my TODO?

Sounds good!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in 919580e


return newColor.toString();
};
Expand Down
1 change: 1 addition & 0 deletions src/css/_cursor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.cursor-default { cursor: default; }
.cursor-pointer { cursor: pointer; }
.cursor-crosshair { cursor: crosshair; }
.cursor-move { cursor: move; }
Expand Down
82 changes: 54 additions & 28 deletions src/plots/cartesian/graph_interact.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
'use strict';

var d3 = require('d3');
var tinycolor = require('tinycolor2');
var isNumeric = require('fast-isnumeric');

var Lib = require('../../lib');
Expand Down Expand Up @@ -246,9 +245,7 @@ function quadrature(dx, dy) {

// size and display constants for hover text
var HOVERARROWSIZE = constants.HOVERARROWSIZE,
HOVERTEXTPAD = constants.HOVERTEXTPAD,
HOVERFONTSIZE = constants.HOVERFONTSIZE,
HOVERFONT = constants.HOVERFONT;
HOVERTEXTPAD = constants.HOVERTEXTPAD;

// fx.hover: highlight data on hover
// evt can be a mousemove event, or an object with data about what points
Expand Down Expand Up @@ -755,23 +752,36 @@ function cleanPoint(d, hovermode) {
return d;
}

/*
* Draw a single hover item in a pre-existing svg container somewhere
* hoverItem should have keys:
* - x and y (or x0, x1, y0, and y1):
* the pixel position to mark, relative to opts.container
* - xLabel, yLabel, zLabel, text, and name:
* info to go in the label
* - color:
* the background color for the label.
* - idealAlign (optional):
* 'left' or 'right' for which side of the x/y box to try to put this on first
* - borderColor (optional):
* color for the border, defaults to strongest contrast with color
* - fontFamily (optional):
* string, the font for this label, defaults to constants.HOVERFONT
* - fontSize (optional):
* the label font size, defaults to constants.HOVERFONTSIZE
* - fontColor (optional):
* defaults to borderColor
* opts should have keys:
* - bgColor:
* the background color this is against, used if the trace is
* non-opaque, and for the name, which goes outside the box
* - container:
* a dom <svg> element - must be big enough to contain the whole
* hover label
* - outerContainer:
* TODO: what exactly is container vs outerContainer?
*/
fx.loneHover = function(hoverItem, opts) {
// draw a single hover item in a pre-existing svg container somewhere
// hoverItem should have keys:
// - x and y (or x0, x1, y0, and y1):
// the pixel position to mark, relative to opts.container
// - xLabel, yLabel, zLabel, text, and name:
// info to go in the label
// - color:
// the background color for the label. text & outline color will
// be chosen black or white to contrast with this
// opts should have keys:
// - bgColor:
// the background color this is against, used if the trace is
// non-opaque, and for the name, which goes outside the box
// - container:
// a dom <svg> element - must be big enough to contain the whole
// hover label
var pointData = {
color: hoverItem.color || Color.defaultLine,
x0: hoverItem.x0 || hoverItem.x || 0,
Expand All @@ -785,6 +795,12 @@ fx.loneHover = function(hoverItem, opts) {
name: hoverItem.name,
idealAlign: hoverItem.idealAlign,

// optional extra bits of styling
borderColor: hoverItem.borderColor,
fontFamily: hoverItem.fontFamily,
fontSize: hoverItem.fontSize,
fontColor: hoverItem.fontColor,

// filler to make createHoverText happy
trace: {
index: 0,
Expand Down Expand Up @@ -830,6 +846,12 @@ function createHoverText(hoverData, opts) {
container = opts.container,
outerContainer = opts.outerContainer,

// opts.fontFamily/Size are used for the common label
// and as defaults for each hover label, though the individual labels
// can override this.
fontFamily = opts.fontFamily || constants.HOVERFONT,
fontSize = opts.fontSize || constants.HOVERFONTSIZE,

c0 = hoverData[0],
xa = c0.xa,
ya = c0.ya,
Expand Down Expand Up @@ -874,7 +896,7 @@ function createHoverText(hoverData, opts) {
lpath.enter().append('path')
.style({fill: Color.defaultLine, 'stroke-width': '1px', stroke: Color.background});
ltext.enter().append('text')
.call(Drawing.font, HOVERFONT, HOVERFONTSIZE, Color.background)
.call(Drawing.font, fontFamily, fontSize, Color.background)
// prohibit tex interpretation until we can handle
// tex and regular text together
.attr('data-notex', 1);
Expand Down Expand Up @@ -955,13 +977,12 @@ function createHoverText(hoverData, opts) {
// trace name label (rect and text.name)
g.append('rect')
.call(Color.fill, Color.addOpacity(bgColor, 0.8));
g.append('text').classed('name', true)
.call(Drawing.font, HOVERFONT, HOVERFONTSIZE);
g.append('text').classed('name', true);
// trace data label (path and text.nums)
g.append('path')
.style('stroke-width', '1px');
g.append('text').classed('nums', true)
.call(Drawing.font, HOVERFONT, HOVERFONTSIZE);
.call(Drawing.font, fontFamily, fontSize);
});
hoverLabels.exit().remove();

Expand All @@ -977,8 +998,7 @@ function createHoverText(hoverData, opts) {
traceColor = Color.combine(baseColor, bgColor),

// find a contrasting color for border and text
contrastColor = tinycolor(traceColor).getBrightness() > 128 ?
'#000' : Color.background;
contrastColor = d.borderColor || Color.contrast(traceColor);

// to get custom 'name' labels pass cleanPoint
if(d.nameOverride !== undefined) d.name = d.nameOverride;
Expand Down Expand Up @@ -1023,7 +1043,10 @@ function createHoverText(hoverData, opts) {

// main label
var tx = g.select('text.nums')
.style('fill', contrastColor)
.call(Drawing.font,
d.fontFamily || fontFamily,
d.fontSize || fontSize,
d.fontColor || contrastColor)
.call(Drawing.setPosition, 0, 0)
.text(text)
.attr('data-notex', 1)
Expand All @@ -1036,7 +1059,10 @@ function createHoverText(hoverData, opts) {

// secondary label for non-empty 'name'
if(name && name !== text) {
tx2.style('fill', traceColor)
tx2.call(Drawing.font,
d.fontFamily || fontFamily,
d.fontSize || fontSize,
traceColor)
.text(name)
.call(Drawing.setPosition, 0, 0)
.attr('data-notex', 1)
Expand Down
Loading