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

Rethinking helpers for forms input #1336

Open
simonihmig opened this issue Feb 7, 2023 · 2 comments
Open

Rethinking helpers for forms input #1336

simonihmig opened this issue Feb 7, 2023 · 2 comments

Comments

@simonihmig
Copy link
Contributor

The main test helper for simulating how users enter data into a form field is fillIn(). Trying to use it to simulate form validations that happen on either input or change revealed that this helper is actually pretty weird IMHO.

I think the guiding principle for test helpers in this addon, and for the overall testing story of Ember in general, is that they replicate what a real user would do, in a real browser.

And it turns out fillIn() does not pass these criteria, as it will mutate input.value (ok), trigger input (also ok) and change, without triggering blur/focusout - which is an interaction that a real user cannot do, at least for a text input!

change is triggered...

When the element loses focus after its value was changed: for elements where the user's interaction is typing rather than selection

Source

In my case I was trying to test things while a user is typing, i.e. they enter text into an input, the input event gets triggered, but (in a real-world scenario) no change event is triggered. But fillIn() does that, giving me false positives in tests, i.e. they pass while the real world behaviour of my component is actually wrong (because it relied on change events which don't happen while typing)

I don't think we can even think of dropping fillIn() or changing its behaviour, given that it would break every app, and would cause a mountain of unnecessary churn, while most tests probably don't care about the subtle differences here. But I would at least like to have additional test helpers that correctly replicate user interactions as they occur in the real world. Off the top of my head something like

  • input() (or type()) that simulates a user typing into a field without explicitly "committing" the change. Which would mean:
    • for text-based input only input is triggered
    • for selects/radios/checkboxes/date pickers etc.: input and change are triggered, as they are implicitly committed (see the MDN doc)
  • enter() (or fillInAndLeave() or whatever): simulate a user entering data (or selecting something) and leaving the field, which would trigger input, change (as fillIn() does today) but also blur and focusout (as again, you cannot have change without blur for text-based inputs)

What do you think?

If there is some general level of agreement, would that be a case for "PRs welcome" or "needs RFC"? 🤔

@jgwhite
Copy link

jgwhite commented May 25, 2023

My 2€…

It looks like testing-library’s type() behaves exactly as @simonihmig suggests — it doesn’t trigger blur, focusout, or change, it just types into the input and leaves the cursor blinking. I wonder whether users of testing-library ever get tripped up by this, expecting change to be called implicitly.

I suspect it would be beneficial for fillIn to more-closely model real user interactions, but agree the churn cost would be high. Perhaps we could introduce the enhanced behavior as an opt-in? Like a feature flag? With an eye to enabling it by default eventually?

import Application from 'acme/app';
import config from 'acme/config/environment';
import * as QUnit from 'qunit';
import { setApplication, enableFeature } from '@ember/test-helpers';
import { setup } from 'qunit-dom';
import { start } from 'ember-qunit';

setApplication(Application.create(config.APP));

setup(QUnit.assert);

enableFeature('fill-in-realistic-mode'); // <---

start();

@HeroicEric
Copy link
Member

Just wanted to add that there is also a beforeinput event that is not possible to test using the current helpers. If we're going to add additional helpers and/or modify the existing ones it would be great if we could also look at including that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants