AMD conversion for axial (z-axis) vectors, fix incorrect call angle function call #275
Conversation
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
incorrect call to angle function introduced in commit dcb1f0c This fixes bug https://bugs.dojotoolkit.org/ticket/17652
|
|
dylans
reviewed
Jul 14, 2017
| var angle = positioning.angle(obj.start, obj); | ||
| angle<0 ? angle = 360 + angle : angle; | ||
| var angle = this.util.angle(obj); | ||
| angle = angle<0 ? 360 + angle : angle; |
There was a problem hiding this comment.
Perhaps to avoid this type of mistake again, we should make it angle = (angle<0) ?
There was a problem hiding this comment.
I believe that operator precedence makes the parentheses superfluous, but you can certainly add it for clarity.
| @@ -352,7 +351,11 @@ if(0 && dojox.drawing.defaults.zAxisEnabled){ | |||
| } | |||
| },this); | |||
| }; | |||
| dojo.connect(this, "onRenderStencil", this, function(){ if(this.zSelected){ this.zDeselect(this.zSelected)}}); | |||
| dojo.connect(this, "onRenderStencil", this, function(){ | |||
There was a problem hiding this comment.
Given the other changes, it would be nice to replace connect statements with dojo/on and/or aspect.after, but I realize that's non-trivial.
There was a problem hiding this comment.
I'd like to get this fix in now, since the fix has been sitting there for four years.
|
I am not sure what is going wrong. I did sign the CLA. In fact, it won't let me sign it again. |
|
OK, now the CLA is showing up ... |
dylans
approved these changes
Jul 14, 2017
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.
Add AMD conversion for axial (z-axis) vectors.
Fix incorrect call to angle function introduced in commit
dcb1f0c
This fixes bug https://bugs.dojotoolkit.org/ticket/17652