-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Angle Field Fixes #2501
Angle Field Fixes #2501
Conversation
core/field_angle.js
Outdated
@@ -295,6 +307,7 @@ Blockly.FieldAngle.prototype.updateGraph_ = function() { | |||
} | |||
// Always display the input (i.e. getText) even if it is invalid. | |||
var angleDegrees = Number(this.getText()) + Blockly.FieldAngle.OFFSET; | |||
angleDegrees = angleDegrees % 360; |
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 is a rare opportunity to use the %=
operator.
angleDegrees %= 360;
While you're at it, there's another opportunity for a %=
in Blockly.FieldAngle.prototype.doClassValidation_
.
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.
Oooo fun! :D
Fixed
@NeilFraser does this look good to you now? |
The basics
The details
Resolves
N/A
Proposed Changes
Reason for Changes
It was broken. (The below is a clockwise angle field)
It was always displaying between 0 and 360. It's pretty confusing if it changes when you close th editor.
This just bugged me.
Angle fields should always display a number value (this is different from pure text inputs, which can be empty).
Test Coverage
I tested lots of combinations of CLOCKWISE, OFFSET, and WRAP. Here are some highlights:
Clockwise, Offset 90
Widdershins, Offset 90
Widdershins, Wrap 180
Widdershins, Wrap 180, Offset 90
And here's it now working with the empty string:
Tested on:
Additional Information
N/A