Skip to content

Introducing 3D annotations #1638

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 29 commits into from
May 26, 2017
Merged

Introducing 3D annotations #1638

Changes from 3 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
aa58118
replace forEach with for-loop over ['x', 'y']
etpinard Apr 28, 2017
8b8bdea
don't (uselessly) add data-index to ann text+bg & arrow groups
etpinard Apr 28, 2017
1a8f1ed
don't (uselessly) add annotation class to ann text group inner
etpinard Apr 28, 2017
f4b14e6
fine tune annotation arrow selector
etpinard Apr 28, 2017
4dc16e4
use ax.?2? while composing convert methods
etpinard Apr 28, 2017
8e6ab7d
split annotation drawOne into drawRaw
etpinard Apr 28, 2017
c5f6eb2
introduce 3D annotations
etpinard Apr 28, 2017
b7ab656
don't try to edit x,y,z positions for 3D annotations
etpinard Apr 28, 2017
fbdcee8
Merge branch 'master' into gl3d-annotations
etpinard May 9, 2017
981b1a1
Merge branch 'master' into gl3d-annotations
etpinard May 16, 2017
94b940d
rename anchor -> anchor3 so that it doesn't conflict w/ for-loop block
etpinard May 16, 2017
8ac2551
Merge branch 'master' into gl3d-annotations
etpinard May 24, 2017
ababc49
Revert "don't (uselessly) add annotation class to ann text group inner"
etpinard May 24, 2017
7cfba36
fix 3d annotations on type: 'category' axes
etpinard May 24, 2017
8f9e370
make sure to clear 3d annotations when parent scene gets removed
etpinard May 24, 2017
1bbabad
implement autorange logic for annotations
etpinard May 24, 2017
9520943
first pass 3d annotations jasmine tests
etpinard May 24, 2017
f61cd4d
first pass 3d annotation image test support
etpinard May 24, 2017
8cb10c1
try to make test that update the scene camera more robust
etpinard May 25, 2017
a31549d
replace 'annotation' class for 'annotation-text' for annation text nodes
etpinard May 25, 2017
bd9c867
mv 3d annotations code out of plots/gl3d into component/annotations3d
etpinard May 25, 2017
21c8cc6
add common annotations defaults modules
etpinard May 25, 2017
adfc305
declare `ax` and `ay` as `valType: number` for annotation3d
etpinard May 25, 2017
c084da0
rename _scene ref to scene id in annotation container _sceneId
etpinard May 25, 2017
ef985cf
put 'captureevents' under common annotation defaults
etpinard May 25, 2017
0b4214f
Merge branch 'master' into gl3d-annotations
etpinard May 25, 2017
8b8a088
pass subplotId as arg to Annotations.drawRaw
etpinard May 26, 2017
1d5606a
include subplotId in clickannotation event data
etpinard May 26, 2017
46ab63d
rm subplotId key from clickannotations event data when falsy
etpinard May 26, 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
20 changes: 9 additions & 11 deletions src/components/annotations/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ 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));
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 another relic 🔪

.classed('annotation-text-g', true);

var annTextGroupInner = annTextGroup.append('g')
.style('pointer-events', options.captureevents ? 'all' : null)
Expand Down Expand Up @@ -169,7 +168,6 @@ function drawOne(gd, index) {
var font = options.font;

var annText = annTextGroupInner.append('text')
.classed('annotation', true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This commit is reverted in ababc49 to make the tests added in #1683 ✅

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you've hit on a flaw in how github displays comments... I'm reading:

This commit is reverted in ababc49 to make the tests added in #1683 ✅

but I'm looking at a31549d which came after ababc49 and rereverts the change originally made in 1a8f1ed 💫

Anyway, totally approve of this change. My best guess is these got the same .annotation class because at some point in the distant past they weren't all encapsulated in one group like they are now, and I wanted to be able to more easily select the whole annotation. Has always been confusing to deal with...

Copy link
Contributor Author

@etpinard etpinard May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

In brief, as of a31549d, the annotations text nodes have class name annotation-text.

.attr('data-unformatted', options.text)
.text(options.text);

Expand Down Expand Up @@ -225,10 +223,13 @@ function drawOne(gd, index) {
}

var annotationIsOffscreen = false;
['x', 'y'].forEach(function(axLetter) {
var axRef = options[axLetter + 'ref'] || axLetter,
var letters = ['x', 'y'];

for(var i = 0; i < letters.length; i++) {
var axLetter = letters[i],
axRef = options[axLetter + 'ref'] || axLetter,
tailRef = options['a' + axLetter + 'ref'],
ax = Axes.getFromId(gd, axRef),
ax = {x: xa, y: ya}[axLetter],
dimAngle = (textangle + (axLetter === 'x' ? 0 : -90)) * Math.PI / 180,
// note that these two can be either positive or negative
annSizeFromWidth = outerWidth * Math.cos(dimAngle),
Expand Down Expand Up @@ -350,7 +351,7 @@ function drawOne(gd, index) {
// size/shift are used during dragging
options['_' + axLetter + 'size'] = annSize;
options['_' + axLetter + 'shift'] = textShift;
});
}

if(annotationIsOffscreen) {
annTextGroupInner.remove();
Expand Down Expand Up @@ -471,8 +472,7 @@ function drawOne(gd, index) {

var arrowGroup = annGroup.append('g')
.style({opacity: Color.opacity(arrowColor)})
.classed('annotation-arrow-g', true)
.attr('data-index', String(index));
.classed('annotation-arrow-g', true);

var arrow = arrowGroup.append('path')
.attr('d', 'M' + tailX + ',' + tailY + 'L' + headX + ',' + headY)
Expand All @@ -492,10 +492,8 @@ function drawOne(gd, index) {
arrowDragHeadY += options.standoff * (tailY - headY) / arrowLength;
}
var arrowDrag = arrowGroup.append('path')
.classed('annotation', true)
.classed('anndrag', true)
.attr({
'data-index': String(index),
d: 'M3,3H-3V-3H3ZM0,0L' + (tailX - arrowDragHeadX) + ',' + (tailY - arrowDragHeadY),
transform: 'translate(' + arrowDragHeadX + ',' + arrowDragHeadY + ')'
})
Expand Down