Add `typeIn` helper to trigger keyup, keypress and keyup events when filling inputs #397

mfeckie · 2018-07-14T08:55:39Z

The current implementation of fillIn does not trigger keydown, keypress and keyup events.

I've recently been working on some stuff that uses these events. One example being a formatted text input for bank account numbers that only allow certain characters.

I'm not sure if other folks would find this helpful so submitting as a PR for consideration / discussion.

cibernox

I personally oppose to this change. I do think it's valuable but it should be, at least at first, a different helper.

Also if we want to do it right, it should modify the value char by char triggering the full sequence of events for each.

Ideally it should also allow to specify a delay between keystrokes to simulate slow typing for scenarios with debouncing.

mfeckie · 2018-07-14T09:27:16Z

Consider it a proof of concept. I'm happy to make it a seperate helper. What would it be called?

Also happy to update the input char by char and allow delays

mfeckie · 2018-07-14T09:29:39Z

How about fillUsingKeyEvents ?

cibernox · 2018-07-14T09:40:28Z

My preference is "typeIn"

mfeckie · 2018-07-14T09:41:19Z

❤️ Less verbose and says what it does :) will use

mfeckie · 2018-07-14T10:01:02Z

Looking at the codebase, it seems that async/await is explicitly disabled, which would make cleanly implementing a delay between each key input difficult. Suggestions welcome.

Turbo87 · 2018-07-14T10:07:11Z

@mfeckie you can use promise chains instead

mfeckie · 2018-07-14T13:10:08Z

OK folks, I've updated the PR to address all the feedback and suggestions so far.

I've also updated the documentation to go along with it.

mfeckie · 2018-07-19T10:09:55Z

Any updates on the this folks?

cibernox · 2018-07-19T12:32:24Z

tests/unit/dom/type-in-test.js

