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

Error when attempting to fillIn/typeIn a disabled form control #741

Merged
merged 1 commit into from
Apr 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions addon-test-support/@ember/test-helpers/dom/-is-focusable.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import isFormControl from './-is-form-control';
import { isDocument } from './-target';
import { isDocument, isContentEditable } from './-target';

const FOCUSABLE_TAGS = ['A'];

type FocusableElement = HTMLAnchorElement;

// eslint-disable-next-line require-jsdoc
function isFocusableElement(
element: HTMLElement | SVGElement | Element
): element is FocusableElement {
function isFocusableElement(element: Element): element is FocusableElement {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you change the types here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

return FOCUSABLE_TAGS.indexOf(element.tagName) > -1;
}

Expand All @@ -24,11 +22,11 @@ export default function isFocusable(
return false;
}

if (
isFormControl(element) ||
(element as HTMLElement).isContentEditable ||
isFocusableElement(element)
) {
if (isFormControl(element)) {
return !element.disabled;
}

if (isContentEditable(element) || isFocusableElement(element)) {
return true;
}

Expand Down
5 changes: 5 additions & 0 deletions addon-test-support/@ember/test-helpers/dom/-target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,8 @@ export function isElement(target: any): target is Element {
export function isDocument(target: any): target is Document {
return target.nodeType === Node.DOCUMENT_NODE;
}

// eslint-disable-next-line require-jsdoc
export function isContentEditable(element: Element): element is HTMLElement {
return 'isContentEditable' in element && (element as HTMLElement).isContentEditable;
}
22 changes: 13 additions & 9 deletions addon-test-support/@ember/test-helpers/dom/fill-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { __focus__ } from './focus';
import settled from '../settled';
import fireEvent from './fire-event';
import { nextTickPromise } from '../-utils';
import Target from './-target';
import Target, { isContentEditable } from './-target';
import { log } from '@ember/test-helpers/dom/-logging';

/**
Expand Down Expand Up @@ -32,25 +32,29 @@ export default function fillIn(target: Target, text: string): Promise<void> {
throw new Error('Must pass an element or selector to `fillIn`.');
}

let element = getElement(target) as any;
let element = getElement(target) as Element | HTMLElement;
if (!element) {
throw new Error(`Element not found when calling \`fillIn('${target}')\`.`);
}
let isControl = isFormControl(element);
if (!isControl && !element.isContentEditable) {
throw new Error('`fillIn` is only usable on form controls or contenteditable elements.');
}

if (typeof text === 'undefined' || text === null) {
throw new Error('Must provide `text` when calling `fillIn`.');
}

__focus__(element);
if (isFormControl(element)) {
if (element.disabled) {
throw new Error(`Can not \`fillIn\` disabled '${target}'.`);
}

__focus__(element);

if (isControl) {
element.value = text;
} else {
} else if (isContentEditable(element)) {
__focus__(element);

element.innerHTML = text;
} else {
throw new Error('`fillIn` is only usable on form controls or contenteditable elements.');
}

fireEvent(element, 'input');
Expand Down
11 changes: 6 additions & 5 deletions addon-test-support/@ember/test-helpers/dom/type-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import settled from '../settled';
import getElement from './-get-element';
import isFormControl, { FormControl } from './-is-form-control';
import { __focus__ } from './focus';
import isFocusable from './-is-focusable';
import { Promise } from 'rsvp';
import fireEvent from './fire-event';
import Target from './-target';
Expand Down Expand Up @@ -58,12 +57,14 @@ export default function typeIn(target: Target, text: string, options: Options =
throw new Error('Must provide `text` when calling `typeIn`.');
}

let { delay = 50 } = options;

if (isFocusable(element)) {
__focus__(element);
if (element.disabled) {
throw new Error(`Can not \`typeIn\` disabled '${target}'.`);
}

__focus__(element);

let { delay = 50 } = options;

return fillOut(element, text, delay)
.then(() => fireEvent(element, 'change'))
.then(settled);
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/dom/fill-in-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,26 @@ module('DOM Helper: fillIn', function (hooks) {
);
});

test('filling in a disabled element', async function (assert) {
element = buildInstrumentedElement('input');
element.dataset.testDisabled = '';
element.setAttribute('disabled', true);

await setupContext(context);

assert.rejects(
fillIn(`[data-test-disabled]`, 'foo'),
new Error("Can not `fillIn` disabled '[data-test-disabled]'."),
'renders user selector'
);

assert.rejects(
fillIn(element, 'foo'),
new Error("Can not `fillIn` disabled '[object HTMLInputElement]'."),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

'renders Element instance'
);
});

test('rejects if selector is not found', async function (assert) {
element = buildInstrumentedElement('div');

Expand Down
25 changes: 21 additions & 4 deletions tests/unit/dom/type-in-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ module('DOM Helper: typeIn', function (hooks) {
document.getElementById('ember-testing').innerHTML = '';
});

test('filling in an input', async function (assert) {
test('typing in an input', async function (assert) {
element = buildInstrumentedElement('input');
await typeIn(element, 'foo');

Expand Down Expand Up @@ -100,7 +100,7 @@ module('DOM Helper: typeIn', function (hooks) {
assert.verifySteps(expectedEventsWithArguments);
});

test('filling in an input with a delay', async function (assert) {
test('typing in an input with a delay', async function (assert) {
element = buildInstrumentedElement('input');
await typeIn(element, 'foo', { delay: 150 });

Expand All @@ -109,7 +109,7 @@ module('DOM Helper: typeIn', function (hooks) {
assert.equal(element.value, 'foo');
});

test('filling in a textarea', async function (assert) {
test('typing in a textarea', async function (assert) {
element = buildInstrumentedElement('textarea');
await typeIn(element, 'foo');

Expand All @@ -118,13 +118,30 @@ module('DOM Helper: typeIn', function (hooks) {
assert.equal(element.value, 'foo');
});

test('filling in a non-fillable element', async function (assert) {
test('typing in not a form control', async function (assert) {
element = buildInstrumentedElement('div');

await setupContext(context);
assert.rejects(typeIn(`#${element.id}`, 'foo'), /`typeIn` is only usable on form controls/);
});

test('typing in a disabled element', async function (assert) {
element = buildInstrumentedElement('input');
element.dataset.testDisabled = '';
element.setAttribute('disabled', '');

await setupContext(context);
assert.rejects(
typeIn(`[data-test-disabled]`, 'foo'),
new Error("Can not `typeIn` disabled '[data-test-disabled]'.")
);

assert.rejects(
typeIn(element, 'foo'),
new Error("Can not `typeIn` disabled '[object HTMLInputElement]'.")
);
});

test('rejects if selector is not found', async function (assert) {
element = buildInstrumentedElement('div');

Expand Down