From 38145b7b3d7993c290d4da2698a6e9c0d546a0fc Mon Sep 17 00:00:00 2001 From: Roman Pougatchev <83311511+roman-bc-dev@users.noreply.github.com> Date: Thu, 28 Jul 2022 16:44:53 -0400 Subject: [PATCH] feature: Remove support for setting nonstandard attributes as props (#7857) * remove statement that handles attributes in props argument * clean up function * add unit test * add unit test * remove duplicate set attr * revert function cleanup --- src/js/slider/slider.js | 3 +-- src/js/utils/dom.js | 11 +---------- test/unit/utils/dom.test.js | 13 +++++++++++++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/js/slider/slider.js b/src/js/slider/slider.js index e5e0daa1e6..5644a0a1f2 100644 --- a/src/js/slider/slider.js +++ b/src/js/slider/slider.js @@ -133,8 +133,7 @@ class Slider extends Component { 'role': 'slider', 'aria-valuenow': 0, 'aria-valuemin': 0, - 'aria-valuemax': 100, - 'tabIndex': 0 + 'aria-valuemax': 100 }, attributes); return super.createEl(type, props, attributes); diff --git a/src/js/utils/dom.js b/src/js/utils/dom.js index 191e6ca296..c6321ee0d2 100644 --- a/src/js/utils/dom.js +++ b/src/js/utils/dom.js @@ -153,18 +153,9 @@ export function createEl(tagName = 'div', properties = {}, attributes = {}, cont Object.getOwnPropertyNames(properties).forEach(function(propName) { const val = properties[propName]; - // See #2176 - // We originally were accepting both properties and attributes in the - // same object, but that doesn't work so well. - if (propName.indexOf('aria-') !== -1 || propName === 'role' || propName === 'type') { - log.warn('Setting attributes in the second argument of createEl()\n' + - 'has been deprecated. Use the third argument instead.\n' + - `createEl(type, properties, attributes). Attempting to set ${propName} to ${val}.`); - el.setAttribute(propName, val); - // Handle textContent since it's not supported everywhere and we have a // method for it. - } else if (propName === 'textContent') { + if (propName === 'textContent') { textContent(el, val); } else if (el[propName] !== val || propName === 'tabIndex') { el[propName] = val; diff --git a/test/unit/utils/dom.test.js b/test/unit/utils/dom.test.js index 9b0e6a2e67..85d58dfe67 100644 --- a/test/unit/utils/dom.test.js +++ b/test/unit/utils/dom.test.js @@ -30,6 +30,12 @@ QUnit.test('should create an element, supporting textContent', function(assert) } }); +QUnit.test('should create an element with tabIndex prop', function(assert) { + const span = Dom.createEl('span', {tabIndex: '5'}); + + assert.strictEqual(span.tabIndex, 5); +}); + QUnit.test('should create an element with content', function(assert) { const span = Dom.createEl('span'); const div = Dom.createEl('div', undefined, undefined, span); @@ -37,6 +43,13 @@ QUnit.test('should create an element with content', function(assert) { assert.strictEqual(div.firstChild, span); }); +QUnit.test('should be able to set standard props as attributes, and vice versa, on a created element', function(assert) { + const span = Dom.createEl('span', {className: 'bar'}, {id: 'foo'}); + + assert.strictEqual(span.getAttribute('class'), 'bar'); + assert.strictEqual(span.id, 'foo'); +}); + QUnit.test('should insert an element first in another', function(assert) { const el1 = document.createElement('div'); const el2 = document.createElement('div');