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

Multiple input events on MdInput #1261

Closed
korylprince opened this Issue Dec 4, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@korylprince
Copy link
Contributor

commented Dec 4, 2017

Edit: See this comment for what I believe fixes this issue.

Steps to reproduce

Use vue-dev-tools on the vue-material docs. (I can't seem to get the dev tools to activate on vuematerial.io, so I'm using a local copy a la npm run dev.)

Watch the events and type in an input, textarea or change a select.

Which browser?

Latest Vue, git dev version of vue-material, latest stable Chrome

What is expected?

I would expect to only receive one input event. In the case of the select, I would expect to receive one event with the value of the selection.

What is actually happening?

You'll see two or three events for each key stroke. In case of the select, you'll get one input event with the value of the selection and another with the inner text of the selection.

Investigation

In the case of MdInput, there's three sources that these events come from. First in listeners() in MdInput.vue.

In MdFieldMixin.js the set(value) for the computed model, and the watcher for value both set localValue which causes an emit.

I don't know enough about how MdInput is supposed to work to fix it. I did notice that set(value) for model checks if value is an InputEvent. In all my testing the only thing value ever is is a String.

@korylprince

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2017

Update: I'm realizing now, at least for the select problem, that one input event is coming from the MdSelect, and one is coming from the MdInput (built into the MdSelect?), which makes sense. But I still wouldn't think you'd want both events firing...

Edit: I'm dumb. Just because the the Vue dev tools shows the event doesn't mean it get's passed up to the parent. So the MdSelect is working just fine.

But the original issue with MdInput is still the same. The parent is seeing multiple input events.

@korylprince

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2017

Okay with the above in mind, I've isolated the events to two places. MdInput's $listeners (i.e. @input on the actual input) and to the localValue watcher in MdFieldMixin.

I can't find a time when localValue doesn't emit on a change, so it seems like the input listener in MdInput is redundant. Perhaps it should become something like:

  listeners () {
    var l =  this.$listeners
    delete l.input
    return l
  }

or something similar? If someone really wanted to get the native input event, they could just use @input.native.

If this is acceptable, I can do a pull request to fix this issue with MdInput and MdTextarea.

@VdustR

This comment has been minimized.

Copy link
Member

commented Dec 6, 2017

  listeners () {
    var l =  this.$listeners
    delete l.input
    return l
  }

This way would modify the $listener object from Vue API which is read only therefore I prefer to construct another object. And it's really not good to change any data in a computed.

@korylprince

This comment has been minimized.

Copy link
Contributor Author

commented Dec 6, 2017

@VdustR Yeah, that was just example what I think listeners should output. I didn't want to complicate my example with object-cloning code.

@VdustR

This comment has been minimized.

Copy link
Member

commented Dec 6, 2017

try

  listeners () {
    return {
        ...this.$listeners,
        input: undefined
    }
  }

@Samuell1 Samuell1 added the question label Dec 8, 2017

@korylprince

This comment has been minimized.

Copy link
Contributor Author

commented Dec 8, 2017

Oh, that's much more elegant than I was thinking. I'm just waiting on some input from the devs before I do a pull request.

@Samuell1

This comment has been minimized.

Copy link
Member

commented Dec 8, 2017

@korylprince make PR and we can look at it. Its faster then waiting for response in issue.

@korylprince

This comment has been minimized.

Copy link
Contributor Author

commented Dec 8, 2017

@Samuell1 👍

marcosmoura added a commit that referenced this issue Dec 22, 2017

fix(MdField): prevent MdInput and MdTextarea from emit input events d…
…irectly (#1285)

* (Fix #1261) Don't emit input directly in MdInput and MdTextarea

* use spread operator (+babel) instead of Object.assign for browser compatiblity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.