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

Validations not working in 0.5.2 #152

Closed
alexhawkins opened this issue Oct 20, 2016 · 31 comments
Closed

Validations not working in 0.5.2 #152

alexhawkins opened this issue Oct 20, 2016 · 31 comments

Comments

@alexhawkins
Copy link

Validations stopped working when I upgraded from 0.5.1 to 0.5.2. My form looks like this, when I comment out validations, everything works. Without validations, no cigar. Any ideas?

<Formsy.Form
              ref="form"
              className="form"
              onValid={() => this.enableLoginButton()}
              onInvalid={() => this.disableLoginButton()}
              onValidSubmit={(model) => this.login(user, model)}>
              <FormsyText
                name="email"
                floatingLabelText="Email Address"
                fullWidth
                // validations="isEmail"
                // validationError="Please enter a valid email address"
                {...textFieldStyles}
                required
              />

              <FormsyText
                name="password"
                type="password"
                floatingLabelText="Password"
                // validations={{ minLength: 6 }}
                // validationError="Password must contain at least 6 characters"
                fullWidth
                {...textFieldStyles}
                required
              />```
@ryanblakeley
Copy link
Collaborator

ryanblakeley commented Oct 22, 2016

Do you have a console message? When you say it "stopped working", do you mean the form broke or the validation status is not correct?

@bteepell
Copy link

Im seeing this too. Seems with validation in the email field it does not allow edit. There are no console errors. Just will not let edit happen. was working for me with 0.5.0 and formsy-react 0.18.1

@joelkoz
Copy link
Collaborator

joelkoz commented Oct 26, 2016

I am seeing this also. It has to do with any validations, including "required." The culprit seems to be line 81 of FormsyText.js, where a "this.resetValue() is called if the field is not valid. This results in weird behaviors such as:

  1. If you have a validation like "isEmail", and the initial value of the field is blank, typing ANYTHING will be ignored (as the first character is not a valid email address, so the field is reset).
  2. If you have a field marked as "required" and the initial value HAS data and you start backspacing over it, you can remove all but the first character. Once you remove the first character, it becomes "invalid", it resets, and the old value is put back in place.

I commented out that line in the lib/FormsyText.js of my node_modules dir, and it now seems to be behaving. I don't know enough about the code to determine if that is a valid fix or just a band-aid.

@joelkoz
Copy link
Collaborator

joelkoz commented Oct 26, 2016

UPDATE: I forked the project today to make a change on another issue, and while the "resetValue()" is still there in the code, it now appears to work for me. For those who are experiencing this problem, you may want to fork the project and build from source and see if you still have the issue.

@rmoskal
Copy link

rmoskal commented Oct 28, 2016

I'm a new user of the package and it also seems that many validations are broken. isLength and minLength don't let me type anything in input fields. I can reproduce all the same errors with the sample webpack app you ship so I'm pretty sure it's nothing I'm doing wrong.

Problem is definitely here::

https://github.com/mbrookes/formsy-material-ui/blob/master/src/FormsyText.jsx#L81

Commenting out seems to fix things right up.

@ryanblakeley
Copy link
Collaborator

I'm also seeing this issue. When you type in an input that has validations, nothing happens to the input--it remains blank. This issue is not present in v0.5.0, but does show up in v0.5.1 and v0.5.2

@joelkoz
Copy link
Collaborator

joelkoz commented Oct 31, 2016

For those who are experiencing this problem: are you specifying the "updateImmediately" property? I looked at my code again from the last time I reported that I had been experiencing this issue, and I noticed that I started setting the "updateImmediately=true" property in a code change. The FormsyText.js component takes a different code path if updateImmediately is set to truthy, and it AVOIDS the "this.resetValue()" call in the "handleChange" method that I found to be responsible for preventing fields from updating if validation was specified. Try setting the "updateImmediately=true" in your FormsyText components and see if that makes a difference.

joelkoz added a commit to joelkoz/formsy-material-ui that referenced this issue Oct 31, 2016
Issue formsy#152 - fields that have a validation specification do not allow
input at all if the first character typed causes the field to be
invalid.
@joelkoz
Copy link
Collaborator

joelkoz commented Oct 31, 2016

@mbrookes I just confirmed (at least on my machine) that an "npm test", which appears to test FormsyText, fails on the current code in the master branch for 0.5.2. It complains about a lack of an "text is too long" invalid error. It DOES pass with the the above pull request, so I think the removal of that line is important.

@mbrookes
Copy link
Collaborator

I sincerely apologise that these sorts of problems have crept in to recent releases - the fault is entirely mine.

Long story short, I haven't been actively developing anything for the last 6 months or so. I've tried to keep up with critical fixes that the community has kindly submitted, but my inability to properly test fixes before releasing them has resulted in unfortunate problems.

If anyone would like to take over repo, or alternatively, if you have a project that offers a similar solution that you'd like me to highlight, please do let me know!

@ryanblakeley
Copy link
Collaborator

@mbrookes:

You've done a great job with this project, dude. I've shown it to several people who really dig it. I changed my username, but it was @rblakeley -- we collaborated on fixing a bug that I think led to this project. I'm game for taking over if you want.

@rmoskal
Copy link

rmoskal commented Oct 31, 2016

I'd be happy to lend a hand to fix things and add more tests, etc.

On Mon, Oct 31, 2016 at 12:35 PM Ryan B. Blakeley notifications@github.com
wrote:

@mbrookes https://github.com/mbrookes:

You've done a great job with this project, dude. I've shown it to several
people who really dig it. I changed my username, but it was @rblakeley --
we collaborated on fixing a bug that I think led to this project. I'm game
for taking over if you want.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAiTc_lLsQxcWMiQmt1TpX-_wbE3S3Jxks5q5hjNgaJpZM4KceRa
.


Robert Moskal
Most Media
Brooklyn, USA

@mbrookes
Copy link
Collaborator

Hey Ryan, good to hear from you! You're still credited in the README for the original idea. 👍

Thanks to you and @rmoskal for offering to step in. I've added you both as collaborators, joining @codeaholicguy who has done great work already! I'll figure out NPM access when I get a chance (it's midnight here), but Nguyễn has access for now if you need to publish something.

Thanks again, and shout if I need to do anything else to help with the transition.

@matias-casal
Copy link

keep this repo alive! 😁

@rmoskal
Copy link

rmoskal commented Nov 3, 2016

It looks to me like the problem happens when you don't specify a validationError property. The logic that we've all pointed to just needs to be tweaked.

If no one else takes care of it I will in the next few days.

@matias-casal
Copy link

@rmoskal setting updateImmediately={true} the error disapears.
if @mbrookes cant keep the repo, you should
@joelkoz can help to.

@rmoskal
Copy link

rmoskal commented Nov 3, 2016

Then maybe there is no error and just a lack of documentation.

Though it does seem like this is a breaking change.

I just started using the package myself.

On Thu, Nov 3, 2016, 6:22 PM Casy notifications@github.com wrote:

@rmoskal https://github.com/rmoskal setting updateImmediately={true} the
error disapears.
if @mbrookes https://github.com/mbrookes cant keep the repo, you should
@joelkoz https://github.com/joelkoz can help to.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAiTc6sGaUlFgVYxcWrgXqQV6zA9Umeuks5q6l6YgaJpZM4KceRa
.


Robert Moskal
Most Media
Brooklyn, USA

@joelkoz
Copy link
Collaborator

joelkoz commented Nov 4, 2016

@rmoskal - No, there is definitely a bug that was recently introduced. It just so happens that if you use updateImmediately={true} property, the text control takes a different code path and avoids the validation bug. So, that is a work-around, not a fix.

If anyone is interested, I have a branch on my fork of this project named "current-fixes" which contains the two fixes that I have made to the project. You can check out that branch if you wish to get this working again.: https://github.com/joelkoz/formsy-material-ui/tree/current-fixes

@mbrookes
Copy link
Collaborator

mbrookes commented Nov 5, 2016

@joelkoz Could you submit your fixes as a PR? It will make it easier to review. Thanks so much!

@joelkoz
Copy link
Collaborator

joelkoz commented Nov 5, 2016

@mbrookes - I did already. There are two from me waiting.

-jk
(sent from my iPhone)

On Nov 4, 2016, at 9:01 PM, Matt Brookes notifications@github.com wrote:

@joelkoz Could you submit your fixes as a PR? It will make it easier to review. Thanks so much!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@joelkoz
Copy link
Collaborator

joelkoz commented Nov 5, 2016

@mbrookes - I just added a third PR to change the material-ui version to 0.16.0 to fix #153. There don't seem to be any breaking changes in 0.16.0 that affect this project.

@MickaelBZH
Copy link

How is it going?

@telemakhos
Copy link

telemakhos commented Nov 15, 2016

What's up with the PRs fixing this? Hello @mbrookes ?

Installing joelkoz fork for the time being...

@ryanblakeley
Copy link
Collaborator

Just merged several PRs, including those submitted by @joelkoz aimed at fixing this. If we could get some thumbs up on this comment to signal that the @latest is solving this issue, then we can close.

@jadus
Copy link

jadus commented Nov 18, 2016

@telemakhos Sorry but I'm not an npm power user. How can I install joelkoz fork ? Can't find a good tutorial to do this.
Thanks a lot

@joelkoz
Copy link
Collaborator

joelkoz commented Nov 19, 2016

@rojobuffalo - npmjs currently has the old 0.5.2 as the latest version. Any reason to not bump up the patch version and publish a 0.5.3? We know for sure that what is out there is broken, so it can't get any worse.

@mbrookes
Copy link
Collaborator

@rojobuffalo Let me know if you need npm owner rights. @joelkoz I invited you as a collaborator. 👍

@ryanblakeley
Copy link
Collaborator

@joelkoz I was waiting for confirmation from someone else that the latest updates are all working properly as they see it.

@telemakhos
Copy link

telemakhos commented Nov 20, 2016

@rojobuffalo Working ok for us.. I think it's safe to bump up the version.

@jadus You do it with npm i <repoURL> and then building if required, but I think you should just wait and install the official version once they publish it on npm...

@ryanblakeley
Copy link
Collaborator

ryanblakeley commented Nov 21, 2016

@mbrookes I do need npm publish permission. My npm username is rojobuffalo.

@codeaholicguy
Copy link
Collaborator

@rojobuffalo (cc @mbrookes) I added you into npm, please check it again

@ryanblakeley
Copy link
Collaborator

Thanks @codeaholicguy. Just published 0.5.3

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

No branches or pull requests