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

fix(forms/*): allow all form control value checks to work with virtual nodes #2343

Merged
merged 2 commits into from
Jul 7, 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
2 changes: 1 addition & 1 deletion lib/commons/forms/is-aria-combobox.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import getExplicitRole from '../aria/get-explicit-role';
* Determines if an element is an aria combobox element
* @method isAriaCombobox
* @memberof axe.commons.forms
* @param {Element} node Node to determine if aria combobox
* @param {VirtualNode|Element} node Node to determine if aria combobox
* @returns {Bool}
*/
function isAriaCombobox(node) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commons/forms/is-aria-listbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import getExplicitRole from '../aria/get-explicit-role';
* Determines if an element is an aria listbox element
* @method isAriaListbox
* @memberof axe.commons.forms
* @param {Element} node Node to determine if aria listbox
* @param {VirtualNode|Element} node Node to determine if aria listbox
* @returns {Bool}
*/
function isAriaListbox(node) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commons/forms/is-aria-range.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const rangeRoles = ['progressbar', 'scrollbar', 'slider', 'spinbutton'];
* Determines if an element is an aria range element
* @method isAriaRange
* @memberof axe.commons.forms
* @param {Element} node Node to determine if aria range
* @param {VirtualNode|Element} node Node to determine if aria range
* @returns {Bool}
*/
function isAriaRange(node) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commons/forms/is-aria-textbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import getExplicitRole from '../aria/get-explicit-role';
* Determines if an element is an aria textbox element
* @method isAriaTextbox
* @memberof axe.commons.forms
* @param {Element} node Node to determine if aria textbox
* @param {VirtualNode|Element} node Node to determine if aria textbox
* @returns {Bool}
*/
function isAriaTextbox(node) {
Expand Down
10 changes: 7 additions & 3 deletions lib/commons/forms/is-native-select.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { getNodeFromTree } from '../../core/utils';
import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node';

/**
* Determines if an element is a native select element
* @method isNativeSelect
* @memberof axe.commons.forms
* @param {Element} node Node to determine if select
* @param {VirtualNode|Element} node Node to determine if select
* @returns {Bool}
*/
function isNativeSelect(node) {
const nodeName = node.nodeName.toUpperCase();
return nodeName === 'SELECT';
node = node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node);
const nodeName = node.props.nodeName;
return nodeName === 'select';
}

export default isNativeSelect;
13 changes: 9 additions & 4 deletions lib/commons/forms/is-native-textbox.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { getNodeFromTree } from '../../core/utils';
import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node';

const nonTextInputTypes = [
'button',
'checkbox',
Expand All @@ -15,14 +18,16 @@ const nonTextInputTypes = [
* Determines if an element is a native textbox element
* @method isNativeTextbox
* @memberof axe.commons.forms
* @param {Element} node Node to determine if textbox
* @param {VirtualNode|Element} node Node to determine if textbox
* @returns {Bool}
*/
function isNativeTextbox(node) {
const nodeName = node.nodeName.toUpperCase();
node = node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node);
const nodeName = node.props.nodeName;
return (
nodeName === 'TEXTAREA' ||
(nodeName === 'INPUT' && !nonTextInputTypes.includes(node.type))
nodeName === 'textarea' ||
(nodeName === 'input' &&
!nonTextInputTypes.includes((node.attr('type') || '').toLowerCase()))
Copy link
Contributor

Choose a reason for hiding this comment

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

May not be necessary for this PR, but I think we should consider adding virtualNode.props.type by default, in the same way that we set nodeType. That way we don't have to do this type casting, and it'd give us an easier way to deal with inputs defaulting to type=text.

);
}

Expand Down
7 changes: 5 additions & 2 deletions test/commons/forms/is-native-select.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
describe('forms.isNativeSelect', function() {
'use strict';
var isNativeSelect = axe.commons.forms.isNativeSelect;
var queryFixture = axe.testUtils.queryFixture;

it('returns true for a select element', function() {
var node = document.createElement('select');
var node = queryFixture('<select id="target"></select>');
assert.isTrue(isNativeSelect(node));
});

it('returns false for non-select elements', function() {
var nonSelectElements = ['a', 'h1', 'div', 'span', 'main'];
nonSelectElements.forEach(function(nodeName) {
var node = document.createElement(nodeName);
var node = queryFixture(
'<' + nodeName + ' id="target"></' + nodeName + '>'
);
assert.isFalse(
isNativeSelect(node),
'<' + nodeName + '> is not a native select element'
Expand Down
28 changes: 14 additions & 14 deletions test/commons/forms/is-native-textbox.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
describe('forms.isNativeTextbox', function() {
'use strict';
var isNativeTextbox = axe.commons.forms.isNativeTextbox;
var queryFixture = axe.testUtils.queryFixture;

it('returns true for a text inputs', function() {
var textInputs = [
Expand All @@ -19,8 +20,7 @@ describe('forms.isNativeTextbox', function() {
'week'
];
textInputs.forEach(function(type) {
var node = document.createElement('input');
node.setAttribute('type', type);
var node = queryFixture('<input id="target" type="' + type + '"/>');
assert.isTrue(
isNativeTextbox(node),
'<input type="' + type + '"> is a native text input'
Expand All @@ -29,7 +29,7 @@ describe('forms.isNativeTextbox', function() {
});

it('returns true for a textarea element', function() {
var node = document.createElement('textarea');
var node = queryFixture('<textarea id="target"/>');
assert.isTrue(isNativeTextbox(node));
});

Expand All @@ -47,22 +47,22 @@ describe('forms.isNativeTextbox', function() {
'color'
];
nonTextInputs.forEach(function(type) {
var node = document.createElement('input');
node.setAttribute('type', type);
var node = queryFixture('<input id="target" type="' + type + '"/>');

// IE doesn't support color inputs
if (node.type !== 'text') {
assert.isFalse(
isNativeTextbox(node),
'<input type="' + type + '"> is not a native text input'
);
}
assert.isFalse(
isNativeTextbox(node),
'<input type="' + type + '"> is not a native text input'
);
});
});

it('return false for aria textbox elements', function() {
var node = document.createElement('div');
node.setAttribute('role', 'textbox');
var node = queryFixture('<div id="target" role="textbox"></div>');
assert.isFalse(isNativeTextbox(node));
});

it('should ignore type case', function() {
var node = queryFixture('<input id="target" type="TEXT"/>');
assert.isTrue(isNativeTextbox(node));
});
});
3 changes: 3 additions & 0 deletions test/commons/text/form-control-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ describe('text.formControlValue', function() {

it('returns the value of DOM nodes', function() {
fixture.innerHTML = '<input value="foo">';
axe.utils.getFlattenedTree(fixture);
assert.equal(nativeTextboxValue(fixture.querySelector('input')), 'foo');
});

Expand All @@ -127,6 +128,7 @@ describe('text.formControlValue', function() {
var target = document.createElement(nodeName);
target.value = 'foo'; // That shouldn't do anything
fixture.appendChild(target);
axe.utils.getFlattenedTree(fixture);
assert.equal(nativeTextboxValue(target), '');
}
);
Expand Down Expand Up @@ -198,6 +200,7 @@ describe('text.formControlValue', function() {
var target = document.createElement(nodeName);
target.value = 'foo'; // That shouldn't do anything
fixture.appendChild(target);
axe.utils.getFlattenedTree(fixture);
assert.equal(nativeSelectValue(target), '');
}
);
Expand Down