Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
6 changes: 5 additions & 1 deletion src/components/annotations/annotation_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, op
if(!(visible || clickToShow)) return annOut;

coerce('opacity');
coerce('align');
coerce('bgcolor');

var borderColor = coerce('bordercolor'),
Expand All @@ -45,6 +44,11 @@ module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, op
coerce('textangle');
Lib.coerceFont(coerce, 'font', fullLayout.font);

coerce('width');
coerce('height');
coerce('align');
coerce('valign');
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we don't need to coerce valign is height isn't set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 -> 2e132bd


// positioning
var axLetters = ['x', 'y'],
arrowPosDflt = [-10, -30],
Expand Down
40 changes: 36 additions & 4 deletions src/components/annotations/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,27 @@ module.exports = {
font: extendFlat({}, fontAttrs, {
description: 'Sets the annotation text font.'
}),
width: {
valType: 'number',
min: 0,
dflt: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Obviously the default value isn't 0, it's computed from the input text. I believe in plotly.js lingo that means the dflt value should be set to null (similar to e.g. contours.start and xbins.start)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good call -> e01a765

role: 'style',
description: [
'Sets an explicit width for the text box. 0 (default) lets the',
'text set the box width. Wider text will be clipped.',
'There is no automatic wrapping; use <br> to start a new line.'
].join(' ')
},
height: {
valType: 'number',
min: 0,
dflt: 0,
role: 'style',
description: [
'Sets an explicit height for the text box. 0 (default) lets the',
'text set the box height. Taller text will be clipped.'
].join(' ')
},
opacity: {
valType: 'number',
min: 0,
Expand All @@ -63,10 +84,21 @@ module.exports = {
dflt: 'center',
role: 'style',
description: [
'Sets the vertical alignment of the `text` with',
'respect to the set `x` and `y` position.',
'Has only an effect if `text` spans more two or more lines',
'(i.e. `text` contains one or more <br> HTML tags).'
'Sets the horizontal alignment of the `text` within the box.',
'Has an effect only if `text` spans more two or more lines',
'(i.e. `text` contains one or more <br> HTML tags) or if an',
'explicit width is set to override the text width.'
].join(' ')
},
valign: {
valType: 'enumerated',
values: ['top', 'middle', 'bottom'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it 👍

dflt: 'middle',
role: 'style',
description: [
'Sets the vertical alignment of the `text` within the box.',
'Has an effect only if an explicit height is set to override',
'the text height.'
].join(' ')
},
bgcolor: {
Expand Down
74 changes: 55 additions & 19 deletions src/components/annotations/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,19 @@ function drawOne(gd, index) {
.call(Color.stroke, options.bordercolor)
.call(Color.fill, options.bgcolor);

var isSizeConstrained = options.width || options.height;

var annClipID = 'clip' + fullLayout._uid + '_ann' + index;
var annTextClip = fullLayout._defs.select('.clips')
.selectAll('#' + annClipID)
.data(isSizeConstrained ? [0] : []);

annTextClip.enter().append('clipPath')
.classed('annclip', true)
.attr('id', annClipID)
.append('rect');
annTextClip.exit().remove();
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a jasmine test, checking that the clip paths are removed when removing an annotation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In fact they're not removed - they don't proliferate, ie if you repeatedly make 100 annotations and delete them, you'll never get more than 100 clip paths, but I didn't bother deleting clip paths corresponding to annotations past the end of the new array after removing some. I can revisit that though...

Copy link
Contributor

@etpinard etpinard Apr 6, 2017

Choose a reason for hiding this comment

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

Ha I see. We can revisit this when we shift to a more d3-idiomatic pattern in annotation draw. Then, we could easily remove those <clipPath> inside the exit() selection similar to how updatemenu buttons are removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually was easy to remove the unneeded clip paths after all -> b21154b


var font = options.font;

var annText = annTextGroupInner.append('text')
Expand All @@ -144,19 +157,21 @@ function drawOne(gd, index) {
// at the end, even if their position changes
annText.selectAll('tspan.line').attr({y: 0, x: 0});

var mathjaxGroup = annTextGroupInner.select('.annotation-math-group'),
hasMathjax = !mathjaxGroup.empty(),
anntextBB = Drawing.bBox(
(hasMathjax ? mathjaxGroup : annText).node()),
annwidth = anntextBB.width,
annheight = anntextBB.height,
outerwidth = Math.round(annwidth + 2 * borderfull),
outerheight = Math.round(annheight + 2 * borderfull);
var mathjaxGroup = annTextGroupInner.select('.annotation-math-group');
var hasMathjax = !mathjaxGroup.empty();
var anntextBB = Drawing.bBox(
(hasMathjax ? mathjaxGroup : annText).node());
var textWidth = anntextBB.width;
var textHeight = anntextBB.height;
var annWidth = options.width || textWidth;
var annHeight = options.height || textHeight;
var outerWidth = Math.round(annWidth + 2 * borderfull);
var outerHeight = Math.round(annHeight + 2 * borderfull);


// save size in the annotation object for use by autoscale
options._w = annwidth;
options._h = annheight;
options._w = annWidth;
options._h = annHeight;

function shiftFraction(v, anchor) {
if(anchor === 'auto') {
Expand All @@ -181,8 +196,8 @@ function drawOne(gd, index) {
ax = Axes.getFromId(gd, axRef),
dimAngle = (textangle + (axLetter === 'x' ? 0 : -90)) * Math.PI / 180,
// note that these two can be either positive or negative
annSizeFromWidth = outerwidth * Math.cos(dimAngle),
annSizeFromHeight = outerheight * Math.sin(dimAngle),
annSizeFromWidth = outerWidth * Math.cos(dimAngle),
annSizeFromHeight = outerHeight * Math.sin(dimAngle),
// but this one is the positive total size
annSize = Math.abs(annSizeFromWidth) + Math.abs(annSizeFromHeight),
anchor = options[axLetter + 'anchor'],
Expand Down Expand Up @@ -299,22 +314,43 @@ function drawOne(gd, index) {
return;
}

var xShift = 0;
var yShift = 0;

if(options.align !== 'left') {
xShift = (annWidth - textWidth) * (options.align === 'center' ? 0.5 : 1);
}
if(options.valign !== 'top') {
yShift = (annHeight - textHeight) * (options.valign === 'middle' ? 0.5 : 1);
}

if(hasMathjax) {
mathjaxGroup.select('svg').attr({x: borderfull - 1, y: borderfull});
mathjaxGroup.select('svg').attr({
x: borderfull + xShift - 1,
y: borderfull + yShift
})
.call(Drawing.setClipUrl, isSizeConstrained ? annClipID : null);
}
else {
var texty = borderfull - anntextBB.top,
textx = borderfull - anntextBB.left;
annText.attr({x: textx, y: texty});
var texty = borderfull + yShift - anntextBB.top,
textx = borderfull + xShift - anntextBB.left;
annText.attr({
x: textx,
y: texty
})
.call(Drawing.setClipUrl, isSizeConstrained ? annClipID : null);
annText.selectAll('tspan.line').attr({y: texty, x: textx});
}

annTextClip.select('rect').call(Drawing.setRect, borderfull, borderfull,
annWidth, annHeight);

annTextBG.call(Drawing.setRect, borderwidth / 2, borderwidth / 2,
outerwidth - borderwidth, outerheight - borderwidth);
outerWidth - borderwidth, outerHeight - borderwidth);

annTextGroupInner.call(Drawing.setTranslate,
Math.round(annPosPx.x.text - outerwidth / 2),
Math.round(annPosPx.y.text - outerheight / 2));
Math.round(annPosPx.x.text - outerWidth / 2),
Math.round(annPosPx.y.text - outerHeight / 2));

/*
* rotate text and background
Expand Down
Binary file modified test/image/baselines/annotations-autorange.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
40 changes: 33 additions & 7 deletions test/image/mocks/annotations-autorange.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@
"zeroline":false,
"showline":true
},
"height":300,
"height":360,
"width":800,
"margin":{"r":40,"b":40,"l":40,"t":40},
"margin":{"r":40,"b":100,"l":40,"t":40},
"annotations":[
{"ay":0,"ax":50,"x":1,"y":1.5,"text":"Left"},
{"ay":0,"ax":-50,"x":2,"y":1.5,"text":"Right"},
Expand All @@ -60,16 +60,42 @@
"xref":"x2","yref":"y2","text":"From left","y":2,"ax":-17,"ay":0,"x":"2001-01-01",
"xanchor": "right", "yanchor": "top", "textangle": 35, "bordercolor": "#444"
},
{"xref":"x2","yref":"y2","text":"From right","y":2,"x":"2001-03-01","ay":0,"ax":50},
{
"xref":"x2","yref":"y2","text":"From<br>right","y":2,"x":"2001-03-01","ay":0,"ax":50,
"bgcolor": "#eee", "width": 50, "height": 40, "textangle": 70
},
{
"xref":"x2","yref":"y2","text":"From top","y":3,"ax":0,"ay":-27,"x":"2001-02-01",
"xanchor": "left", "yanchor": "bottom", "textangle": -15, "bordercolor": "#444"
},
{"xref":"x2","yref":"y2","text":"From Bottom","y":1,"ax":0,"ay":50,"x":"2001-02-01"},
{"xref":"x3","yref":"y3","text":"Left<br>no<br>arrow","y":1.5,"x":1,"showarrow":false},
{
"xref":"x2","yref":"y2","text":"From Bottom","y":1,"ax":0,"ay":50,"x":"2001-02-01",
"bordercolor": "#444", "borderwidth": 3, "height": 30
},
{
"xref":"x3","yref":"y3","text":"Left<br>no<br>arrow","y":1.5,"x":1,"showarrow":false,
"bordercolor": "#444", "bgcolor": "#eee", "width": 50, "height": 60, "textangle": 10,
"align": "right", "valign": "bottom"
},
{"xref":"x3","yref":"y3","text":"Right<br>no<br>arrow","y":1.5,"x":2,"showarrow":false},
{"xref":"x3","yref":"y3","text":"Bottom<br>no<br>arrow","y":1,"x":1.5,"showarrow":false},
{"xref":"x3","yref":"y3","text":"Top<br>no<br>arrow","y":2,"x":1.5,"showarrow":false}
{
"xref":"x3","yref":"y3","text":"Bottom<br>no<br>arrow","y":1,"x":1.5,"showarrow":false,
"bgcolor": "#eee", "width": 30, "height": 40, "textangle":-10,
"align": "left", "valign": "top"
},
{"xref":"x3","yref":"y3","text":"Top<br>no<br>arrow","y":2,"x":1.5,"showarrow":false},
{
"xref": "paper", "yref": "paper", "text": "On the<br>bottom of the plot",
"x": 0.3, "y": -0.1, "showarrow": false,
"xanchor": "right", "yanchor": "top", "width": 200, "height": 60,
"bgcolor": "#eee", "bordercolor": "#444"
},
{
"xref": "paper", "yref": "paper", "text": "blah blah blah blah<br>blah<br>blah<br>blah<br>blah<br>blah",
"x": 0.3, "y": -0.25, "ax": 100, "ay": 0, "textangle": 40, "borderpad": 4,
"xanchor": "left", "yanchor": "bottom", "align": "left", "valign": "top",
"width": 60, "height": 40, "bgcolor": "#eee", "bordercolor": "#444"
}
]
}
}