Skip to content

Annotation size & text alignment #1551

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.

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 8 commits into from
Apr 6, 2017
Merged
Changes from all 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
7 changes: 6 additions & 1 deletion src/components/annotations/annotation_defaults.js
Original file line number Diff line number Diff line change
@@ -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'),
@@ -45,6 +44,12 @@ module.exports = function handleAnnotationDefaults(annIn, annOut, fullLayout, op
coerce('textangle');
Lib.coerceFont(coerce, 'font', fullLayout.font);

coerce('width');
coerce('align');

var h = coerce('height');
if(h) coerce('valign');

// positioning
var axLetters = ['x', 'y'],
arrowPosDflt = [-10, -30],
40 changes: 36 additions & 4 deletions src/components/annotations/attributes.js
Original file line number Diff line number Diff line change
@@ -49,6 +49,27 @@ module.exports = {
font: extendFlat({}, fontAttrs, {
description: 'Sets the annotation text font.'
}),
width: {
valType: 'number',
min: 1,
dflt: null,
role: 'style',
description: [
'Sets an explicit width for the text box. null (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: 1,
dflt: null,
role: 'style',
description: [
'Sets an explicit height for the text box. null (default) lets the',
'text set the box height. Taller text will be clipped.'
].join(' ')
},
opacity: {
valType: 'number',
min: 0,
@@ -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: {
80 changes: 60 additions & 20 deletions src/components/annotations/draw.js
Original file line number Diff line number Diff line change
@@ -72,9 +72,14 @@ function drawOne(gd, index) {
var optionsIn = (layout.annotations || [])[index],
options = fullLayout.annotations[index];

var annClipID = 'clip' + fullLayout._uid + '_ann' + index;

// this annotation is gone - quit now after deleting it
// TODO: use d3 idioms instead of deleting and redrawing every time
if(!optionsIn || options.visible === false) return;
if(!optionsIn || options.visible === false) {
d3.selectAll('#' + annClipID).remove();
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

easy.

}

var xa = Axes.getFromId(gd, options.xref),
ya = Axes.getFromId(gd, options.yref),
@@ -118,6 +123,18 @@ function drawOne(gd, index) {
.call(Color.stroke, options.bordercolor)
.call(Color.fill, options.bgcolor);

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

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')
@@ -144,19 +161,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') {
@@ -181,8 +200,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'],
@@ -299,22 +318,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
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
@@ -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"},
@@ -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"
}
]
}
}
48 changes: 48 additions & 0 deletions test/jasmine/tests/annotations_test.js
Original file line number Diff line number Diff line change
@@ -1041,3 +1041,51 @@ describe('annotation dragging', function() {
.then(done);
});
});

describe('annotation clip paths', function() {
var gd;

beforeEach(function(done) {
gd = createGraphDiv();

// we've already tested autorange with relayout, so fix the geometry
// completely so we know exactly what we're dealing with
// plot area is 300x300, and covers data range 100x100
Plotly.plot(gd, [{x: [0, 100], y: [0, 100]}], {
annotations: [
{x: 50, y: 50, text: 'hi', width: 50},
{x: 20, y: 20, text: 'bye', height: 40},
{x: 80, y: 80, text: 'why?'}
]
})
.then(done);
});

afterEach(destroyGraphDiv);

it('should only make the clippaths it needs and delete others', function(done) {
expect(d3.select(gd).selectAll('.annclip').size()).toBe(2);

Plotly.relayout(gd, {'annotations[0].visible': false})
.then(function() {
expect(d3.select(gd).selectAll('.annclip').size()).toBe(1);

return Plotly.relayout(gd, {'annotations[2].width': 20});
})
.then(function() {
expect(d3.select(gd).selectAll('.annclip').size()).toBe(2);

return Plotly.relayout(gd, {'annotations[1].height': null});
})
.then(function() {
expect(d3.select(gd).selectAll('.annclip').size()).toBe(1);

return Plotly.relayout(gd, {'annotations[2]': null});
})
.then(function() {
expect(d3.select(gd).selectAll('.annclip').size()).toBe(0);
})
.catch(failTest)
.then(done);
});
});