Add layout.shapes.layer
#439
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ function handleShapeDefaults(shapeIn, fullLayout) { | |
| return Lib.coerce(shapeIn, shapeOut, shapes.layoutAttributes, attr, dflt); | ||
| } | ||
|
|
||
| coerce('layer'); | ||
| coerce('opacity'); | ||
| coerce('fillcolor'); | ||
| coerce('line.color'); | ||
|
|
@@ -171,7 +172,8 @@ function updateAllShapes(gd, opt, value) { | |
| } | ||
|
|
||
| function deleteShape(gd, index) { | ||
| getShapeLayer(gd, index) | ||
| .selectAll('[data-index="' + index + '"]') | ||
| .remove(); | ||
|
|
||
| gd._fullLayout.shapes.splice(index, 1); | ||
|
|
@@ -181,9 +183,9 @@ function deleteShape(gd, index) { | |
| for(var i = index; i < gd._fullLayout.shapes.length; i++) { | ||
| // redraw all shapes past the removed one, | ||
| // so they bind to the right events | ||
| getShapeLayer(gd, i) | ||
| .selectAll('[data-index="' + (i + 1) + '"]') | ||
| .attr('data-index', i); | ||
| shapes.draw(gd, i); | ||
| } | ||
| } | ||
|
|
@@ -201,10 +203,13 @@ function insertShape(gd, index, newShape) { | |
| gd.layout.shapes = [rule]; | ||
| } | ||
|
|
||
| // there is no need to call shapes.draw(gd, index), | ||
| // because updateShape() is called from within shapes.draw() | ||
|
|
||
| for(var i = gd._fullLayout.shapes.length - 1; i > index; i--) { | ||
| getShapeLayer(gd, i) | ||
| .selectAll('[data-index="' + (i - 1) + '"]') | ||
| .attr('data-index', i); | ||
| shapes.draw(gd, i); | ||
| } | ||
| } | ||
|
|
@@ -213,7 +218,8 @@ function updateShape(gd, index, opt, value) { | |
| var i; | ||
|
|
||
| // remove the existing shape if there is one | ||
| getShapeLayer(gd, index) | ||
| .selectAll('[data-index="' + index + '"]') | ||
| .remove(); | ||
|
|
||
| // remember a few things about what was already there, | ||
|
|
@@ -288,23 +294,72 @@ function updateShape(gd, index, opt, value) { | |
| gd._fullLayout.shapes[index] = options; | ||
|
|
||
| var attrs = { | ||
| 'data-index': index, | ||
| 'fill-rule': 'evenodd', | ||
| d: shapePath(gd, options) | ||
| }, | ||
| clipAxes = (options.xref + options.yref).replace(/paper/g, ''); | ||
|
|
||
| var lineColor = options.line.width ? options.line.color : 'rgba(0,0,0,0)'; | ||
|
|
||
| if(options.layer !== 'below') { | ||
| drawShape(gd._fullLayout._shapeUpperLayer); | ||
| } | ||
| else if(options.xref === 'paper' && options.yref === 'paper') { | ||
| drawShape(gd._fullLayout._shapeLowerLayer); | ||
| } else { | ||
| forEachSubplot(gd, function(plotinfo) { | ||
| if(isShapeInSubplot(gd, options, plotinfo.id)) { | ||
| drawShape(plotinfo.shapelayer); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return; | ||
|
|
||
| function drawShape(shapeLayer) { | ||
| var path = shapeLayer.append('path') | ||
| .attr(attrs) | ||
| .style('opacity', options.opacity) | ||
| .call(Color.stroke, lineColor) | ||
| .call(Color.fill, options.fillcolor) | ||
| .call(Drawing.dashLine, options.line.dash, options.line.width); | ||
|
|
||
| if(clipAxes) { | ||
| path.call(Drawing.setClipUrl, | ||
| 'clip' + gd._fullLayout._uid + clipAxes); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function getShapeLayer(gd, index) { | ||
|
There was a problem hiding this comment. Is this thing possible in 3d ??? |
||
| var shape = gd._fullLayout.shapes[index], | ||
| shapeLayer = gd._fullLayout._shapeUpperLayer; | ||
|
|
||
| if(!shape) { | ||
| console.log('getShapeLayer: undefined shape: index', index); | ||
| } | ||
| else if(shape.layer === 'below') { | ||
| shapeLayer = (shape.xref === 'paper' && shape.yref === 'paper') ? | ||
| gd._fullLayout._shapeLowerLayer : | ||
| gd._fullLayout._subplotShapeLayer; | ||
| } | ||
|
|
||
| return shapeLayer; | ||
| } | ||
|
|
||
| function isShapeInSubplot(gd, shape, subplot) { | ||
| var xa = Plotly.Axes.getFromId(gd, subplot, 'x')._id, | ||
| ya = Plotly.Axes.getFromId(gd, subplot, 'y')._id; | ||
| return shape.layer === 'below' && (xa === shape.xref || ya === shape.yref); | ||
| } | ||
|
|
||
| function forEachSubplot(gd, fn) { | ||
|
There was a problem hiding this comment. 🐄 You may argue that adding Please move the |
||
| var plots = gd._fullLayout._plots || {}, | ||
| subplots = Object.getOwnPropertyNames(plots); | ||
|
There was a problem hiding this comment. 🐄 Agreed. So, please swap |
||
|
|
||
| for(var i = 0, n = subplots.length; i < n; i++) { | ||
| fn(plots[subplots[i]]); | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,30 +24,114 @@ describe('Test shapes:', function() { | |
|
|
||
| afterEach(destroyGraphDiv); | ||
|
|
||
| function countShapesInLowerLayer() { | ||
| return gd._fullLayout.shapes.filter(isShapeInLowerLayer).length; | ||
| } | ||
|
|
||
| function countShapesInUpperLayer() { | ||
| return gd._fullLayout.shapes.filter(isShapeInUpperLayer).length; | ||
| } | ||
|
|
||
| function countShapesInSubplots() { | ||
| return gd._fullLayout.shapes.filter(isShapeInSubplot).length; | ||
| } | ||
|
|
||
| function isShapeInUpperLayer(shape) { | ||
| return shape.layer !== 'below'; | ||
| } | ||
|
|
||
| function isShapeInLowerLayer(shape) { | ||
| return (shape.xref === 'paper' && shape.yref === 'paper') && | ||
| !isShapeInUpperLayer(shape); | ||
| } | ||
|
|
||
| function isShapeInSubplot(shape) { | ||
| return !isShapeInUpperLayer(shape) && !isShapeInLowerLayer(shape); | ||
| } | ||
|
|
||
| function countShapeLowerLayerNodes() { | ||
| return d3.selectAll('.shapelayer-below').size(); | ||
| } | ||
|
|
||
| function countShapeUpperLayerNodes() { | ||
| return d3.selectAll('.shapelayer-above').size(); | ||
| } | ||
|
|
||
| function countShapeLayerNodesInSubplots() { | ||
| return d3.selectAll('.shapelayer-subplot').size(); | ||
| } | ||
|
|
||
| function countSubplots(gd) { | ||
| return Object.getOwnPropertyNames(gd._fullLayout._plots || {}).length; | ||
|
|
||
| } | ||
|
|
||
| function countShapePathsInLowerLayer() { | ||
| return d3.selectAll('.shapelayer-below > path').size(); | ||
| } | ||
|
|
||
| function countShapePathsInUpperLayer() { | ||
| return d3.selectAll('.shapelayer-above > path').size(); | ||
| } | ||
|
|
||
| function countShapePathsInSubplots() { | ||
| return d3.selectAll('.shapelayer-subplot > path').size(); | ||
| } | ||
|
|
||
| describe('*shapeLowerLayer*', function() { | ||
| it('has one node', function() { | ||
| expect(countShapeLowerLayerNodes()).toEqual(1); | ||
| }); | ||
|
|
||
| it('has as many *path* nodes as shapes in the lower layer', function() { | ||
| expect(countShapePathsInLowerLayer()) | ||
| .toEqual(countShapesInLowerLayer()); | ||
| }); | ||
|
|
||
| it('should be able to get relayout', function(done) { | ||
| Plotly.relayout(gd, {height: 200, width: 400}).then(function() { | ||
| expect(countShapeLowerLayerNodes()).toEqual(1); | ||
| expect(countShapePathsInLowerLayer()) | ||
| .toEqual(countShapesInLowerLayer()); | ||
| }).then(done); | ||
| }); | ||
| }); | ||
|
|
||
| describe('*shapeUpperLayer*', function() { | ||
| it('has one node', function() { | ||
| expect(countShapeUpperLayerNodes()).toEqual(1); | ||
| }); | ||
|
|
||
| it('has as many *path* nodes as shapes in the upper layer', function() { | ||
| expect(countShapePathsInUpperLayer()) | ||
| .toEqual(countShapesInUpperLayer()); | ||
| }); | ||
|
|
||
| it('should be able to get relayout', function(done) { | ||
| Plotly.relayout(gd, {height: 200, width: 400}).then(function() { | ||
| expect(countShapeUpperLayerNodes()).toEqual(1); | ||
| expect(countShapePathsInUpperLayer()) | ||
| .toEqual(countShapesInUpperLayer()); | ||
| }).then(done); | ||
| }); | ||
| }); | ||
|
|
||
| describe('each *subplot*', function() { | ||
| it('has one *shapelayer*', function() { | ||
| expect(countShapeLayerNodesInSubplots()) | ||
| .toEqual(countSubplots(gd)); | ||
| }); | ||
|
|
||
| it('has as many *path* nodes as shapes in the subplot', function() { | ||
|
|
||
| expect(countShapePathsInSubplots()) | ||
| .toEqual(countShapesInSubplots()); | ||
| }); | ||
|
|
||
| it('should be able to get relayout', function(done) { | ||
| Plotly.relayout(gd, {height: 200, width: 400}).then(function() { | ||
| expect(countShapeLayerNodesInSubplots()) | ||
| .toEqual(countSubplots(gd)); | ||
| expect(countShapePathsInSubplots()) | ||
| .toEqual(countShapesInSubplots()); | ||
| }).then(done); | ||
| }); | ||
| }); | ||
|
|
@@ -75,33 +159,32 @@ describe('Test shapes:', function() { | |
|
|
||
| describe('Plotly.relayout', function() { | ||
| it('should be able to add a shape', function(done) { | ||
| var pathCount = countShapePathsInUpperLayer(); | ||
| var index = countShapes(gd); | ||
| var shape = getRandomShape(); | ||
|
|
||
| Plotly.relayout(gd, 'shapes[' + index + ']', shape).then(function() | ||
| { | ||
|
There was a problem hiding this comment. we should really pick an I'd vote for 🐄 So please, bring up that There was a problem hiding this comment. I'm trying to keep lines 80 chars or less. Shall I break the line before the function, like this: There was a problem hiding this comment. Ah good point. Don't worry about going over 80 characters. I don't think we'll ever enforce that rule in our jasmine test suites. |
||
| expect(countShapePathsInUpperLayer()).toEqual(pathCount + 1); | ||
| expect(getLastShape(gd)).toEqual(shape); | ||
| expect(countShapes(gd)).toEqual(index + 1); | ||
| }).then(done); | ||
| }); | ||
|
|
||
| it('should be able to remove a shape', function(done) { | ||
| var pathCount = countShapePathsInUpperLayer(); | ||
| var index = countShapes(gd); | ||
| var shape = getRandomShape(); | ||
|
|
||
| Plotly.relayout(gd, 'shapes[' + index + ']', shape).then(function() | ||
| { | ||
|
|
||
| expect(countShapePathsInUpperLayer()).toEqual(pathCount + 1); | ||
| expect(getLastShape(gd)).toEqual(shape); | ||
| expect(countShapes(gd)).toEqual(index + 1); | ||
| }).then(function() { | ||
| Plotly.relayout(gd, 'shapes[' + index + ']', 'remove'); | ||
| }).then(function() { | ||
| expect(countShapePathsInUpperLayer()).toEqual(pathCount); | ||
| expect(countShapes(gd)).toEqual(index); | ||
| }).then(done); | ||
| }); | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐄 no need for this. Save the bytes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove it.
I usually write closures at the end of a function and use the
returnto indicate that there are no more statements hidden after the closures.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand. I'm not saying that it's a bad habit. It's really a matter of consistency with the rest of the source code.