Skip to content

Commit

Permalink
fix: use flattened nodes on helper slot
Browse files Browse the repository at this point in the history
A 3rd custom element using `vaadin-text-field` and defining a slot
area for the `vaadin-text-field`'s helper slot was making
`vaadin-text-field` see the slot element as a slotted contend and
setting `has-helper="slotted"` to itself.

This change uses flattened nodes to check the presence of slotted
elements on `vaadin-text-field` `helper` slot.

In this change
- use `FlattenedNodesObserver` instead of `slotchange` event
- use `FlattenedNodesObserver.getFlattenedNodes` to get slot children
- apply changes to tests
- remove checks for unsupported types
  • Loading branch information
DiegoCardoso committed Jun 1, 2020
1 parent 09fc63a commit 4dd463e
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 38 deletions.
29 changes: 21 additions & 8 deletions src/vaadin-text-field-mixin.html
Expand Up @@ -6,6 +6,7 @@

<link rel="import" href="../../polymer/lib/utils/async.html">
<link rel="import" href="../../polymer/lib/utils/debounce.html">
<link rel="import" href="../../polymer/lib/utils/flattened-nodes-observer.html">

<dom-module id="vaadin-text-field-shared-styles">
<template>
Expand Down Expand Up @@ -499,25 +500,25 @@

/** @private */
_labelChanged(label) {
this._setOrToggleAttribute('has-label', label === 0 || !!label, this);
this._setOrToggleAttribute('has-label', !!label, this);
}

/** @private */
_helperTextChanged(helperText) {
this._setOrToggleAttribute('has-helper', helperText === 0 || !!helperText, this);
this._setOrToggleAttribute('has-helper', !!helperText, this);
}

/** @private */
_errorMessageChanged(errorMessage) {
this._setOrToggleAttribute('has-error-message', errorMessage === 0 || !!errorMessage, this);
this._setOrToggleAttribute('has-error-message', !!errorMessage, this);
}

