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

[TextField] Spurious onBlur() with undefined event #9027

Closed
1 task done
nareshbhatia opened this issue Nov 7, 2017 · 6 comments
Closed
1 task done

[TextField] Spurious onBlur() with undefined event #9027

nareshbhatia opened this issue Nov 7, 2017 · 6 comments
Labels
bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@nareshbhatia
Copy link
Contributor

onBlur() is being called on a TextField with event parameter as undefined

  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

onBlur() should always be called with an event.

Current Behavior

In some situations, onBlur() is being called with an undefined event.

Steps to Reproduce (for bugs)

I am not able to reproduce this in a small example, but I can tell you which code path is making this spurious call to onBlur. The culprit is essentially the componentWillUpdate() method in Input.js. See here.

  1. On line 395, there is a call to muiFormControl.onBlur(), without passing an event!!!
  2. This calls FormControl.handleBlur(), which expects an event.
  3. This in turn calls this.props.onBlur(event), which is my event handler that gets an undefined event.

Context

This issue is requiring me to check for an undefined event in the beginning of my event handler.

Your Environment

Tech Version
Material-UI 1.0.0-beta.20
React 16.0.0
browser Chrome
etc
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 7, 2017

@nareshbhatia This is a nice finding. The logic was introduced in order to fix #8811. What do you think of asserting that the event is defined before triggering the property callback here?
https://github.com/callemall/material-ui/blob/86dbd4986c6fdc1f32c088abb3b73d25974272e8/src/Form/FormControl.js#L180-L182

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. labels Nov 7, 2017
@nareshbhatia
Copy link
Contributor Author

@oliviertassinari, I think that would work perfectly. It is essentially what I am doing in my event handler, but better to catch this upstream in the framework code.

@oliviertassinari
Copy link
Member

@nareshbhatia Do you want to give it a try?

@nareshbhatia
Copy link
Contributor Author

Sure. Will send you a PR.

@reznord
Copy link

reznord commented Nov 10, 2017

@oliviertassinari in current stable version (v0.19.x) it is not possible to use onBlur() for textFields right?

If so, where do you recommend to validate the fields? (onChange)?

@oliviertassinari
Copy link
Member

@reznord I have no clue.

the-noob pushed a commit to the-noob/material-ui that referenced this issue Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

3 participants