diff --git a/packages/choice-input/src/ChoiceInputMixin.js b/packages/choice-input/src/ChoiceInputMixin.js index 37a0346a4..b0479a186 100644 --- a/packages/choice-input/src/ChoiceInputMixin.js +++ b/packages/choice-input/src/ChoiceInputMixin.js @@ -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 */ diff --git a/packages/choice-input/test/ChoiceInputMixin.test.js b/packages/choice-input/test/ChoiceInputMixin.test.js index 3d41e2caf..9c53fc66e 100644 --- a/packages/choice-input/test/ChoiceInputMixin.test.js +++ b/packages/choice-input/test/ChoiceInputMixin.test.js @@ -68,10 +68,12 @@ describe('ChoiceInputMixin', () => { let counter = 0; const el = await fixture(html` { + @user-input-changed="${() => { counter += 1; - }} - > + }}" + > + + `); expect(counter).to.equal(0); // Here we try to mimic user interaction by firing browser events @@ -100,8 +102,6 @@ describe('ChoiceInputMixin', () => { const precheckedElementAttr = await fixture(html` `); - el._syncValueUpwards = () => {}; // We need to disable the method for the test to pass - expect(precheckedElementAttr.checked).to.equal(true, 'initially checked via attribute'); }); diff --git a/packages/field/src/FormatMixin.js b/packages/field/src/FormatMixin.js index ddc3853a3..ea33cc36d 100644 --- a/packages/field/src/FormatMixin.js +++ b/packages/field/src/FormatMixin.js @@ -182,11 +182,37 @@ export const FormatMixin = dedupeMixin( } __callParser(value = this.formattedValue) { - let result; - if (typeof value === 'string') { - result = this.parser(value, this.formatOptions); + // A) check if we need to parse at all + + // 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() { diff --git a/packages/field/test/FormatMixin.test.js b/packages/field/test/FormatMixin.test.js index 2e11a89cb..1c7905364 100644 --- a/packages/field/test/FormatMixin.test.js +++ b/packages/field/test/FormatMixin.test.js @@ -212,7 +212,7 @@ describe('FormatMixin', () => { 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 el = await fixture(html` <${elem} .parser="${parserSpy}"> @@ -221,8 +221,25 @@ describe('FormatMixin', () => { `); el.modelValue = 'foo'; expect(parserSpy.callCount).to.equal(1); + // This could happen for instance in a reset el.modelValue = undefined; 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}> + + + `); + // 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 on `user-input-changed` ', async () => {