Skip to content

Add "arrowanchor" property to annotations #2164

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 6 commits into from
Dec 7, 2017
Merged

Add "arrowanchor" property to annotations #2164

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f598c92
add arrowanchor property
apalchys Nov 16, 2017
c4a0f1f
add 'startarrowhead', 'startarrowsize' and 'startstandoff' and renam…
apalchys Nov 21, 2017
19a06c6
update auto range based on startarrowsize and baseline tests
apalchys Dec 6, 2017
42138e7
fix tests
apalchys Dec 7, 2017
191d5d2
coerce arrowhead, arrowsize and startarrowsize only when it is needed
apalchys Dec 7, 2017
8e75b74
update baseline image for gl2d_annotations
apalchys Dec 7, 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
49 changes: 46 additions & 3 deletions src/components/annotations/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,25 @@ module.exports = {
dflt: 1,
role: 'style',
editType: 'arraydraw',
description: 'Sets the annotation arrow head style.'
description: 'Sets the end annotation arrow head style.'
},
startarrowhead: {
valType: 'integer',
min: 0,
max: ARROWPATHS.length,
dflt: 1,
role: 'style',
editType: 'arraydraw',
description: 'Sets the start annotation arrow head style.'
},
arrowside: {
valType: 'flaglist',
flags: ['end', 'start'],
extras: ['none'],
dflt: 'end',
role: 'style',
editType: 'arraydraw',
description: 'Sets the annotation arrow head position.'
},
arrowsize: {
valType: 'number',
Expand All @@ -182,7 +200,18 @@ module.exports = {
role: 'style',
editType: 'calcIfAutorange',
description: [
'Sets the size of the annotation arrow head, relative to `arrowwidth`.',
'Sets the size of the end annotation arrow head, relative to `arrowwidth`.',
'A value of 1 (default) gives a head about 3x as wide as the line.'
].join(' ')
},
startarrowsize: {
valType: 'number',
min: 0.3,
dflt: 1,
role: 'style',
editType: 'calcIfAutorange',
description: [
'Sets the size of the start annotation arrow head, relative to `arrowwidth`.',
'A value of 1 (default) gives a head about 3x as wide as the line.'
].join(' ')
},
Expand All @@ -200,7 +229,21 @@ module.exports = {
role: 'style',
editType: 'calcIfAutorange',
description: [
'Sets a distance, in pixels, to move the arrowhead away from the',
'Sets a distance, in pixels, to move the end arrowhead away from the',
'position it is pointing at, for example to point at the edge of',
'a marker independent of zoom. Note that this shortens the arrow',
'from the `ax` / `ay` vector, in contrast to `xshift` / `yshift`',
'which moves everything by this amount.'
].join(' ')
},
startstandoff: {
valType: 'number',
min: 0,
dflt: 0,
role: 'style',
editType: 'calcIfAutorange',
description: [
'Sets a distance, in pixels, to move the start arrowhead away from the',
'position it is pointing at, for example to point at the edge of',
'a marker independent of zoom. Note that this shortens the arrow',
'from the `ax` / `ay` vector, in contrast to `xshift` / `yshift`',
Expand Down
29 changes: 19 additions & 10 deletions src/components/annotations/calc_autorange.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ function annAutorange(gd) {
Lib.filterVisible(fullLayout.annotations).forEach(function(ann) {
var xa = Axes.getFromId(gd, ann.xref),
ya = Axes.getFromId(gd, ann.yref),
headSize = 3 * ann.arrowsize * ann.arrowwidth || 0;
headSize = 3 * ann.arrowsize * ann.arrowwidth || 0,
startHeadSize = 3 * ann.startarrowsize * ann.arrowwidth || 0;

var headPlus, headMinus;
var headPlus, headMinus, startHeadPlus, startHeadMinus;

if(xa && xa.autorange) {
headPlus = headSize + ann.xshift;
headMinus = headSize - ann.xshift;
startHeadPlus = startHeadSize + ann.xshift;
startHeadMinus = startHeadSize - ann.xshift;

if(ann.axref === ann.xref) {
// expand for the arrowhead (padded by arrowhead)
Expand All @@ -64,36 +67,42 @@ function annAutorange(gd) {
});
// again for the textbox (padded by textbox)
Axes.expand(xa, [xa.r2c(ann.ax)], {
ppadplus: ann._xpadplus,
ppadminus: ann._xpadminus
ppadplus: Math.max(ann._xpadplus, startHeadPlus),
ppadminus: Math.max(ann._xpadminus, startHeadMinus)
});
}
else {
startHeadPlus = ann.ax ? startHeadPlus + ann.ax : startHeadPlus;
startHeadMinus = ann.ax ? startHeadMinus - ann.ax : startHeadMinus;
Axes.expand(xa, [xa.r2c(ann.x)], {
ppadplus: Math.max(ann._xpadplus, headPlus),
ppadminus: Math.max(ann._xpadminus, headMinus)
ppadplus: Math.max(ann._xpadplus, headPlus, startHeadPlus),
ppadminus: Math.max(ann._xpadminus, headMinus, startHeadMinus)
});
}
}

if(ya && ya.autorange) {
headPlus = headSize - ann.yshift;
headMinus = headSize + ann.yshift;
startHeadPlus = startHeadSize - ann.yshift;
startHeadMinus = startHeadSize + ann.yshift;

if(ann.ayref === ann.yref) {
Axes.expand(ya, [ya.r2c(ann.y)], {
ppadplus: headPlus,
ppadminus: headMinus
});
Axes.expand(ya, [ya.r2c(ann.ay)], {
ppadplus: ann._ypadplus,
ppadminus: ann._ypadminus
ppadplus: Math.max(ann._ypadplus, startHeadPlus),
ppadminus: Math.max(ann._ypadminus, startHeadMinus)
});
}
else {
startHeadPlus = ann.ay ? startHeadPlus + ann.ay : startHeadPlus;
startHeadMinus = ann.ay ? startHeadMinus - ann.ay : startHeadMinus;
Axes.expand(ya, [ya.r2c(ann.y)], {
ppadplus: Math.max(ann._ypadplus, headPlus),
ppadminus: Math.max(ann._ypadminus, headMinus)
ppadplus: Math.max(ann._ypadplus, headPlus, startHeadPlus),
ppadminus: Math.max(ann._ypadminus, headMinus, startHeadMinus)
});
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/components/annotations/common_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,18 @@ module.exports = function handleAnnotationCommonDefaults(annIn, annOut, fullLayo
if(h) coerce('valign');

if(showArrow) {
var arrowside = coerce('arrowside');
var arrowhead = coerce('arrowhead');
var arrowsize = coerce('arrowsize');

if(arrowside.indexOf('start') !== -1) {
coerce('startarrowhead', arrowhead);
Copy link
Contributor

Choose a reason for hiding this comment

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

coerce('startarrowsize', arrowsize);

and

coerce('startstandoff');

should be inside this block as well - as they have no effect w/o start arrow.

Copy link
Contributor Author

@apalchys apalchys Nov 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

There was possibility to use standoff with arrowhead: 0 (no arrow) before my changes and I decided to keep this functionality for ‘startstandoff’ -> when we have arrowside: none, we still can set both properties of standoff and see the result.

Please just confirm that it is not expected behaviour and I will apply suggested changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ha good point. I think you have it right here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, so (start)standoff always needs to be coerced - but doesn't startarrowsize go in here?

Also if we have no 'end' we don't need arrowhead and arrowsize do we?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like @apalchys posted and deleted a comment, so maybe he noticed what I'm about to say... (incidentally, if that's the case, let me encourage you to edit comments, using strikethrough if necessary, rather than deleting them, as it makes the conversation flow clearer)

(start)arrowhead and (start)arrowsize are not needed logically when we're not showing a head on that end, so that's a warning sign that providing them could lead to bugs. Indeed, I think there is a subtle one: if you choose an arrowhead that has a backoff (including the default arrowhead: 1! backoff is how much you need to move the end of the line backward so the arrowhead can point to the right place without the line jutting through it) but you don't show that arrowhead, you'll get an excessive standoff.

options.arrowhead || 0 ensures an un-coerced arrowhead is treated as no arrow, and we may need options.arrowsize || 0 (this value probably doesn't matter as long as it's numeric) to make the math well-defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I recognized that my answer is wrong right after I submit it. I will commit an update soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check my commit - 191d5d2
Is it what you expected?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect, thanks! Looks like this tweaked one more test image (gl2d_annotations) but once that's fixed I think we're ready to go 🎉

}
coerce('startarrowsize', arrowsize);
coerce('arrowcolor', borderOpacity ? annOut.bordercolor : Color.defaultLine);
coerce('arrowhead');
coerce('arrowsize');
coerce('arrowwidth', ((borderOpacity && borderWidth) || 1) * 2);
coerce('standoff');
coerce('startstandoff');

}

Expand Down
5 changes: 3 additions & 2 deletions src/components/annotations/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
});

var strokewidth = options.arrowwidth,
arrowColor = options.arrowcolor;
arrowColor = options.arrowcolor,
arrowSide = options.arrowside;

var arrowGroup = annGroup.append('g')
.style({opacity: Color.opacity(arrowColor)})
Expand All @@ -520,7 +521,7 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
.style('stroke-width', strokewidth + 'px')
.call(Color.stroke, Color.rgb(arrowColor));

drawArrowHead(arrow, 'end', options);
drawArrowHead(arrow, arrowSide, options);

// the arrow dragger is a small square right at the head, then a line to the tail,
// all expanded by a stroke width of 6px plus the arrow line width
Expand Down
95 changes: 55 additions & 40 deletions src/components/annotations/draw_arrow_head.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ var ARROWPATHS = require('./arrow_paths');
*
* @param {d3.selection} el3: a d3-selected line or path element
*
* @param {string} ends: 'start', 'end', or 'start+end' for which ends get arrowheads
* @param {string} ends: 'none', 'start', 'end', or 'start+end' for which ends get arrowheads
*
* @param {object} options: style information. Must have all the following:
* @param {number} options.arrowhead: head style - see ./arrow_paths
* @param {number} options.arrowsize: relative size of the head vs line width
* @param {number} options.standoff: distance in px to move the arrow point from its target
* @param {number} options.arrowhead: end head style - see ./arrow_paths
* @param {number} options.startarrowhead: start head style - see ./arrow_paths
* @param {number} options.arrowsize: relative size of the end head vs line width
* @param {number} options.startarrowsize: relative size of the start head vs line width
* @param {number} options.standoff: distance in px to move the end arrow point from its target
* @param {number} options.startstandoff: distance in px to move the start arrow point from its target
* @param {number} options.arrowwidth: width of the arrow line
* @param {string} options.arrowcolor: color of the arrow line, for the head to match
* Note that the opacity of this color is ignored, as it's assumed the container
Expand All @@ -35,10 +38,13 @@ var ARROWPATHS = require('./arrow_paths');
module.exports = function drawArrowHead(el3, ends, options) {
var el = el3.node();
var headStyle = ARROWPATHS[options.arrowhead || 0];
var startHeadStyle = ARROWPATHS[options.startarrowhead || 0];
var scale = (options.arrowwidth || 1) * options.arrowsize;
var startScale = (options.arrowwidth || 1) * options.startarrowsize;
var doStart = ends.indexOf('start') >= 0;
var doEnd = ends.indexOf('end') >= 0;
var backOff = headStyle.backoff * scale + options.standoff;
var startBackOff = startHeadStyle.backoff * startScale + options.startstandoff;

var start, end, startRot, endRot;

Expand All @@ -51,6 +57,13 @@ module.exports = function drawArrowHead(el3, ends, options) {

startRot = Math.atan2(dy, dx);
endRot = startRot + Math.PI;
if(backOff && startBackOff) {
if(backOff + startBackOff > Math.sqrt(dx * dx + dy * dy)) {
hideLine();
return;
}
}

if(backOff) {
if(backOff * backOff > dx * dx + dy * dy) {
hideLine();
Expand All @@ -59,16 +72,24 @@ module.exports = function drawArrowHead(el3, ends, options) {
var backOffX = backOff * Math.cos(startRot),
backOffY = backOff * Math.sin(startRot);

if(doStart) {
start.x -= backOffX;
start.y -= backOffY;
el3.attr({x1: start.x, y1: start.y});
}
if(doEnd) {
end.x += backOffX;
end.y += backOffY;
el3.attr({x2: end.x, y2: end.y});
end.x += backOffX;
end.y += backOffY;
el3.attr({x2: end.x, y2: end.y});

}

if(startBackOff) {
if(startBackOff * startBackOff > dx * dx + dy * dy) {
hideLine();
return;
}
var startBackOffX = startBackOff * Math.cos(startRot),
startbackOffY = startBackOff * Math.sin(startRot);

start.x -= startBackOffX;
start.y -= startbackOffY;
el3.attr({x1: start.x, y1: start.y});

}
}
else if(el.nodeName === 'path') {
Expand All @@ -79,59 +100,53 @@ module.exports = function drawArrowHead(el3, ends, options) {
// combine the two
dashArray = '';

if(pathlen < backOff) {
if(pathlen < backOff + startBackOff) {
hideLine();
return;
}

if(doStart) {
var start0 = el.getPointAtLength(0);
var dstart = el.getPointAtLength(0.1);

startRot = Math.atan2(start0.y - dstart.y, start0.x - dstart.x);
start = el.getPointAtLength(Math.min(backOff, pathlen));
var start0 = el.getPointAtLength(0);
var dstart = el.getPointAtLength(0.1);

if(backOff) dashArray = '0px,' + backOff + 'px,';
}
startRot = Math.atan2(start0.y - dstart.y, start0.x - dstart.x);
start = el.getPointAtLength(Math.min(startBackOff, pathlen));

if(doEnd) {
var end0 = el.getPointAtLength(pathlen);
var dend = el.getPointAtLength(pathlen - 0.1);
dashArray = '0px,' + startBackOff + 'px,';

endRot = Math.atan2(end0.y - dend.y, end0.x - dend.x);
end = el.getPointAtLength(Math.max(0, pathlen - backOff));
var end0 = el.getPointAtLength(pathlen);
var dend = el.getPointAtLength(pathlen - 0.1);

if(backOff) {
var shortening = dashArray ? 2 * backOff : backOff;
dashArray += (pathlen - shortening) + 'px,' + pathlen + 'px';
}
}
else if(dashArray) dashArray += pathlen + 'px';
endRot = Math.atan2(end0.y - dend.y, end0.x - dend.x);
end = el.getPointAtLength(Math.max(0, pathlen - backOff));

var shortening = dashArray ? startBackOff + backOff : backOff;
dashArray += (pathlen - shortening) + 'px,' + pathlen + 'px';

if(dashArray) el3.style('stroke-dasharray', dashArray);
el3.style('stroke-dasharray', dashArray);
}

function hideLine() { el3.style('stroke-dasharray', '0px,100px'); }

function drawhead(p, rot) {
if(!headStyle.path) return;
if(headStyle.noRotate) rot = 0;
function drawhead(arrowHeadStyle, p, rot, arrowScale) {
if(!arrowHeadStyle.path) return;
if(arrowHeadStyle.noRotate) rot = 0;

d3.select(el.parentNode).append('path')
.attr({
'class': el3.attr('class'),
d: headStyle.path,
d: arrowHeadStyle.path,
transform:
'translate(' + p.x + ',' + p.y + ')' +
(rot ? 'rotate(' + (rot * 180 / Math.PI) + ')' : '') +
'scale(' + scale + ')'
'scale(' + arrowScale + ')'
})
.style({
fill: Color.rgb(options.arrowcolor),
'stroke-width': 0
});
}

if(doStart) drawhead(start, startRot);
if(doEnd) drawhead(end, endRot);
if(doStart) drawhead(startHeadStyle, start, startRot, startScale);
if(doEnd) drawhead(headStyle, end, endRot, scale);
};
4 changes: 4 additions & 0 deletions src/components/annotations3d/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,13 @@ module.exports = overrideAll({
showarrow: annAtts.showarrow,
arrowcolor: annAtts.arrowcolor,
arrowhead: annAtts.arrowhead,
startarrowhead: annAtts.startarrowhead,
arrowside: annAtts.arrowside,
arrowsize: annAtts.arrowsize,
startarrowsize: annAtts.startarrowsize,
arrowwidth: annAtts.arrowwidth,
standoff: annAtts.standoff,
startstandoff: annAtts.startstandoff,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this really work in 3D or are these attributes there just to make the supply-default function not error out?

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, it works fine

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. Would you mind adding an annotations item showing off the new attributes to gl3d_annotations.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

hovertext: annAtts.hovertext,
hoverlabel: annAtts.hoverlabel,
captureevents: annAtts.captureevents,
Expand Down
Binary file modified test/image/baselines/annotations-autorange.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/annotations.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/category-autorange.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl2d_annotations.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_annotations.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading