Skip to content

Commit 4203088

Browse files
nicholas-esterernicholas-esterer
committed
PR review integration
1 parent aa2d6eb commit 4203088

File tree

6 files changed

+37
-55
lines changed

6 files changed

+37
-55
lines changed

‎src/components/annotations/draw.js

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ function drawOne(gd, index) {
7878
// the plot.
7979
// axDomainRef: if true and axa defined, draws relative to axis domain,
8080
// otherwise draws relative to data (if axa defined) or paper (if not).
81-
function shiftPosition(axa, optAx, dAx, gsDim, vertical, axDomainRef) {
81+
function shiftPosition(axa, optAx, dAx, gsDim, vertical, axRef) {
82+
var axDomainRef = Axes.getRefType(axRef) === 'domain';
8283
if(axa) {
8384
if(axDomainRef) {
8485
// here optAx normalized to length of axis (e.g., normally in range
@@ -339,20 +340,17 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
339340
basePx = ax._offset + ax.r2p(options[axLetter]);
340341
autoAlignFraction = 0.5;
341342
} else {
343+
var axRefTypeEqDomain = axRefType === 'domain';
342344
if(axLetter === 'x') {
343345
alignPosition = options[axLetter];
344-
if(axRefType === 'domain') {
345-
basePx = ax._offset + ax._length * alignPosition;
346-
} else {
346+
basePx = axRefTypeEqDomain ?
347+
ax._offset + ax._length * alignPosition :
347348
basePx = gs.l + gs.w * alignPosition;
348-
}
349349
} else {
350350
alignPosition = 1 - options[axLetter];
351-
if(axRefType === 'domain') {
352-
basePx = ax._offset + ax._length * alignPosition;
353-
} else {
351+
basePx = axRefTypeEqDomain ?
352+
ax._offset + ax._length * alignPosition :
354353
basePx = gs.t + gs.h * alignPosition;
355-
}
356354
}
357355
autoAlignFraction = options.showarrow ? 0.5 : alignPosition;
358356
}
@@ -612,24 +610,22 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
612610
var ycenter = annxy0[1] + dy;
613611
annTextGroupInner.call(Drawing.setTranslate, xcenter, ycenter);
614612

615-
var xRefType = Axes.getRefType(options.xref);
616-
var yRefType = Axes.getRefType(options.yref);
617613
modifyItem('x',
618-
shiftPosition(xa, options.x, dx, gs.w, false, xRefType === 'domain'));
614+
shiftPosition(xa, options.x, dx, gs.w, false, options.xref));
619615
modifyItem('y',
620-
shiftPosition(ya, options.y, dy, gs.h, true, yRefType === 'domain'));
616+
shiftPosition(ya, options.y, dy, gs.h, true, options.yref));
621617

622618
// for these 2 calls to shiftPosition, it is assumed xa, ya are
623619
// defined, so gsDim will not be used, but we put it in
624620
// anyways for consistency
625621
if(options.axref === options.xref) {
626622
modifyItem('ax', shiftPosition(xa, options.ax, dx, gs.w, false,
627-
xRefType === 'domain'));
623+
options.xref));
628624
}
629625

630626
if(options.ayref === options.yref) {
631627
modifyItem('ay', shiftPosition(ya, options.ay, dy, gs.h, true,
632-
yRefType === 'domain'));
628+
options.yref));
633629
}
634630

635631
arrowGroup.attr('transform', 'translate(' + dx + ',' + dy + ')');
@@ -663,22 +659,20 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
663659
},
664660
moveFn: function(dx, dy) {
665661
var csr = 'pointer';
666-
var xRefType = Axes.getRefType(options.xref);
667-
var yRefType = Axes.getRefType(options.yref);
668662
if(options.showarrow) {
669663
// for these 2 calls to shiftPosition, it is assumed xa, ya are
670664
// defined, so gsDim will not be used, but we put it in
671665
// anyways for consistency
672666
if(options.axref === options.xref) {
673667
modifyItem('ax', shiftPosition(xa, options.ax, dx, gs.h, false,
674-
xRefType === 'domain'));
668+
options.xref));
675669
} else {
676670
modifyItem('ax', options.ax + dx);
677671
}
678672

