Add disableOnInputFields option, which allows to automatically disable keyboard events on all input fields #557
Conversation
Could you please add more details about what you expect? Like a duplicated acceptance test (ember-keyboard-test.js), with the new option enabled, so we can ensure everything still works as expected? |
Sounds like a good start. The place where this change is affects nearly all the functionality of the plugin so we would want the tests to give us confidence that it is doing so in an expected manner, and that future changes do not affect the intended effect of this configuration option. |
|
cool. Just pushed some tests. Hopefully that will do. No offence, but the tests are just so much adapted to this addon, it's quite an effort to figure out what is going on. |
| @@ -65,6 +69,15 @@ export default class KeyboardService extends Service { | |||
|
|
|||
| @action | |||
| _respond(event) { | |||
| if (this._disableOnInput && event.target) { | |||
There was a problem hiding this comment.
@st-h @lukemelia I think it may be more flexible to be able to configure this per modifier instance, however tat may be done as a next step enhancement
|
@st-h would you mind to add docs for this new feature so it could be explored by others? |
@SergeAstapov I've added a short paragraph to tests/dummy/app/templates/usage.hbs |
|
@st-h my bad, missed it on first pass. This looks good to me! We can iterate on tests and restructure them in a follow up PRs. Thanks for the call out! |
| ); | ||
| }); | ||
|
|
||
| test('test event does not propagate on input field', async function (assert) { |
There was a problem hiding this comment.
This is the key test of the file. Let's move it to the top and also add one that shows a key event firing associated with a non-input element and still triggering responders.
There was a problem hiding this comment.
moved that test to the top. What is wrong with the other tests In order to ensure that? Am I missing something? I copied all the other tests, because they test the standard features and added a test that makes sure input fields now do not propagate. Anything special you are looking for?
|
sorry about taking so long. I've made the requested changes. Yet, I wonder... wouldn't it have been easier to just make the changes instead of writing them down here? |
|
Released as 7.0.1 |
resolves #212