cibernox/ember-native-dom-helpers

fillIn should fire onKeyUp and onKeyDown events

lstrzebinczyk opened this issue · 3 comments

lstrzebinczyk commented

Hey

Current implementation of fillIn helper doesn't trigger onKeyUp and onKeyDown events. This is how users actually interact with forms, and if there is logic implemented to run after those events, fillIn can't be used.

I would like to suggest, that fillIn is extended to do the following:

  1. First, clean up the input entirely, run input and change events on this state
  2. Write one character at a time, run onKeyUp and onKeyDown events as needed
  3. Only after the entire content was written, run input and change events on the entire written input

Would you accept an MR with those changes?

EDIT

Since that would be backwards incompatible for people that count number of called events in tests, a new function called writeIn with this behavior could be added. fillIn would simulate copy-paste vehavior, and writeIn would more closely match user writing down.

cibernox commented

I do not think that fillIn should use keyUp/Down events really. First of all, most of the time using keyUp/Down is a mistake, and users should use oninput and onchange, because it's not true that inputs can only be filled by typing. They can be filled with right-click > menu > paste, or with drag and drop of a selection, or even using the keyboard, the content can be introduced with ctrl-c + ctrl-v, or with an "undo" operation (ctrl-z).

Developers very often forget about these edge cases and use keyUp which is wrong 90% of the time and leave they apps broken in subtle ways.
For the other 10%, perhaps a different helper, like typeText(selector, str) could be helpful, and I wouldn't oppose to it, although it will be tricky to properly replicate how users type: key events holding SHIFT for caps, sequences of keypresses for accents... It's not a trivial task to do simulate it properly.

If you want to do it I can help, but I don't think we should modify how fillIn works.

lstrzebinczyk commented

I do realize now that we might not cover some of those edge cases you mentioned, and I will look into that in our code.

Nevertheless, if that is ok with you, I'll try to provide a PR with the typeText helper.

cibernox commented

That helper might be helpful for some people. Let me know if you make such PR.

In the meantime, I'm going to close this issue.