Added JSON Customization Support For Angle Fields #2654
Conversation
| */ | ||
| Blockly.FieldAngle.RADIUS = Blockly.FieldAngle.HALF - 1; | ||
|
|
||
| /* See <DOCS LINK HERE> for more information about these properties. */ |
|
Do you want any block factory support for this? Maybe a mutator with the extra options? |
| * or the global setting (null). | ||
| * @return {!Blockly.FieldAngle} Returns itself (for method chaining). | ||
| */ | ||
| Blockly.FieldAngle.prototype.setClockwise = function(clockwise) { |
There was a problem hiding this comment.
@RoboErikG thoughts on returning itself for method chaining? It's a reasonable pattern, but 1) we don't use it anywhere else and 2) we probably want to be consistent.
There was a problem hiding this comment.
It's nice for convenience, but it's pretty non-standard for instances. I think I'd rather start using an opt_config parameter on the constructor with a map of the configuration options. The keys should be the same as the JSON config which will also let you use the same code for both.
There was a problem hiding this comment.
Do you want me to go ahead and update it? or are you waiting for feedback from Rachel?
There was a problem hiding this comment.
+1 to Erik. Go ahead and update it.
There was a problem hiding this comment.
I went to go update this, and then I realized I had more questions and wasn't sure how far reaching the change should be. I created an issue to make discussion easier.
There was a problem hiding this comment.
If you're happy with the clarification on #2722, then just remove the method chaining and this will be good to go.
There was a problem hiding this comment.
I was planning on getting the abstract field changes in first (just to make sure how I'm thinking about it in my head is actually going to work). But don't worry I haven't forgotten about this little guy!
Also atm #2722 has this guy down as no hooks. This is b/c, for example, changing the WRAP value at runtime means the field would have to do some extra work recalculating it's value. I think I could get it to work though.
Did you want hooks for the angle field or no?
BeksOmega
force-pushed
the
feature/AngleJSON
branch
from
fe57b35 to
0ccfc10
Compare
Removed TODOs, devs should handle value changes themselves (matches colour field).
BeksOmega
force-pushed
the
feature/AngleJSON
branch
from
0ccfc10 to
4ef426e
Compare
|
Closing after discussion with Sam. Will revive after configuration is happy. |
3 participants
The basics
The details
Resolves
#2315 plus some
Proposed Changes
Reason for Changes
Why did I add individual properties?
I absolutely love love love angle fields. I think they're probably the coolest field in Blockly and I think they're super under utilized. I would hate to see them hamstrung by the JSON.
For example, if there were only the two modes available you couldn't make super cool blocks like this:
which have built-in affordances for directionally-challenged users.
Providing the modes & the properties (with the properties given override power since they're more specific) is the best of both ease of use, and power.
Test Coverage
Added test blocks for all of the modes and all of the properties.
Tested all of the blocks to make sure they were functioning properly.
Tested on:
Additional Information
Setting round to 0 looks crazy lol:
But devs can fix it by setting round to 1 instead, so it doesn't need a fix.