Added custom fields demo. #2594
Conversation
There was a problem hiding this comment.
There seems to be a bad interaction between the no-matches validator and the default values. I sometimes end up with a different turtle than the xml. Here's an XML sample that reproduces the issue:
<xml xmlns="http://www.w3.org/1999/xhtml"> <block type="turtle_basic" id="HnH^R=5ve^f#Ad$f?sJM" x="-304" y="80"> <field name="TURTLE" pattern="Dots" hat="Stovepipe">Yertle</field> <comment pinned="false" h="80" w="160">Demonstrates a turtle field with no validator.</comment> </block> <block type="turtle_nullifier" id="Q}[Nkv/CuXg!C!pV{GmB" x="-365" y="84"> <field name="TURTLE" pattern="Dots" hat="Stovepipe">Yertle</field> <comment pinned="false" h="80" w="160">Validates combinations of names and hats to null (invalid) if they could be considered infringe-y. This turns the turtle field red. Infringe-y combinations are: (Leonardo, Mask), (Yertle, Crown), and (Franklin, Propeller).</comment> </block> <block type="turtle_changer" x="10" y="230"> <field name="TURTLE1" pattern="Stripes" hat="Propeller">Yertle</field> <field name="TURTLE2" pattern="Hexagons" hat="Mask">Bowser</field> <comment pinned="false" h="80" w="160">Validates the input so that the two turtles may never have the same hat, pattern, or name.</comment> </block> </xml>
Thank you for noticing this! Sadly this issue is unfixable with the current API, so I'll come up with a different example and add it in. |
| var valid = this.doClassValidation_(value); | ||
| if (valid === null) { | ||
| // See the doClassValidation_ function for information on the | ||
| // cachedValidatedValue_ property. |
There was a problem hiding this comment.
Why return null and create this cachedValidatedValue_? You'd get the same effect by setting the properties on newValue to the default when it's invalid and returning the newValue in doClassValidation_.
There was a problem hiding this comment.
You're correct that in this case it does give you the same effect, but I decided to do it this way because that's what's recommended in the documentation.
This is probably easier to explain with an example use case:
Say you want to create a Vector2 field that takes in text input. You would want to be able to validate the text of each number separately, and turn the text inputs red separately. To do this you would need to use the cachedValidatedValue_ pattern to save the mixed-validated state.
As you noticed the turtle field isn't a great example of this pattern, but I figured it was better to include it anyway.
There was a problem hiding this comment.
Sounds reasonable. As a followup change (separate PR) if you'd like to make it clearer how doValueInvalid_ can make use of this you could color the parts of the turtle that are invalid red (hat, shell, name) in addition to tinting the background. ;)
There was a problem hiding this comment.
Thinking about it more, styling the text of the picker or adding a warning icon next to the text would probably be clearer. That way it's easy to tell what part to change to fix the invalid selection. =)
demos/custom-fields/field_turtle.js
Outdated
|
|
||
| // The field group is created by the abstract field's init_ function. *All | ||
| // elements* should be children of the fieldGroup_. | ||
| this.movableGroup_ = Blockly.utils.dom.createSvgElement('g', |
There was a problem hiding this comment.
You might want to pull out the svg code into a separate function and put it at the bottom of the file. It's not really important to the concepts so moving it out of the way will make it easier for folks to follow the rest of the code.
|
Oh, looks like Neil also submitted a change requiring a rebuild. You'll need to rebase and build again, sorry about that. |
BeksOmega
force-pushed
the
demos/CustomFields
branch
from
a6d82b5
to
f837b7c
Compare
Done! :D |
The basics
I added some comments in places where comments are generally not supposed to go.
The details
Resolves
Work on #1623
Proposed Changes
Adds a custom fields demo with turtle fields.
Reason for Changes
Demos are fun.
Test Coverage
Tested using all of the buttons and all of the blocks, everything looks good.
Tested on:
Additional Information
To test out the demo I think you need to rebuild the core because of the utils name change.
I added inline comments as was suggested, but I may have gone a bit overboard.