Skip to content
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

Number block accepts non-number value #1238

Closed
ewpatton opened this issue May 20, 2018 · 5 comments · Fixed by #2073
Closed

Number block accepts non-number value #1238

ewpatton opened this issue May 20, 2018 · 5 comments · Fixed by #2073

Comments

@ewpatton
Copy link
Member

Typically, the number block rejects values that do not parse as numbers. For example, typing cat and pressing enter will reject the string and revert the block to its original value. However, if one clicks on the workspace background the block will accept the change.

@ColinTree
Copy link
Contributor

Refer to this line

https://github.com/mit-cml/appinventor-sources/blob/master/appinventor/blocklyeditor/src/blocks/math.js#L24

I wondered why we use
new Blockly.FieldTextInput('0', Blockly.Blocks.math_number.validator) (Text field with a custom validator)
instead of
new Blockly.FieldNumber(0) (blockly built-in number field)
directly?

@ewpatton
Copy link
Member Author

ewpatton commented Jun 6, 2018

@ColinTree The implementation of the number block in App Inventor predates the implementation of Blockly.FieldNumber. I'm not opposed to switching from our existing implementation to FieldNumber, but it will need significant testing to ensure we don't break existing apps when we switch.

@SusanRatiLane
Copy link
Contributor

Additional testing indicates that this is a display problem. The change event for the number block is never called, and the stored value is not saved. The block displays as if it has changed until the project is reloaded.

@BeksOmega
Copy link
Collaborator

Dug into this a bit b/c fields are my jam. Turns out this is a bug inside Blockly, but it's fixed in the latest version.

Explanation of why this occurs:

  1. When you enter "cat" the validator returns null here.
  2. This causes the field to reset the text to the previous value here.
  3. Then when the base setValue gets called here it sees that the old value and new value match here.
  4. So setText never gets called, so it still displays "cat".

It could be fixed by monkey-patching the individual field's setValue function. Or by monkey-patching Blockly.Field's setValue function. But those aren't great options :/

@ewpatton
Copy link
Member Author

@BeksOmega I would just go ahead and monkey patch it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants