Fix shapes and images referencing a missing subplot #1481
Merged
Conversation
This file contains hidden or 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
alexcjohnson
commented
Mar 15, 2017
| @@ -107,7 +107,7 @@ describe('Layout images', function() { | |||
| it('should draw images on the right layers', function() { | |||
|
|
|||
| Plotly.plot(gd, data, { images: [{ | |||
| source: 'imageabove', | |||
There was a problem hiding this comment.
These were generating 404 warnings when you run the tests, which seem distracting and unnecessary, so I just put in jsLogo everywhere.
alexcjohnson
commented
Mar 15, 2017
| 'use strict'; | ||
| function countShapesInLowerLayer(gd) { | ||
| return gd._fullLayout.shapes.filter(isShapeInLowerLayer).length; | ||
| } |
There was a problem hiding this comment.
Moved these helpers to the root level so I could use them in a different describe block (which wants to set up a different plot in gd)
etpinard
reviewed
Mar 16, 2017
| // Fall back to _shapeLowerLayer in case the requested subplot doesn't exist. | ||
| // This can happen if you reference the shape to an x / y axis combination | ||
| // that doesn't have any data on it (and layer is below) | ||
| drawShape(gd._fullLayout._shapeLowerLayer); |
There was a problem hiding this comment.
Looks good.
I see that annotations don't need a similar fix. @alexcjohnson Can I ask why?
There was a problem hiding this comment.
Annotations don't have the concept of layer. They're always on top.
There was a problem hiding this comment.
Right. That makes sense 👍
|
Thanks again for making shapes/images 🚅 proof! 💃 my comment is non-blocking. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Soooo many edge cases... This one comes up in the workspace trying to move shapes or images between unlinked subplots, but it can also happen on initial draw - for example you put data on
xyandx2y2but put a shape onxy2- not sure why you'd do this but it should be allowed, and certainly shouldn't error as it does now.Meanwhile on the workspace side I'll just move these components to
layer: 'above'- which may be less confusing there anyway since much of the time the objects would get hidden behind a plot otherwise.@etpinard please review.