-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Relayout edge case fixes #1494
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.
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
Relayout edge case fixes #1494
Conversation
- this is also done in the default step - but we need to reset it here to get margin relayout calls to not throw excpetions.
- so that they don't throw exceptions with the excepted axis container is undefined.
@@ -26,6 +26,8 @@ var toLogRange = require('../../lib/to_log_range'); | |||
* same relayout call should override this conversion. | |||
*/ | |||
module.exports = function convertCoords(gd, ax, newType, doExtra) { | |||
ax = ax || {}; |
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.
.. so that e.g. in relayout
blank axis objects that pass the forgiving AX_NAME_PATTERN
(which also matches zaxis?
by the way) don't leave to exceptions.
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.
cc @alexcjohnson
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.
Man, if only I could turn back time and not use linearized coordinates on log axes...
@@ -714,6 +714,7 @@ proto.setConvert = function() { | |||
for(var i = 0; i < 3; ++i) { | |||
var ax = this.fullSceneLayout[axisProperties[i]]; | |||
Axes.setConvert(ax, this.fullLayout); | |||
ax.setScale = Lib.noop; |
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.
This statement also appears in the 3d axis_defaults
.
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.
Looks good. I can't really tell if these problems generalize beyond these fixes or not, so I guess if anything else comes up we'll just have to add more test cases... 💃 |
The two commits here each fix one relayout edge case broken since #1431 and #1403 respectively.