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.

Sign up for GitHub

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

Jump to bottom

Ensure className bindings work correctly when ShadyDOM.noPatch is… #5518

Merged
merged 9 commits into from
Apr 20, 2019
Merged

Ensure className bindings work correctly when ShadyDOM.noPatch is… #5518

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eb2385a
Ensure `className` bindings work correctly when `ShadyDOM.noPatch` is…
Apr 1, 2019
4e1d6a1
Address review feedback via comment.
Apr 2, 2019
439c245
fix lint issues
dfreedm Apr 2, 2019
002ef94
fix closure compiler error
dfreedm Apr 4, 2019
a1c67e4
Update webcomponents dev dependency for testing className fix
dfreedm Apr 9, 2019
b13e656
Fix `className` on browsers without good native accessors
Apr 11, 2019
6adbc23
Fix typo
Apr 11, 2019
3b7c9f8
upgrade dependencies.
Apr 16, 2019
652fea9
Merge branch 'master' into fix-className-bindings-master
dfreedm Apr 19, 2019
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/legacy/polymer.dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ if (window['ShadyDOM'] && window['ShadyDOM']['inUse'] && window['ShadyDOM']['noP
]);

forwardProperties(DomApiNative.prototype, [
'textContent', 'innerHTML'
'textContent', 'innerHTML', 'className'
]);
}

Expand Down
8 changes: 8 additions & 0 deletions lib/mixins/property-effects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,10 @@ function setupCompoundStorage(node, binding) {
storage[target] = literals;
// Configure properties with their literal parts
if (binding.literal && binding.kind == 'property') {
// Note, className needs style scoping so this needs wrapping.
if (target === 'className') {
node = wrap(node);
}
node[target] = binding.literal;
}
}
Expand Down Expand Up @@ -1407,6 +1411,10 @@ export const PropertyEffects = dedupingMixin(superClass => {
// implement a whitelist of tag & property values that should never
// be reset (e.g. <input>.value && <select>.value)
if (value !== node[prop] || typeof value == 'object') {
// Note, className needs style scoping so this needs wrapping.
if (prop === 'className') {
node = wrap(node);
}
node[prop] = value;
}
}
Expand Down
62 changes: 62 additions & 0 deletions test/unit/polymer-dom-nopatch.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,37 @@
</script>
</dom-module>

<dom-module id="x-class-bindings">
<template>
<style>
.a {
border: 2px solid orange;
}

.aa {
border: 12px solid red;
}

.b {
padding: 4px;
}
</style>
<div>
<div id="class" class$="[[clazz]] b">class</div>
<div id="className" class-name="[[clazz]] b">class</div>
</div>
</template>
<script type="module">
import { Polymer } from '../../polymer-legacy.js';
Polymer({
properties: {
clazz: {type: String}
},
is: 'x-class-bindings'
});
</script>
</dom-module>

<test-fixture id="scoped">
<template>
<x-event-scoped></x-event-scoped>
Expand Down Expand Up @@ -493,6 +524,37 @@
assert.equal(el.className, 'foo');
});

test('className', function() {
dom(el).className = 'foo';
assert.isTrue(el.classList.contains('foo'));
});

test('class bindings work and remained scoped', function() {
const el = document.createElement('x-class-bindings');
document.body.appendChild(el);
assert.equal(getComputedStyle(el.$.class)['border-top-width'], '0px');
assert.equal(getComputedStyle(el.$.class)['padding-top'], '4px');
assert.isTrue(el.$.class.classList.contains('style-scope'));
assert.equal(getComputedStyle(el.$.className)['border-top-width'], '0px');
assert.equal(getComputedStyle(el.$.className)['padding-top'], '4px');
assert.isTrue(el.$.className.classList.contains('style-scope'));
el.clazz = 'a';
assert.equal(getComputedStyle(el.$.class)['border-top-width'], '2px');
assert.equal(getComputedStyle(el.$.class)['padding-top'], '4px');
assert.isTrue(el.$.class.classList.contains('style-scope'));
assert.equal(getComputedStyle(el.$.className)['border-top-width'], '2px');
assert.equal(getComputedStyle(el.$.className)['padding-top'], '4px');
assert.isTrue(el.$.className.classList.contains('style-scope'));
el.clazz = 'aa';
assert.equal(getComputedStyle(el.$.class)['border-top-width'], '12px');
assert.equal(getComputedStyle(el.$.class)['padding-top'], '4px');
assert.isTrue(el.$.class.classList.contains('style-scope'));
assert.equal(getComputedStyle(el.$.className)['border-top-width'], '12px');
assert.equal(getComputedStyle(el.$.className)['padding-top'], '4px');
assert.isTrue(el.$.className.classList.contains('style-scope'));
document.body.removeChild(el);
})

});
</script>

Expand Down
58 changes: 57 additions & 1 deletion test/unit/polymer-dom.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,38 @@
is: 'x-focusable-in-shadow'
});
</script>
</dom-module>
</dom-module>

<dom-module id="x-class-bindings">
<template>
<style>
.a {
border: 2px solid orange;
}

.aa {
border: 12px solid red;
}

.b {
padding: 4px;
}
</style>
<div>
<div id="class" class$="[[clazz]] b">class</div>
<div id="className" class-name="[[clazz]] b">class</div>
</div>
</template>
<script type="module">
import { Polymer } from '../../polymer-legacy.js';
Polymer({
properties: {
clazz: {type: String}
},
is: 'x-class-bindings'
});
</script>
</dom-module>

<test-fixture id="scoped">
<template>
Expand Down Expand Up @@ -480,6 +511,31 @@
assert.equal(el.className, 'foo');
});

test('className', function() {
dom(el).className = 'foo';
assert.isTrue(el.classList.contains('foo'));
});

test('class bindings', function() {
const el = document.createElement('x-class-bindings');
document.body.appendChild(el);
assert.equal(getComputedStyle(el.$.class)['border-top-width'], '0px');
assert.equal(getComputedStyle(el.$.class)['padding-top'], '4px');
assert.equal(getComputedStyle(el.$.className)['border-top-width'], '0px');
assert.equal(getComputedStyle(el.$.className)['padding-top'], '4px');
el.clazz = 'a';
assert.equal(getComputedStyle(el.$.class)['border-top-width'], '2px');
assert.equal(getComputedStyle(el.$.class)['padding-top'], '4px');
assert.equal(getComputedStyle(el.$.className)['border-top-width'], '2px');
assert.equal(getComputedStyle(el.$.className)['padding-top'], '4px');
el.clazz = 'aa';
assert.equal(getComputedStyle(el.$.class)['border-top-width'], '12px');
assert.equal(getComputedStyle(el.$.class)['padding-top'], '4px');
assert.equal(getComputedStyle(el.$.className)['border-top-width'], '12px');
assert.equal(getComputedStyle(el.$.className)['padding-top'], '4px');
document.body.removeChild(el);
})

});
</script>

Expand Down