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

Blur event doesn't remove focus from Input inside the TextField component in mobile [issue already with a possible Fix] #366

Closed
fabiozaffani opened this issue May 14, 2017 · 3 comments
Labels
Milestone

Comments

@fabiozaffani
Copy link

fabiozaffani commented May 14, 2017

This problem is related to the component TextField in the following file
https://github.com/mlaursen/react-md/blob/1e709eb98a5933c6199db127865edea68ef45dd4/src/js/TextFields/TextField.js

At line 456 and 457 you will find

fn('mousedown', this._handleOutsideClick);
fn('touchstart', this._handleOutsideClick);

which reference the following function in line 611

_handleOutsideClick(e) {
    if (!this._node.contains(e.target)) {
      this._blur();
    }
  }

All of this will trigger a blur on the Input container (the TextField component) but will not trigger a blur on the inner input field. This problem can be detected in mobile when you touchstart scrolling the page. The TextField will lose its focused layout (label back to it's place and colors removed) but the cursor will still be inside the Input field which will be focused. Testing locally a simple fix was to add a line in the function above, which will become:

_handleOutsideClick(e) {
    if (!this._node.contains(e.target)) {
      this._blur();
      this._field.blur(); // remove the focus from the input field
    }
  }

Another better fix to keep everything consistent would be to add it as a callback to the setState of the _blur function (line 600), which would then become:

_blur() {  
    const value = this._field.getValue();

    const state = { active: false, error: this.props.required && !value };
    if (!this.props.block) {
      state.floating = !!value;
    }

    this.setState(state, () => this._field.blur());
  }

I can make a PR if you want, just tell me which fix you think it's better and which suits better your coding choice.

Tested in

  • React 15.4.2
  • React-MD 1.0.13
  • Browser Chrome 58.0.3029.96 (64-bit)
@fabiozaffani fabiozaffani changed the title Blur event don't remove focus from Input inside the TextField component in mobile Blur event don't remove focus from Input inside the TextField component in mobile [issue already with a possible Fix] May 14, 2017
@fabiozaffani fabiozaffani changed the title Blur event don't remove focus from Input inside the TextField component in mobile [issue already with a possible Fix] Blur event doesn't remove focus from Input inside the TextField component in mobile [issue already with a possible Fix] May 14, 2017
@mlaursen mlaursen added the bug label May 18, 2017
@mlaursen mlaursen added this to the v1.0.15 milestone May 23, 2017
@mlaursen
Copy link
Owner

I think the first approach would be best just since the keydown (tab) listener also calls the blur function and might trigger 2 blur events? If it doesn't, I like the second approach better.

I'd welcome a pull request if you have the time!

mlaursen added a commit that referenced this issue May 24, 2017
Updated the TextFields so that they correctly blur when a mobile device
scrolls after focusing a text field.

Closes #366
@mlaursen
Copy link
Owner

I actually had time today and implemented your second suggestion. Thanks!

@fabiozaffani
Copy link
Author

Hi @mlaursen , sorry by the delay, just saw your response but you fixed it already :)

Btw, you are doing an amazing job with this library, having already used the react material ui library, google-md (now discontinued), react-toolbox and implementend my own I can confidently say that yours are, by far the best one around.

You even have deprecated notices in the documentation! Never saw that before in a one man only open source library.

Once again, congratz!

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

No branches or pull requests

2 participants