do value update if the value is valid again #4351
Conversation
|
Hello @halitanildonmez Thanks for submitting a fix for this :D I think that you're definitely looking in the right spot wrt the bug you mentioned! But I think it might be better to modify the logic of the setValue function, rather than modify the processValidation_ function. With the current fix it might run into problems with doValueUpdate_ and forceRerender getting called on values that pass the class validator, but not the local validator. I don't think this would cause bad behavior or anything, but rerenders can be pretty expensive, so it's best to minimize them when possible. Do you feel like that's workable? And it seems like the travis build failed because some mocha tests need to be updated for the new behavior. These tests are located in your my-blockly-clone/tests/mocha directory. There is also an index.html file in that directory, and if you open that in your browser it will give you more details about what tests are passing/failing. Specifically it looks like some sinon assertions need updates, so the sinon assertions docs might be helpful. Thank you again for the submission! I know the fields stuff can be kind of a mess hehe so thanks for digging through it :D If you have any questions/comments obviously please feel free to reply! |
|
Hello @BeksOmega ! Thank you for the feedback and the help! 😄 I have fixed it with the last two commits (i hope) and hope it looks better now. Let me know if you want me to change something else 😄 |
|
one note, I changed the tests to calledOnce for value update, hope that is ok |
|
Looks perfect! Thank you for the updates @halitanildonmez :D |
|
Awesome! So glad to hear 😄 |
core/field.js
Outdated
| @@ -899,6 +899,7 @@ Blockly.Field.prototype.setValue = function(newValue) { | |||
| var oldValue = this.getValue(); | |||
| if (oldValue === newValue) { | |||
| doLogging && console.log('same, return'); | |||
There was a problem hiding this comment.
I think this needs to be updated to reflect the change. Something like 'same, doValueUpdate_ then return'.
There was a problem hiding this comment.
Fixed it! 😄 thank you for the input!
The basics
The details
Resolves
#4345
Proposed Changes
The issue was, in the field, if the user has entered invalid once the flag
isTextValid_;was being set to false atdoValueInvalid_which is fine. If however the user enters correct value, then the flag is never se to true.I made it so that at field.js
processValidation_we dodoValueUpdate_so that we update the flag.Reason for Changes
To solve the bug that was opened.
Test Coverage
Tested on:
Desktop Chrome
Desktop Firefox
Documentation
Additional Information