Skip to content

🐛 Input Handlers were being ignored on first invoke#38

Merged
Kerrick merged 3 commits intosecondstreet:masterfrom
KnownSubset:input-handlers-event-listener-bug
Oct 10, 2017
Merged

🐛 Input Handlers were being ignored on first invoke#38
Kerrick merged 3 commits intosecondstreet:masterfrom
KnownSubset:input-handlers-event-listener-bug

Conversation

@KnownSubset
Copy link
Copy Markdown

@KnownSubset KnownSubset commented Oct 10, 2017

New Feature

What will it allow you to do that you can't do today? Tie action handlers to input event listeners

{{mdc-textfield 
   label="test" 
   value=test 
   onfocus=(action 'logToConsole' 'focus')
   onblur=(action 'logToConsole' 'blur')
   onkeydown=(action 'logToConsole' 'keydown')
   oninput=(action (pipe (action 'logToConsole' 'input')))
}}
actions: {
  logToConsole(event) {
    console.log("hi from the action:" + event); 
  },
}

Why do you need this feature and how will it benefit other users? The only method to registering input event listeners is to pass inputKeydownHandlers directly, but that's something that's supposed to be private and interface with the MDC Foundation class.

Are there any drawbacks to this feature? No, this mirrors the way other inputs handle registering event listeners.

Found a bug (this is outdated, but was the source of the new feature)

Which version of the project did you use when you noticed the bug? 0.0.36
How do you reproduce the error condition?

{{mdc-textfield label="test" value=test inputKeydownHandlers=(array (action "handleKeyPress")) }}
actions: {
  handleKeyPress() {
    console.log('hi from the action');  // is not invoked on first key down event
  },
}

What happened that you think is a bug? The action handler was not invoked on first keypress

What should it do instead? The action handler should be invoked on input event

PR details

Did you confirm this fix/feature is something that is needed? Yes

Did you write automated tests? Nope, since there are not any real tests currently written

Did you add documentation for the changes you made? Yes, updated the application.hbs to detail that someone can register event handlers to the text-field.

In lieu of a style guide, did you match your code style to the rest of the project? Yes

@Kerrick
Copy link
Copy Markdown
Contributor

Kerrick commented Oct 10, 2017

It's probably a good idea not to encourage people to pass in inputKeydownHandlers directly, as that's something that's supposed to be private and interface with the MDC Foundation class. Instead, how about exposing onkeydown and other event handlers like {{mdc-textfield/input}} does?

//region Attributes
/**
* @type {Function}
* @param {jQuery.Event}
*/
onfocus: x => x,
/**
* @type {Function}
* @param {jQuery.Event}
*/
onblur: x => x,
/**
* @type {Function}
* @param {jQuery.Event}
*/
oninput: x => x,
/**
* @type {Function}
* @param {jQuery.Event}
*/
onkeydown: x => x,
//endregion

Then, you can call those from the handle action of {{mdc-textfield}}.

handle(type, ev) {
get(this, Ember.String.camelize(`input ${type} handlers`)).forEach(handler => handler(ev));
},

Nathan Dauber and others added 2 commits October 10, 2017 14:08
Added `onfocus`, `onblur`, `onkeydown`, and `oninput` event handlers for the `mdc-textfield`.
@Kerrick
Copy link
Copy Markdown
Contributor

Kerrick commented Oct 10, 2017

Thank you so much for the PR! This will ship out with v0.0.37 👍

Kerrick added a commit that referenced this pull request Oct 19, 2017
- 🐛 {{mdc-tab}} indicator fixes (#31)
- 🔧 Use unminified js/css in non-prod environments (#46)
- 📝 Add contributor gallery to README (#40)
- ✨ Implement {{mdc-layout-grid}} (#42)
- ✨ Implement {{mdc-grid-list}} (#43)
- 🐛 📝 Fix toolbar bug, add toolbar docs. (#37)
- 💡 📝 Update documentation to use ember-freestyle (#44)
- 🐛 {{mdc-textfield}}'s placeholder and value no longer overlap (#41)
- 💥 {{mdc-card}}'s children now yield their own children (#39)
- ✨ {{mdc-textfield}} exposes event handlers (#38)
- ✨ {{mdc-list}} now yields text and secondary text (#36)
- 📝 Create CODE_OF_CONDUCT.md (#35)
@KnownSubset KnownSubset deleted the input-handlers-event-listener-bug branch October 23, 2017 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants