Skip to content

Commit

Permalink
feature: Remove support for setting nonstandard attributes as props (v…
Browse files Browse the repository at this point in the history
…ideojs#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
  • Loading branch information
roman-bc-dev authored and edirub committed Jun 8, 2023
1 parent 041d136 commit 38145b7
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
3 changes: 1 addition & 2 deletions src/js/slider/slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 1 addition & 10 deletions src/js/utils/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions test/unit/utils/dom.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,26 @@ 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);

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');
Expand Down

0 comments on commit 38145b7

Please sign in to comment.