+import hasEmberVersion from 'ember-test-helpers/has-ember-version';
+
+let expectedEvents = [
+  'focus',


After some investigation, I think the order of the events may be wrong. Check this jsbin: https://jsbin.com/zitazuxabe/edit?html,js,console,output

Seems that the sequence when typing "abc" is:

keydown | e.target.value is "" keypress | e.target.value is "" input | e.target.value is "a" keyup | e.target.value is "a" keydown | e.target.value is " a" keypress | e.target.value is "a" input | e.target.value is "ab" keyup | e.target.value is "ab" keydown | e.target.value is " ab" keypress | e.target.value is "ab" input | e.target.value is "abc" keyup | e.target.value is "abc" change | e.target.value is "abc"

Changes I request:

input fires before keyup

It's important that keydown and keypress are fired before the input gets the new value, and the rest are fired after

The change event should only be fired once at the end.

Can you add this JSBin as an inline comment? It sure will make "reality" testing easier over time...

cibernox · 2018-07-19T12:33:11Z

@rwjblue how do you feel about this helper once it has been polished?

cibernox · 2018-07-19T14:39:18Z

Code-wise, I think this is good. I'll defer to @rwjblue to decide wether we want this helper in this repo or in another one, but sounds good.

rwjblue · 2018-07-19T15:42:50Z

Conceptually, I think it is a good helper to have. I haven't had much time to dig into the implementation just yet though (sorry).

mfeckie · 2018-07-31T07:47:30Z

Anything I can do to help progress this good humans?

rwjblue

Looking good, only a few suggested tweaks/changes...

rwjblue · 2018-07-31T14:27:08Z

API.md

@@ -3,14 +3,15 @@
 ### Table of Contents

 -   [DOM Interaction Helpers](#dom-interaction-helpers)
+    -   [blur](#blur)


Can you reset the changes here to API.md? I'll regenerate it before publishing (this way the API.md on master roughly continues to match what actual users are using)...

rwjblue · 2018-07-31T14:28:46Z

addon-test-support/@ember/test-helpers/dom/type-in.js

+ * @param {number} millisecondDelay number of millisecons to wait per keypress
+ * @return {Promise<void>} resolves when the application is settled
+ */
+export default function typeIn(target, text, millisecondDelay = 0) {


Lets go with an { delay: 50 } for the third arg

rwjblue · 2018-07-31T14:29:56Z

addon-test-support/@ember/test-helpers/dom/type-in.js

+
+// eslint-disable-next-line require-jsdoc
+function fillOut(element, text, delay) {
+  return new Promise(resolve => {


I don't understand this promise nesting

The reason for the promise nesting is because fillOut takes each step and executes keyDown, keyPress, input and keyUp events for each character in the input string.

We need to ensure that each promise is triggered in order.

Without this, everything essentially happens without delay.

Hmm, I don’t think that I see what you mean. It seems that executeEvents does all of that and fillOut just wraps another promise around the promise returned by executeEvents without doing anything else.

Yep, your right. Have removed. Sorry for misunderstanding the original question!

rwjblue · 2018-07-31T14:31:29Z

addon-test-support/@ember/test-helpers/dom/type-in.js

+
+    __focus__(element);
+
+    return fillOut(element, text, millisecondDelay)


Any reason we can't use executeEvents directly here? I don't see what fillOut provides us...

return executeEvents(element, text, delay).then(() => fireEvent(element, 'change'))

fillOut packages up the promise tree.

rwjblue · 2018-07-31T14:31:48Z

addon-test-support/@ember/test-helpers/dom/type-in.js

+      throw new Error('Must provide `text` when calling `typeIn`.');
+    }
+
+    __focus__(element);


Does __focus__ return a promise that we need to chain?

This is exactly the same as I found it in fill-in.js so I kept for consistency

rwjblue · 2018-07-31T14:33:35Z

tests/unit/dom/type-in-test.js

+import hasEmberVersion from 'ember-test-helpers/has-ember-version';
+
+let expectedEvents = [
+  'focus',


Can you add this JSBin as an inline comment? It sure will make "reality" testing easier over time...

makepanic · 2018-08-07T07:50:32Z

fillIn supports contenteditable as target elements.
Is it possible to add this to the typeIn helper too?

In its current state, one can't use typeIn for contenteditable elements.

mfeckie · 2018-08-07T07:53:47Z

I specifically excluded contenteditable as I don't believe it follows the same event pattern as native inputs.

Perhaps you could take a look after / if this PR gets merged?

mfeckie · 2018-08-09T02:25:21Z

@rwjblue I think I've addressed all your feedback? Anything outstanding?

mfeckie · 2018-08-25T12:04:19Z

Bump

rwjblue

Sorry for letting this sit so long @mfeckie.

mfeckie force-pushed the feature/add-fill-in-with-keyboard-events branch from 3e32044 to 7119b82 Compare July 14, 2018 09:13

cibernox reviewed Jul 14, 2018

View reviewed changes

mfeckie force-pushed the feature/add-fill-in-with-keyboard-events branch from 7119b82 to f62242f Compare July 14, 2018 13:09

Turbo87 changed the title ~~Allow fillIn to trigger keyup, keypress and keyup events as well~~ Add typeIn helper to trigger keyup, keypress and keyup events when filling inputs Jul 19, 2018

cibernox requested changes Jul 19, 2018

View reviewed changes

cibernox approved these changes Jul 19, 2018

View reviewed changes

rwjblue requested changes Jul 31, 2018

View reviewed changes

Add a typeIn helper with optional delay

3478211

mfeckie force-pushed the feature/add-fill-in-with-keyboard-events branch from 5d098b0 to 3478211 Compare August 1, 2018 09:42

Remove unecessary extra function

fa35ed8

rwjblue added the enhancement label Aug 6, 2018

rwjblue approved these changes Sep 27, 2018

View reviewed changes

rwjblue merged commit 1e1d54c into emberjs:master Sep 27, 2018

rwjblue mentioned this pull request Oct 2, 2018

Update @ember/test-helpers to v0.7.26. emberjs/ember-qunit#339

Merged

rwjblue mentioned this pull request Oct 2, 2018

Update minimum version of @ember/test-helpers to 0.7.26. emberjs/ember-mocha#222

Merged

ro0gr mentioned this pull request Jan 21, 2019

Ability to send keypress events to component element san650/ember-cli-page-object#312

Open


		__focus__(element);

		return fillOut(element, text, millisecondDelay)

Add typeIn helper to trigger keyup, keypress and keyup events when filling inputs #397

Add typeIn helper to trigger keyup, keypress and keyup events when filling inputs #397

Conversation

mfeckie commented Jul 14, 2018

cibernox left a comment

Choose a reason for hiding this comment

mfeckie commented Jul 14, 2018

mfeckie commented Jul 14, 2018

cibernox commented Jul 14, 2018

mfeckie commented Jul 14, 2018 • edited Loading

mfeckie commented Jul 14, 2018

Turbo87 commented Jul 14, 2018

mfeckie commented Jul 14, 2018

mfeckie commented Jul 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cibernox commented Jul 19, 2018

cibernox commented Jul 19, 2018

rwjblue commented Jul 19, 2018

mfeckie commented Jul 31, 2018

rwjblue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfeckie Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makepanic commented Aug 7, 2018 • edited Loading

mfeckie commented Aug 7, 2018

mfeckie commented Aug 9, 2018

mfeckie commented Aug 25, 2018

rwjblue left a comment

Choose a reason for hiding this comment

Add `typeIn` helper to trigger keyup, keypress and keyup events when filling inputs #397

Add `typeIn` helper to trigger keyup, keypress and keyup events when filling inputs #397

mfeckie commented Jul 14, 2018 •

edited

Loading

mfeckie Aug 1, 2018 •

edited

Loading

makepanic commented Aug 7, 2018 •

edited

Loading