679673
if(options.ayref === options.yref) {
680674
modifyItem('ay', shiftPosition(ya, options.ay, dy, gs.w, true,
681-
yRefType === 'domain'));
675+
options.yref));
682676
} else {
683677
modifyItem('ay', options.ay + dy);
684678
}
@@ -690,7 +684,7 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
690684
// shiftPosition will not execute code where xa was
691685
// undefined, so we use to calculate xUpdate too
692686
xUpdate = shiftPosition(xa, options.x, dx, gs.h, false,
693-
xRefType === 'domain');
687+
options.xref);
694688
} else {
695689
var widthFraction = options._xsize / gs.w;
696690
var xLeft = options.x + (options._xshift - options.xshift) / gs.w - widthFraction / 2;
@@ -703,7 +697,7 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
703697
// shiftPosition will not execute code where ya was
704698
// undefined, so we use to calculate yUpdate too
705699
yUpdate = shiftPosition(ya, options.y, dy, gs.w, true,
706-
yRefType === 'domain');
700+
options.yref);
707701
} else {
708702
var heightFraction = options._ysize / gs.h;
709703
var yBottom = options.y - (options._yshift + options.yshift) / gs.h - heightFraction / 2;

‎src/components/images/draw.js

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -135,20 +135,16 @@ module.exports = function draw(gd) {
135135
var size = fullLayout._size;
136136
var width, height;
137137
if(xa !== undefined) {
138-
if((typeof(d.xref) === 'string') && d.xref.endsWith(' domain')) {
139-
width = xa._length * d.sizex;
140-
} else {
141-
width = Math.abs(xa.l2p(d.sizex) - xa.l2p(0));
142-
}
138+
width = ((typeof(d.xref) === 'string') && d.xref.endsWith(' domain')) ?
139+
xa._length * d.sizex :
140+
Math.abs(xa.l2p(d.sizex) - xa.l2p(0));
143141
} else {
144142
width = d.sizex * size.w;
145143
}
146144
if(ya !== undefined) {
147-
if((typeof(d.yref) === 'string') && d.yref.endsWith(' domain')) {
148-
height = ya._length * d.sizey;
149-
} else {
150-
height = Math.abs(ya.l2p(d.sizey) - ya.l2p(0));
151-
}
145+
height = ((typeof(d.yref) === 'string') && d.yref.endsWith(' domain')) ?
146+
ya._length * d.sizey :
147+
Math.abs(ya.l2p(d.sizey) - ya.l2p(0));
152148
} else {
153149
height = d.sizey * size.h;
154150
}
@@ -162,23 +158,19 @@ module.exports = function draw(gd) {
162158
// Final positions
163159
var xPos, yPos;
164160
if(xa !== undefined) {
165-
if((typeof(d.xref) === 'string') && d.xref.endsWith(' domain')) {
166-
xPos = xa._length * d.x + xa._offset;
167-
} else {
168-
xPos = xa.r2p(d.x) + xa._offset;
169-
}
161+
xPos = ((typeof(d.xref) === 'string') && d.xref.endsWith(' domain')) ?
162+
xa._length * d.x + xa._offset :
163+
xa.r2p(d.x) + xa._offset;
170164
} else {
171165
xPos = d.x * size.w + size.l;
172166
}
173167
xPos += xOffset;
174168
if(ya !== undefined) {
175-
if((typeof(d.yref) === 'string') && d.yref.endsWith(' domain')) {
169+
yPos = ((typeof(d.yref) === 'string') && d.yref.endsWith(' domain')) ?
176170
// consistent with "paper" yref value, where positive values
177171
// move up the page
178-
yPos = ya._length * (1 - d.y) + ya._offset;
179-
} else {
180-
yPos = ya.r2p(d.y) + ya._offset;
181-
}
172+
ya._length * (1 - d.y) + ya._offset :
173+
ya.r2p(d.y) + ya._offset;
182174
} else {
183175
yPos = size.h - d.y * size.h + size.t;
184176
}

‎src/components/shapes/helpers.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ exports.getPixelToData = function(gd, axis, isVertical, opt) {
9292
if(axis) {
9393
if(opt === 'domain') {
9494
pixelToData = function(p) {
95+
var q = p - axis._offset;
9596
return ((isVertical ?
96-
(1 - (p - axis._offset) / axis._length) :
97-
(p - axis._offset) / axis._length));
97+
(1 - q / axis._length) :
98+
q / axis._length));
9899
};
99100
} else {
100101
var r2d = exports.rangeToShapePosition(axis);

‎src/plot_api/helpers.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ exports.cleanLayout = function(layout) {
174174
cleanAxRef(shape, 'yref');
175175
}
176176

177-
var imagesLen = Array.isArray(layout.images) ? layout.images.length : 0;
177+
var imagesLen = isArray(layout.images) ? layout.images.length : 0;
178178
for(i = 0; i < imagesLen; i++) {
179179
var image = layout.images[i];
180180

‎src/plots/cartesian/axes.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption
9595

9696
if(!dflt) dflt = axlist[0] || (typeof extraOption === 'string' ? extraOption : extraOption[0]);
9797
if(!extraOption) extraOption = dflt;
98-
if(domainRef) axlist = axlist.concat(axlist.map(function(x) { return x + ' domain'; }));
98+
if(domainRef) axlist.push(axlist.map(function(x) { return x + ' domain'; }));
9999

100100
// data-ref annotations are not supported in gl2d yet
101101

‎src/plots/cartesian/axis_ids.js

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,13 @@ exports.name2id = function name2id(name) {
3737
* ' domain' part is kept at the end of the axis ID string.
3838
*/
3939
exports.cleanId = function cleanId(id, axLetter, domainId) {
40+
var domainTest = /( domain)$/.test(id);
4041
if(typeof id !== 'string' || !id.match(constants.AX_ID_PATTERN)) return;
4142
if(axLetter && id.charAt(0) !== axLetter) return;
42-
if(/( domain)$/.test(id) && (!domainId)) return;
43+
if(domainTest && (!domainId)) return;
4344
var axNum = id.split(' ')[0].substr(1).replace(/^0+/, '');
4445
if(axNum === '1') axNum = '';
45-
if(/( domain)$/.test(id) && domainId) {
46-
return id.charAt(0) + axNum + ' domain';
47-
}
48-
return id.charAt(0) + axNum;
46+
return id.charAt(0) + axNum + (domainTest && domainId ? ' domain' : '');
4947
};
5048

5149
// get all axis objects, as restricted in listNames
@@ -92,7 +90,7 @@ exports.listIds = function(gd, axLetter) {
9290
exports.getFromId = function(gd, id, type) {
9391
var fullLayout = gd._fullLayout;
9492
// remove "domain" suffix
95-
id = ((id === undefined) || (typeof(id) !== 'string')) ? id : id.replace(/ *domain/, '');
93+
id = ((id === undefined) || (typeof(id) !== 'string')) ? id : id.replace(' domain', '');
9694

9795
if(type === 'x') id = id.replace(/y[0-9]*/, '');
9896
else if(type === 'y') id = id.replace(/x[0-9]*/, '');
@@ -146,8 +144,5 @@ exports.ref2id = function(ar) {
146144
// This assumes ar has been coerced via coerceRef, and uses the shortcut of
147145
// checking if the first letter matches [xyz] to determine if it should
148146
// return the axis ID. Otherwise it returns false.
149-
if(/^[xyz]/.test(ar)) {
150-
return ar.split(' ')[0];
151-
}
152-
return false;
147+
return (/^[xyz]/.test(ar)) ? ar.split(' ')[0] : false;
153148
};

0 commit comments

Comments
 (0)