fix(field): don't return Unparseable on empty strings

This commit is contained in:
Thijs Louisse 2019-07-01 17:23:26 +02:00
parent caedb05372
commit 6a85dbcef7
4 changed files with 55 additions and 11 deletions

View file

@ -212,6 +212,13 @@ export const ChoiceInputMixin = superclass =>
}; };
} }
/**
* @override
* Overridden from FormatMixin, since a different modelValue is used for choice inputs.
* Synchronization from user input is already arranged in this Mixin.
*/
_syncValueUpwards() {}
/** /**
* @deprecated use .checked * @deprecated use .checked
*/ */

View file

@ -12,8 +12,6 @@ describe('ChoiceInputMixin', () => {
if (super.connectedCallback) super.connectedCallback(); if (super.connectedCallback) super.connectedCallback();
this.type = 'checkbox'; // could also be 'radio', should be tested in integration test this.type = 'checkbox'; // could also be 'radio', should be tested in integration test
} }
_syncValueUpwards() {} // We need to disable the method for the test to pass
} }
customElements.define('choice-input', ChoiceInput); customElements.define('choice-input', ChoiceInput);
}); });
@ -82,7 +80,6 @@ describe('ChoiceInputMixin', () => {
const nativeInput = el.inputElement; const nativeInput = el.inputElement;
nativeInput.dispatchEvent(new CustomEvent('input', { bubbles: true })); // fired by (at least) Chrome nativeInput.dispatchEvent(new CustomEvent('input', { bubbles: true })); // fired by (at least) Chrome
expect(counter).to.equal(0); expect(counter).to.equal(0);
el._syncValueUpwards = () => {}; // We need to disable the method for the test to pass
nativeInput.dispatchEvent(new CustomEvent('change', { bubbles: true })); nativeInput.dispatchEvent(new CustomEvent('change', { bubbles: true }));
expect(counter).to.equal(1); expect(counter).to.equal(1);
}); });
@ -105,8 +102,6 @@ describe('ChoiceInputMixin', () => {
const precheckedElementAttr = await fixture(html` const precheckedElementAttr = await fixture(html`
<choice-input .checked=${true}></choice-input> <choice-input .checked=${true}></choice-input>
`); `);
el._syncValueUpwards = () => {}; // We need to disable the method for the test to pass
expect(precheckedElementAttr.checked).to.equal(true, 'initially checked via attribute'); expect(precheckedElementAttr.checked).to.equal(true, 'initially checked via attribute');
}); });
@ -122,7 +117,6 @@ describe('ChoiceInputMixin', () => {
it('can be checked and unchecked via user interaction', async () => { it('can be checked and unchecked via user interaction', async () => {
const el = await fixture(`<choice-input></choice-input>`); const el = await fixture(`<choice-input></choice-input>`);
el._syncValueUpwards = () => {}; // We need to disable the method for the test to pass
el.inputElement.click(); el.inputElement.click();
expect(el.checked).to.be.true; expect(el.checked).to.be.true;
el.inputElement.click(); el.inputElement.click();

View file

@ -171,11 +171,37 @@ export const FormatMixin = dedupeMixin(
} }
__callParser(value = this.formattedValue) { __callParser(value = this.formattedValue) {
let result; // A) check if we need to parse at all
if (typeof value === 'string') {
result = this.parser(value, this.formatOptions); // A.1) The end user had no intention to parse
if (value === '') {
// Ideally, modelValue should be undefined for empty strings.
// For backwards compatibility we return an empty string:
// - it triggers validation for required validators (see ValidateMixin.validate())
// - it can be expected by 3rd parties (for instance unit tests)
// TODO: In a breaking refactor of the Validation System, this behaviot can be corrected.
return '';
} }
return typeof result !== 'undefined' ? result : new Unparseable(value);
// A.2) Handle edge cases We might have no view value yet, for instance because
// inputElement.value was not available yet
if (typeof value !== 'string') {
// This means there is nothing to find inside the view that can be of
// interest to the Application Developer or needed to store for future form state
// retrieval.
return undefined;
}
// B) parse the view value
// - if result:
// return the successfully parsed viewValue
// - if no result:
// Apparently, the parser was not able to produce a satisfactory output for the desired
// modelValue type, based on the current viewValue. Unparseable allows to restore all
// states (for instance from a lost user session), since it saves the current viewValue.
const result = this.parser(value, this.formatOptions);
return result !== undefined ? result : new Unparseable(value);
} }
__callFormatter() { __callFormatter() {

View file

@ -193,7 +193,7 @@ describe('FormatMixin', () => {
expect(formatterSpy.args[0][1].decimalSeparator).to.equal('-'); expect(formatterSpy.args[0][1].decimalSeparator).to.equal('-');
}); });
it('will only call the parser for string values', async () => { it('will only call the parser for defined values', async () => {
const parserSpy = sinon.spy(); const parserSpy = sinon.spy();
const el = await fixture(html` const el = await fixture(html`
<${elem} .parser="${parserSpy}"> <${elem} .parser="${parserSpy}">
@ -202,8 +202,25 @@ describe('FormatMixin', () => {
`); `);
el.modelValue = 'foo'; el.modelValue = 'foo';
expect(parserSpy.callCount).to.equal(1); expect(parserSpy.callCount).to.equal(1);
// This could happen for instance in a reset
el.modelValue = undefined; el.modelValue = undefined;
expect(parserSpy.callCount).to.equal(1); expect(parserSpy.callCount).to.equal(1);
// This could happen when the user erases the input value
mimicUserInput(el, '');
expect(parserSpy.callCount).to.equal(1);
});
it('will not return Unparseable when empty strings are inputted', async () => {
const el = await fixture(html`
<${elem}>
<input slot="input" value="string">
</${elem}>
`);
// This could happen when the user erases the input value
mimicUserInput(el, '');
// For backwards compatibility, we keep the modelValue an empty string here.
// Undefined would be more appropriate 'conceptually', however
expect(el.modelValue).to.equal('');
}); });
it('will only call the formatter for valid values', async () => { it('will only call the formatter for valid values', async () => {