Skip to content

Commit 388287b

Browse files
alexcjohnsonalexcjohnson
authored
Merge pull request #1481 from plotly/below-no-plot
Fix shapes and images referencing a missing subplot
2 parents abd0c45 + 9369bc3 commit 388287b

File tree

4 files changed

+150
-67
lines changed

4 files changed

+150
-67
lines changed

‎src/components/images/draw.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,15 @@ module.exports = function draw(gd) {
3030
subplot = img.xref + img.yref;
3131

3232
var plotinfo = fullLayout._plots[subplot];
33+
34+
if(!plotinfo) {
35+
// Fall back to _imageLowerLayer in case the requested subplot doesn't exist.
36+
// This can happen if you reference the image to an x / y axis combination
37+
// that doesn't have any data on it (and layer is below)
38+
imageDataBelow.push(img);
39+
continue;
40+
}
41+
3342
if(plotinfo.mainplot) {
3443
subplot = plotinfo.mainplot.id;
3544
}

‎src/components/shapes/draw.js‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,17 @@ function drawOne(gd, index) {
7575
drawShape(gd._fullLayout._shapeLowerLayer);
7676
}
7777
else {
78-
var plotinfo = gd._fullLayout._plots[options.xref + options.yref],
79-
mainPlot = plotinfo.mainplot || plotinfo;
80-
81-
drawShape(mainPlot.shapelayer);
78+
var plotinfo = gd._fullLayout._plots[options.xref + options.yref];
79+
if(plotinfo) {
80+
var mainPlot = plotinfo.mainplot || plotinfo;
81+
drawShape(mainPlot.shapelayer);
82+
}
83+
else {
84+
// Fall back to _shapeLowerLayer in case the requested subplot doesn't exist.
85+
// This can happen if you reference the shape to an x / y axis combination
86+
// that doesn't have any data on it (and layer is below)
87+
drawShape(gd._fullLayout._shapeLowerLayer);
88+
}
8289
}
8390

8491
function drawShape(shapeLayer) {

‎test/jasmine/tests/layout_images_test.js‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ describe('Layout images', function() {
107107
it('should draw images on the right layers', function() {
108108

109109
Plotly.plot(gd, data, { images: [{
110-
source: 'imageabove',
110+
source: jsLogo,
111111
layer: 'above'
112112
}]});
113113

@@ -116,7 +116,7 @@ describe('Layout images', function() {
116116
destroyGraphDiv();
117117
gd = createGraphDiv();
118118
Plotly.plot(gd, data, { images: [{
119-
source: 'imagebelow',
119+
source: jsLogo,
120120
layer: 'below'
121121
}]});
122122

@@ -125,7 +125,7 @@ describe('Layout images', function() {
125125
destroyGraphDiv();
126126
gd = createGraphDiv();
127127
Plotly.plot(gd, data, { images: [{
128-
source: 'imagesubplot',
128+
source: jsLogo,
129129
layer: 'below',
130130
xref: 'x',
131131
yref: 'y'
@@ -134,12 +134,36 @@ describe('Layout images', function() {
134134
checkLayers(0, 0, 1);
135135
});
136136

137+
it('should fall back on imageLowerLayer for below missing subplots', function() {
138+
Plotly.newPlot(gd, [
139+
{x: [1, 3], y: [1, 3]},
140+
{x: [1, 3], y: [1, 3], xaxis: 'x2', yaxis: 'y2'}
141+
], {
142+
xaxis: {domain: [0, 0.5]},
143+
yaxis: {domain: [0, 0.5]},
144+
xaxis2: {domain: [0.5, 1], anchor: 'y2'},
145+
yaxis2: {domain: [0.5, 1], anchor: 'x2'},
146+
images: [{
147+
source: jsLogo,
148+
layer: 'below',
149+
xref: 'x',
150+
yref: 'y2'
151+
}, {
152+
source: jsLogo,
153+
layer: 'below',
154+
xref: 'x2',
155+
yref: 'y'
156+
}]
157+
});
158+
159+
checkLayers(0, 2, 0);
160+
});
161+
137162
describe('with anchors and sizing', function() {
138163

139164
function testAspectRatio(xAnchor, yAnchor, sizing, expected) {
140-
var anchorName = xAnchor + yAnchor;
141165
Plotly.plot(gd, data, { images: [{
142-
source: anchorName,
166+
source: jsLogo,
143167
xanchor: xAnchor,
144168
yanchor: yAnchor,
145169
sizing: sizing
@@ -405,7 +429,7 @@ describe('images log/linear axis changes', function() {
405429
],
406430
layout: {
407431
images: [{
408-
source: 'https://images.plot.ly/language-icons/api-home/python-logo.png',
432+
source: pythonLogo,
409433
x: 1,
410434
y: 1,
411435
xref: 'x',

‎test/jasmine/tests/shapes_test.js‎

Lines changed: 100 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -103,75 +103,75 @@ describe('Test shapes defaults:', function() {
103103
});
104104
});
105105

106-
describe('Test shapes:', function() {
107-
'use strict';
106+
function countShapesInLowerLayer(gd) {
107+
return gd._fullLayout.shapes.filter(isShapeInLowerLayer).length;
108+
}
108109

109-
var mock = require('@mocks/shapes.json');
110-
var gd;
110+
function countShapesInUpperLayer(gd) {
111+
return gd._fullLayout.shapes.filter(isShapeInUpperLayer).length;
112+
}
111113

112-
beforeEach(function(done) {
113-
gd = createGraphDiv();
114+
function countShapesInSubplots(gd) {
115+
return gd._fullLayout.shapes.filter(isShapeInSubplot).length;
116+
}
114117

115-
var mockData = Lib.extendDeep([], mock.data),
116-
mockLayout = Lib.extendDeep({}, mock.layout);
118+
function isShapeInUpperLayer(shape) {
119+
return shape.layer !== 'below';
120+
}
117121

118-
Plotly.plot(gd, mockData, mockLayout).then(done);
119-
});
122+
function isShapeInLowerLayer(shape) {
123+
return (shape.xref === 'paper' && shape.yref === 'paper') &&
124+
!isShapeInUpperLayer(shape);
125+
}
120126

121-
afterEach(destroyGraphDiv);
127+
function isShapeInSubplot(shape) {
128+
return !isShapeInUpperLayer(shape) && !isShapeInLowerLayer(shape);
129+
}
122130

123-
function countShapesInLowerLayer() {
124-
return gd._fullLayout.shapes.filter(isShapeInLowerLayer).length;
125-
}
131+
function countShapeLowerLayerNodes() {
132+
return d3.selectAll('.layer-below > .shapelayer').size();
133+
}
126134

127-
function countShapesInUpperLayer() {
128-
return gd._fullLayout.shapes.filter(isShapeInUpperLayer).length;
129-
}
135+
function countShapeUpperLayerNodes() {
136+
return d3.selectAll('.layer-above > .shapelayer').size();
137+
}
130138

131-
function countShapesInSubplots() {
132-
return gd._fullLayout.shapes.filter(isShapeInSubplot).length;
133-
}
139+
function countShapeLayerNodesInSubplots() {
140+
return d3.selectAll('.layer-subplot').size();
141+
}
134142

135-
function isShapeInUpperLayer(shape) {
136-
return shape.layer !== 'below';
137-
}
143+
function countSubplots(gd) {
144+
return Object.keys(gd._fullLayout._plots || {}).length;
145+
}
138146

139-
function isShapeInLowerLayer(shape) {
140-
return (shape.xref === 'paper' && shape.yref === 'paper') &&
141-
!isShapeInUpperLayer(shape);
142-
}
147+
function countShapePathsInLowerLayer() {
148+
return d3.selectAll('.layer-below > .shapelayer > path').size();
149+
}
143150

144-
function isShapeInSubplot(shape) {
145-
return !isShapeInUpperLayer(shape) && !isShapeInLowerLayer(shape);
146-
}
151+
function countShapePathsInUpperLayer() {
152+
return d3.selectAll('.layer-above > .shapelayer > path').size();
153+
}
147154

148-
function countShapeLowerLayerNodes() {
149-
return d3.selectAll('.layer-below > .shapelayer').size();
150-
}
155+
function countShapePathsInSubplots() {
156+
return d3.selectAll('.layer-subplot > .shapelayer > path').size();
157+
}
151158

152-
function countShapeUpperLayerNodes() {
153-
return d3.selectAll('.layer-above > .shapelayer').size();
154-
}
159+
describe('Test shapes:', function() {
160+
'use strict';
155161

156-
function countShapeLayerNodesInSubplots() {
157-
return d3.selectAll('.layer-subplot').size();
158-
}
162+
var mock = require('@mocks/shapes.json');
163+
var gd;
159164

160-
function countSubplots(gd) {
161-
return Object.keys(gd._fullLayout._plots || {}).length;
162-
}
165+
beforeEach(function(done) {
166+
gd = createGraphDiv();
163167

164-
function countShapePathsInLowerLayer() {
165-
return d3.selectAll('.layer-below > .shapelayer > path').size();
166-
}
168+
var mockData = Lib.extendDeep([], mock.data),
169+
mockLayout = Lib.extendDeep({}, mock.layout);
167170

168-
function countShapePathsInUpperLayer() {
169-
return d3.selectAll('.layer-above > .shapelayer > path').size();
170-
}
171+
Plotly.plot(gd, mockData, mockLayout).then(done);
172+
});
171173

172-
function countShapePathsInSubplots() {
173-
return d3.selectAll('.layer-subplot > .shapelayer > path').size();
174-
}
174+
afterEach(destroyGraphDiv);
175175

176176
describe('*shapeLowerLayer*', function() {
177177
it('has one node', function() {
@@ -180,14 +180,14 @@ describe('Test shapes:', function() {
180180

181181
it('has as many *path* nodes as shapes in the lower layer', function() {
182182
expect(countShapePathsInLowerLayer())
183-
.toEqual(countShapesInLowerLayer());
183+
.toEqual(countShapesInLowerLayer(gd));
184184
});
185185

186186
it('should be able to get relayout', function(done) {
187187
Plotly.relayout(gd, {height: 200, width: 400}).then(function() {
188188
expect(countShapeLowerLayerNodes()).toEqual(1);
189189
expect(countShapePathsInLowerLayer())
190-
.toEqual(countShapesInLowerLayer());
190+
.toEqual(countShapesInLowerLayer(gd));
191191
})
192192
.catch(failTest)
193193
.then(done);
@@ -201,14 +201,14 @@ describe('Test shapes:', function() {
201201

202202
it('has as many *path* nodes as shapes in the upper layer', function() {
203203
expect(countShapePathsInUpperLayer())
204-
.toEqual(countShapesInUpperLayer());
204+
.toEqual(countShapesInUpperLayer(gd));
205205
});
206206

207207
it('should be able to get relayout', function(done) {
208208
Plotly.relayout(gd, {height: 200, width: 400}).then(function() {
209209
expect(countShapeUpperLayerNodes()).toEqual(1);
210210
expect(countShapePathsInUpperLayer())
211-
.toEqual(countShapesInUpperLayer());
211+
.toEqual(countShapesInUpperLayer(gd));
212212
})
213213
.catch(failTest)
214214
.then(done);
@@ -223,15 +223,15 @@ describe('Test shapes:', function() {
223223

224224
it('has as many *path* nodes as shapes in the subplot', function() {
225225
expect(countShapePathsInSubplots())
226-
.toEqual(countShapesInSubplots());
226+
.toEqual(countShapesInSubplots(gd));
227227
});
228228

229229
it('should be able to get relayout', function(done) {
230230
Plotly.relayout(gd, {height: 200, width: 400}).then(function() {
231231
expect(countShapeLayerNodesInSubplots())
232232
.toEqual(countSubplots(gd));
233233
expect(countShapePathsInSubplots())
234-
.toEqual(countShapesInSubplots());
234+
.toEqual(countShapesInSubplots(gd));
235235
})
236236
.catch(failTest)
237237
.then(done);
@@ -464,6 +464,49 @@ describe('shapes axis reference changes', function() {
464464
});
465465
});
466466

467+
describe('shapes edge cases', function() {
468+
'use strict';
469+
470+
var gd;
471+
472+
beforeAll(function() {
473+
jasmine.addMatchers(customMatchers);
474+
});
475+
476+
beforeEach(function() { gd = createGraphDiv(); });
477+
478+
afterEach(destroyGraphDiv);
479+
480+
it('falls back on shapeLowerLayer for below missing subplots', function(done) {
481+
Plotly.newPlot(gd, [
482+
{x: [1, 3], y: [1, 3]},
483+
{x: [1, 3], y: [1, 3], xaxis: 'x2', yaxis: 'y2'}
484+
], {
485+
xaxis: {domain: [0, 0.5]},
486+
yaxis: {domain: [0, 0.5]},
487+
xaxis2: {domain: [0.5, 1], anchor: 'y2'},
488+
yaxis2: {domain: [0.5, 1], anchor: 'x2'},
489+
shapes: [{
490+
x0: 1, x1: 2, y0: 1, y1: 2, type: 'circle',
491+
layer: 'below',
492+
xref: 'x',
493+
yref: 'y2'
494+
}, {
495+
x0: 1, x1: 2, y0: 1, y1: 2, type: 'circle',
496+
layer: 'below',
497+
xref: 'x2',
498+
yref: 'y'
499+
}]
500+
}).then(function() {
501+
expect(countShapePathsInLowerLayer()).toBe(2);
502+
expect(countShapePathsInUpperLayer()).toBe(0);
503+
expect(countShapePathsInSubplots()).toBe(0);
504+
})
505+
.catch(failTest)
506+
.then(done);
507+
});
508+
});
509+
467510
describe('shapes autosize', function() {
468511
'use strict';
469512

0 commit comments

Comments
 (0)