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

[paper-input] preventing exception when typing with mobile keyboard #551

Merged
merged 4 commits into from
Nov 8, 2016

Conversation

Juraci
Copy link
Contributor

@Juraci Juraci commented Nov 8, 2016

I got the following exception on paper-input when typing with the Android keyboard:

paper-input error
Cannot read property 'val' of undefined.

Steps to reproduce:

requirements:
Android smartphone with google chrome

  1. Open Chrome dev tools and setup it for remote debugging
  2. On the phone open this page dialog example
  3. In the prompt dialog example type some text with the phone keyboard (has to be with the phone keyboard)
  4. Click on confirm action on the dialogue

Expected: Dialog closes
Actual: Dialog remains opened with the error linked above

The fix in the pull request prevents this exception while maintaining the current behavior.

@Juraci Juraci changed the title [paper-input] preventing exception when typing with mobile keybord [paper-input] preventing exception when typing with mobile keyboard Nov 8, 2016
@waldemarnt
Copy link

👍

@miguelcobain
Copy link
Collaborator

@Juraci I wonder why this fix is needed. I'm generally opposed to merging fixes without knowing what is the root cause of it. Do you have any idea?

@Juraci
Copy link
Contributor Author

Juraci commented Nov 8, 2016

@miguelcobain I did not have the time to dig deeper into why this is happening on the mobile only. I do suspect that the setValue method is being called after a dialog action has been made, but I do not know why this happens when using the mobile keyboard only.

A fix is definitely needed because the prompt dialog is currently unusable in mobile platforms

@miguelcobain
Copy link
Collaborator

miguelcobain commented Nov 8, 2016

@Juraci I think I know the root cause of the problem.
We're using run.next with setValue here: https://github.com/miguelcobain/ember-paper/blob/master/addon/components/paper-input.js#L144-L146

What I suspect is happening is that, by the time setValue runs, the template is already destroyed (especially on android due to low performance, I assume).
The proper fix would be to test if the component is destroyed, before calling setValue. We can test that checking this.isDestroyed value.

The code would become something like:

run.next(() => {
  if (this.isDestroyed) { return; }
  this.setValue(this.get('value'));
});

Could you please test this, and if it solves the issue, update the PR?

@Juraci
Copy link
Contributor Author

Juraci commented Nov 8, 2016

@miguelcobain, sure I will do that.

@Juraci
Copy link
Contributor Author

Juraci commented Nov 8, 2016

@miguelcobain you are correct, this solves the problem, just tested it here.

@miguelcobain
Copy link
Collaborator

@Juraci Great. While you're at it, could you please add an entry to the changelog?

@miguelcobain miguelcobain merged commit 0fb9433 into adopted-ember-addons:master Nov 8, 2016
@miguelcobain
Copy link
Collaborator

Thanks a lot for this!

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

Successfully merging this pull request may close these issues.

3 participants