Error when attempting to fillIn/typeIn a disabled form control

[ro0gr]

fixes: #680
continues: #681

With this PR fillIn and typeIn helpers start to fail on attempt to interact with disabled form controls.

There is a follow up PR, doing a similar thing for readonly(#779). Extracted it to make review easier.

---

my original intention was also to cover click and doubleclick and the rest of interaction helpers with the same logic, but seems they intentionally just skip focus for non-focusable elements:

ember-test-helpers/addon-test-support/@ember/test-helpers/dom/click.ts

Lines 18 to 20 in f053fe0

	if (isFocusable(element)) {
	__focus__(element);
	}

Assuming changing of this behavior, might be considered a breaking change, I've extracted this part as a separate follow-up PR(#778)

[ro0gr]

not sure what does fail on CI, it just says:

The command "yarn test" exited with 139.

w/o any context.

[ro0gr]

hm.. turns out, in click we don't even try to do anything with disabled inputs:

ember-test-helpers/addon-test-support/@ember/test-helpers/dom/click.ts

Lines 82 to 86 in 7d8a9a5

	let isDisabledFormControl = isFormControl(element) && element.disabled;
	if (!isDisabledFormControl) {
	__click__(element, options);
	}

I think it should throw. Will update.

[rwjblue]

I think it should throw. Will update.

👍

[rwjblue]

I think this is good to go once click has been updated to throw (along with tests).

Thanks for picking this up @ro0gr!

[ro0gr]

@rwjblue thanks for the feedback and sorry for letting it hang, going to come back to it this week.

[rwjblue]

No problem, thanks for working on it!

[ro0gr]

Just added an error for click, and made sure all the other interactions fail if disabled element. rebased and squashed.

But a slight doubt appeared on my mind. I suspect it might be considered a breaking change, since we didn't fail on clicking disableds before.

I'm pretty sure there are some tests in the wild, like:

this.doSmth = () => {
  this.called = true
}

await render(hbs`<Something 
  @disabled={{true}}
  @onClick={{this.doSmth}}
/>`

await click('.Something');

assert.equal(called, false)

which I believe is totally valid with existing click implementation, but would fail with this proposed change. From the other hand, not throwing for disableds in {fill,type}-in should not be allowed, even it'd break some tests (imo).

@rwjblue please let me know, what do you think? Should I split the PR into 2, in order to merge {fill,type}-in stuff first, and let the the rest wait for the next major, or is it fine for you to go as is?

[ro0gr]

Should I split the PR into 2, in order to merge {fill,type}-in stuff first, and let the the rest wait for the next major, or is it fine for you to go as is?

extracted everything unrelated to fillIn and typeIn into another PR #778

[Turbo87]

why did you change the types here?

[ro0gr]

I believe this was my attempt to simlify a bit. Both HTMLElement are SVGElement derrive from Element, so it does seem redundant to specify each.
Also, implementation wise it just checks if it's not an A tag!

[Turbo87]

why do we have [object HTMLInputElement] in here?

[ro0gr]

we are passing an element instace, which is not focusable, into throw statement. Should prbably change it to use specifier(selector or Element) passed by user. Will update, thanks!

[ro0gr]

@Turbo87 just improved error messages. See this one, and 1 assert above. Please let me know if it looks good for you.

[rwjblue]

This looks good to me, I think it will need a rebase to fix some conflicts but we should be able to land and get it out in 2.0.0.

[ro0gr]

CI is failing due to some conflicts in ember-data/ember types. I suspect this was caused by this PR #784, which doesn't have travis results reported in the PR for some reason, so we've missed the breakage.

I think it's a good time to upgrade that types and ec-ts at once. Will look into it.

[rwjblue]

I rerolled the yarn.lock in #814 (since floating deps tests worked), mind trying again?