typeIn helper forces maximum length check

[mfeckie]

When working with date / time inputs, it's sometimes desired behaviour to allow a user to keep typing beyond the maxlength attribute, for example wrapping a time or date input.

When upgrading to test helpers recently we've seen a lot of failing tests because a new check has been added to the typeIn helper. Whilst it seem well-intentioned, not providing a way to opt out of this behaviour adds a limitation that isn't necessarily what is desired.

Could I add a flag to opt out of maxlength checking?

[ro0gr]

I'm surprised to hear about maxlength in conjunction with type="time" or type="date". Does maxlength have some meaning for this kind of inputs?

I've just tried:

<input type="time" maxlength="1" />
<input type="date" maxlength="1" />

and seems neither of date and time respect the maxlength constraint at all in both FF and Chrome 🤔

[mfeckie]

This is relating to using a text input for time / date, vs the native type= version (because none of the browser native versions are styleable in any meaningful way)

[ro0gr]

Could I add a flag to opt out of maxlength checking?

I think opting out the maxlength only is not enough. We should probably avoid mutating the element.value, and avoid the input event being triggered. See

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

Lines 114 to 123 in d1a05f4

	if (isFormControl(element)) {
	const newValue = element.value + character;
	guardForMaxlength(element, newValue, 'typeIn');
	element.value = newValue;
	} else {
	const newValue = element.innerHTML + character;
	element.innerHTML = newValue;
	}
	fireEvent(element, 'input');

Othewise, if only bypass the guard, test-helpers would do something that a regular user can not do.

[ro0gr]

just wondering if it's reasonable to test this "type-in despite maxlength" use case via triggerKeyEvents instead?

[mfeckie]

I actually contributed the original typeIn helper, so it's kinda surprising to have the behaviour change so significantly.

It seems like a lot of additional things have been added that detract from the original use case.

I'm not sure what the way forward is, but we won't be able to update our project to the 5.x version until we come up with something.

[ro0gr]

I participated in adding a hard error for the fillIn and typeIn, and I'm sorry to hear it causes issues for you! Let me help to figure it out please.

In general, I think, bypassing the check conditionally is a wrong path to go, cause it complicates the typeIn implementation by adding 2 different strategies for a single operation. It seems better to me to find a single solution, which just works.

For instance, instead of a hard error, typeIn could just avoid mutating the value and sending the input event, and keep emitting the rest of keyboard events. At the same type fillIn could keep erroring hardly, cause it is a higher level operation, with a lower level of control.

From the other hand, it might be reasonable to just recommend using triggerKeyEvent to test components which ignore the input event, and rely on keyboard events instead. Though, in this case, I don't understand what can be a role of the maxlength in the markup; so the current typeIn behaviour may be correct.

At this point, w/o an example of your use case, it's hard reason about of what's wrong, and what should be fixed. Specifically, I think it can be helpful to know how/if does your component rely on the input and keyup events.

[mfeckie]

My example as mentioned above is an input that is used for date / time input. It's a plain <input type='text'. It has maxlength="2". For a time, it has three inputs, one for HH, one for MM and on for AM/PM.

If a user types 12 into HH then we can jump to the MM field. If the user types invalid characters (like 9, then 1), then we can keep accepting the input, but shifting, so 91, pops off the 9 and leaves the 1.

So it's 'valid' to type more characters than are allowed by maxlength because it wraps.

I don't want to use triggerKeyEvent because as you see in the implementation of typeIn there's many things to send, keyDown, keyUp etc.

[ro0gr]

Thank you for the explanation! Yes, I can see how a manual mix of the typeIn( and triggerKeyEvent(s can become cumbersome for your use case.

In my opinion, we should relax the typeIn( to not throw, and just stop directly mutating the value and emitting the input event in case of maxlength constraint. Just in case, it still makes sense to preserve themaxlength error for the fillIn(.

@rwjblue what do you think?

[rwjblue]

Hmm, ya that seems reasonable @ro0gr