/** @private */
_onHelperSlotChange() {
const slottedNodes = this.shadowRoot.querySelector(`[name="helper"]`).assignedNodes();
_helperSlotObserverCallback(info) {
const slottedNodes = Polymer.FlattenedNodesObserver.getFlattenedNodes(info.target);
// Only has slotted helper if not a text node
// Text nodes are added by the helperText prop and not the helper slot
// The filter is added due to shady DOM triggering this slotchange event on helperText prop change
// The filter is added due to shady DOM triggering this callback on helperText prop change
this._hasSlottedHelper = slottedNodes.filter(node => node.nodeType !== 3).length;

if (this._hasSlottedHelper) {
Expand Down Expand Up @@ -680,8 +681,8 @@
this.shadowRoot.querySelector('[name="input"], [name="textarea"]')
.addEventListener('slotchange', this._onSlotChange.bind(this));

this._onHelperSlotChange();
this.shadowRoot.querySelector('[name="helper"]').addEventListener('slotchange', this._onHelperSlotChange.bind(this));
this._helperObserver = new Polymer.FlattenedNodesObserver(
this.shadowRoot.querySelector('[name="helper"]'), this._helperSlotObserverCallback.bind(this));

if (!(window.ShadyCSS && window.ShadyCSS.nativeCss)) {
this.updateStyles();
Expand All @@ -706,6 +707,18 @@
});
}

/** @protected */
connectedCallback() {
super.connectedCallback();
this._helperObserver && this._helperObserver.connect();
}

/** @protected */
disconnectedCallback() {
super.disconnectedCallback();
this.__helperObserver && this.__helperObserver.disconnect();
}

/**
* Returns true if `value` is valid.
* `<iron-form>` uses this to check the validity for all its elements.
Expand Down
21 changes: 10 additions & 11 deletions test/accessibility.html
Expand Up @@ -36,7 +36,7 @@
<test-fixture id="vaadin-text-field-error-fixture-with-slotted-input">
<template>
<vaadin-text-field required error-message="ERR">
<input slot="input">
<input slot="input">
</vaadin-text-field>
</template>
</test-fixture>
Expand All @@ -50,7 +50,7 @@
<test-fixture id="vaadin-text-field-invalid-fixture-with-slotted-input">
<template>
<vaadin-text-field invalid error-message="ERR">
<input slot="input">
<input slot="input">
</vaadin-text-field>
</template>
</test-fixture>
Expand All @@ -65,10 +65,10 @@
<test-fixture id="vaadin-text-field-multiple-fields-with-slotted-input">
<template>
<vaadin-text-field>
<input slot="input">
<input slot="input">
</vaadin-text-field>
<vaadin-text-field>
<input slot="input">
<input slot="input">
</vaadin-text-field>
</template>
</test-fixture>
Expand All @@ -80,7 +80,7 @@
</vaadin-text-field>
</template>
</test-fixture>

<test-fixture id="vaadin-text-area-default">
<template>
<vaadin-text-area></vaadin-text-area>
Expand All @@ -104,7 +104,7 @@
<test-fixture id="vaadin-text-area-error-fixture-with-slotted-input">
<template>
<vaadin-text-area required error-message="ERR">
<textarea slot="textarea"></textarea>
<textarea slot="textarea"></textarea>
</vaadin-text-area>
</template>
</test-fixture>
Expand All @@ -118,7 +118,7 @@
<test-fixture id="vaadin-text-area-invalid-fixture-with-slotted-input">
<template>
<vaadin-text-area invalid error-message="ERR">
<textarea slot="textarea"></textarea>
<textarea slot="textarea"></textarea>
</vaadin-text-area>
</template>
</test-fixture>
Expand All @@ -140,15 +140,15 @@
</vaadin-text-area>
</template>
</test-fixture>

<test-fixture id="vaadin-text-area-with-slotted-helper">
<template>
<vaadin-text-area required error-message="ERR">
<div slot="helper">foo</div>
</vaadin-text-area>
</template>
</test-fixture>

<script>
['', 'with slotted input'].forEach(condition => {
let fixtureName = '';
Expand Down Expand Up @@ -338,9 +338,8 @@
const input = tf.inputElement;
const err = tf.root.querySelector('[part=error-message]');
const helperText = tf.root.querySelector('[part=helper-text]');
const evt = new CustomEvent('slotchange');
tf.validate();
tf.shadowRoot.querySelector('[name="helper"]').dispatchEvent(evt);
tf._helperObserver.flush();
expect(input.getAttribute('aria-describedby')).to.equal(`${helperText.id} ${err.id}`);
});

Expand Down
79 changes: 60 additions & 19 deletions test/text-field.html
Expand Up @@ -35,7 +35,7 @@
</vaadin-text-field>
</template>
</test-fixture>

<test-fixture id="default-with-slotted-helper">
<template>
<vaadin-text-field>
Expand All @@ -44,6 +44,36 @@
</template>
</test-fixture>

<dom-module id="x-element">
<template>
<vaadin-text-field id="textField" helper-text="[[helperText]]">
<slot slot="helper" name="helper"></slot>
</vaadin-text-field>
</template>
</dom-module>

<script>
addEventListener('WebComponentsReady', () => {
class XElement extends Polymer.Element {
static get is() {
return 'x-element';
}
static get properties() {
return {
helperText: String
};
}
}
window.customElements.define(XElement.is, XElement);
});
</script>

<test-fixture id="custom-element-with-slotted-helper">
<template>
<x-element></x-element>
</template>
</test-fixture>

<script>

describe('Wrapped', () => {
Expand Down Expand Up @@ -273,12 +303,6 @@
});

describe(`binding ${condition}`, function() {
function dispatchHelperSlotChange() {
// Workaround not to use timeouts
const evt = new CustomEvent('slotchange');
textField.shadowRoot.querySelector('[name="helper"]').dispatchEvent(evt);
}

it('default value should be empty string', function() {
expect(textField.value).to.be.equal('');
});
Expand Down Expand Up @@ -336,11 +360,6 @@
expect(textField.hasAttribute('has-label')).to.be.false;
});

it('setting number label updates has-label attribute', function() {
textField.label = 0;
expect(textField.hasAttribute('has-label')).to.be.true;
});

it('setting helper updates has-helper attribute', function() {
textField.helperText = 'foo';
expect(textField.hasAttribute('has-helper')).to.be.true;
Expand All @@ -356,14 +375,9 @@
expect(textField.hasAttribute('has-helper')).to.be.false;
});

it('setting number helper updates has-helper attribute', function() {
textField.helperText = 0;
expect(textField.hasAttribute('has-helper')).to.be.true;
});

it('text-field with slotted helper updates has-helper attribute', function() {
const textFieldSlottedHelper = fixture('default-with-slotted-helper');
dispatchHelperSlotChange();
textFieldSlottedHelper._helperObserver.flush();
expect(textFieldSlottedHelper.hasAttribute('has-helper')).to.be.true;
});

Expand All @@ -372,7 +386,8 @@
helper.setAttribute('slot', 'helper');
helper.textContent = 'foo';
textField.appendChild(helper);
dispatchHelperSlotChange();
textField._helperObserver.flush();

expect(textField.hasAttribute('has-helper')).to.be.true;
});

Expand All @@ -387,6 +402,32 @@
}, 1);
});

it('should not add "has-helper" with a slotted "slot" element', function() {
const xElement = fixture('custom-element-with-slotted-helper');
const textField = xElement.$.textField;
expect(textField.hasAttribute('has-helper')).to.be.false;
});

it('should add "has-helper" when slotted "slot" and custom element sets string property', function() {
const xElement = fixture('custom-element-with-slotted-helper');
const textField = xElement.$.textField;
xElement.helperText = 'helper text';
expect(textField.hasAttribute('has-helper')).to.be.true;
});

it('should add "has-helper=slotted" when slotted "slot" and custom element sets content to slot', function() {
const xElement = fixture('custom-element-with-slotted-helper');
const textField = xElement.$.textField;
const span = document.createElement('span');
span.textContent = 'helper text';
span.setAttribute('slot', 'helper');
xElement.appendChild(span);
textField._helperObserver.flush();

expect(textField.hasAttribute('has-helper')).to.be.true;
expect(textField.getAttribute('has-helper')).to.be.equal('slotted');
});

it('setting errorMessage updates has-error-message attribute', function() {
textField.errorMessage = 'foo';
expect(textField.hasAttribute('has-error-message')).to.be.true;
Expand Down

0 comments on commit 4dd463e

Please sign in to comment.