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

[BUG]: Formsy-react is using state._value but formsy-material-ui using state.value #133

Closed
Stupidism opened this issue Sep 6, 2016 · 10 comments

Comments

@Stupidism
Copy link

Stupidism commented Sep 6, 2016

I found the bug in your source code because Chrome auto-filled but Formsy-react not recieving onChange event. I think it's a bug widely ranged in this repo that guys like me who don't know well about this repo can't fix it totally.

Cause:

Chrome autofill triggers change event
Formsy-react is using this.state._value
But formsy-material-ui uses setState({value}) when recieving onChange event

Quick fix for FormsyText:

  handleChange: function handleChange(event) {
    ...
    this.setState({
      value: event.currentTarget.value,
    });
    // change to
    this.setValue(event.currentTarget.value);
    ...
  },
  ...
  render() {
    ...
        value={this.state.value}
        // change to
        value={this.state._value}
    ...
  }
@Stupidism
Copy link
Author

Stupidism commented Sep 6, 2016

I think there are some related issues:

@Stupidism Stupidism changed the title [BUG]: Chrome auto-filled but Formsy-react not recieving onChange event. [BUG]: Formsy-react is using state._value but formsy-material-ui using state.value Sep 6, 2016
@achtan
Copy link

achtan commented Nov 29, 2016

can u pls fix this

@Stupidism
Copy link
Author

Stupidism commented Nov 30, 2016

@achtan
Sorry, I'm busy with company jobs and recently, these work is not related to formsy-material-ui.
Btw, it seems formsy-react is not maintained any more. I'm considering use redux-form instead.

@ryanblakeley
Copy link
Collaborator

This looked like a pretty well explained bug that should be fixed. I'm going to reopen. Also, I've seen no evidence that formsy-react is no longer being maintained. Can anyone else lend any credibility to that claim?

@ryanblakeley ryanblakeley reopened this Dec 1, 2016
@Stupidism
Copy link
Author

Stupidism commented Dec 2, 2016

Sorry, I wanted to say formsy-material-ui is not maintained, at least, community is not active. As I can see, #125 was issued in Aug, and it's not fixed yet, which is actually an important bug.

@ryanblakeley
Copy link
Collaborator

Hey @Stupidism, in your quick fix example, it doesn't look like you setState({_value:.... Was that a typo? Or where would you set _value?

@Stupidism
Copy link
Author

Stupidism commented Dec 5, 2016

this.setValue is the same as this.setState({ _value }). It's something from formsy-react @rojobuffalo

@Stupidism
Copy link
Author

Stupidism commented Dec 5, 2016

Actually, I don't see any necessity to use this.state.value from my glimpse. I think maybe it's a legacy when formsy-react change from value to _value, which is a good change, but formsy-material-ui didn't follow up. Major bugs happend and someone found that add this.setValue() with this.setState({value}) together wound fix it. As a result, it's not fixed in somewhere, like here. And that's why I think this repo is not maintained well now.

@Stupidism
Copy link
Author

Sorry for my arbitrary words before. that's what I was thinking when creating this issue.

I took a look at the current code and latest commits. You are doing a great job, saving a dying repo. @rojobuffalo

Now I find that this issue is somehow solved already. The original title was Chrome auto-filled but Formsy-react not recieving onChange event. .

This issue was one of two things that bothered me very much. The other was that one day I found required error not showing anymore. I think I can close this issue and follow #179 instead

@ryanblakeley
Copy link
Collaborator

@Stupidism ok sounds good.

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

3 participants