fix(field): format conditionally on user input only

This commit is contained in:
Thijs Louisse 2019-06-12 16:41:47 +02:00
parent 187d50b6bc
commit af8046cf53
3 changed files with 82 additions and 34 deletions

View file

@ -6,7 +6,16 @@ import { ObserverMixin } from '@lion/core/src/ObserverMixin.js';
import { Unparseable } from '@lion/validate';
/**
* @polymerMixin
* @desc Designed to be applied on top of a LionField
*
* FormatMixin supports these two main flows:
* 1) Application Developer sets `.modelValue`:
* Flow: `.modelValue` -> `.formattedValue` -> `.inputElement.value`
* -> `.serializedValue`
* 2) End user interacts with field:
* Flow: `@user-input-changed` -> `.modelValue` -> `.formattedValue` - (debounce till reflect condition (formatOn) is met) -> `.inputElement.value`
* -> `.serializedValue`
*
* @mixinFunction
*/
export const FormatMixin = dedupeMixin(
@ -26,7 +35,8 @@ export const FormatMixin = dedupeMixin(
*
* Examples:
* - For a date input: a String '20/01/1999' will be converted to new Date('1999/01/20')
* - For a number input: a formatted String '1.234,56' will be converted to a Number: 1234.56
* - For a number input: a formatted String '1.234,56' will be converted to a Number:
* 1234.56
*/
modelValue: {
type: Object,
@ -48,13 +58,13 @@ export const FormatMixin = dedupeMixin(
/**
* The serialized version of the model value.
* This value exists for maximal compatibility with the platform API.
* The serialized value can be an interface in context where data binding is not supported
* and a serialized string needs to be set.
* The serialized value can be an interface in context where data binding is not
* supported and a serialized string needs to be set.
*
* Examples:
* - For a date input, this would be the iso format of a date, e.g. '1999-01-20'.
* - For a number input this would be the String representation of a float ('1234.56' instead
* of 1234.56)
* - For a number input this would be the String representation of a float ('1234.56'
* instead of 1234.56)
*
* When no parser is available, the value is usually the same as the formattedValue
* (being inputElement.value)
@ -139,15 +149,16 @@ export const FormatMixin = dedupeMixin(
}
/**
* Responsible for storing all representations(modelValue, serializedValue, formattedValue and
* value) of the input value.
* Prevents infinite loops, so all value observers can be treated like they will only be called
* once, without indirectly calling other observers.
* (in fact, some are called twice, but the __preventRecursiveTrigger lock prevents the second
* call from having effect).
* Responsible for storing all representations(modelValue, serializedValue, formattedValue
* and value) of the input value.
* Prevents infinite loops, so all value observers can be treated like they will only be
* called once, without indirectly calling other observers.
* (in fact, some are called twice, but the __preventRecursiveTrigger lock prevents the
* second call from having effect).
*
* @param {string} source - the type of value that triggered this method. It should not be set
* again, so that its observer won't be triggered. Can be: 'model'|'formatted'|'serialized'.
* @param {string} source - the type of value that triggered this method. It should not be
* set again, so that its observer won't be triggered. Can be:
* 'model'|'formatted'|'serialized'.
*/
_calculateValues({ source } = {}) {
if (this.__preventRecursiveTrigger) return; // prevent infinite loops
@ -182,7 +193,19 @@ export const FormatMixin = dedupeMixin(
if (this.modelValue instanceof Unparseable) {
return this.modelValue.viewValue;
}
if (this.errorState) {
// - Why check for this.errorState?
// We only want to format values that are considered valid. For best UX,
// we only 'reward' valid inputs.
// - Why check for __isHandlingUserInput?
// Downwards sync is prevented whenever we are in a `@user-input-changed` flow.
// If we are in a 'imperatively set `.modelValue`' flow, we want to reflect back
// the value, no matter what.
// This means, whenever we are in errorState, we and modelValue is set
// imperatively, we DO want to format a value (it is the only way to get meaningful
// input into `.inputElement` with modelValue as input)
if (this.__isHandlingUserInput && this.errorState) {
return this.inputElement ? this.value : undefined;
}
return this.formatter(this.modelValue, this.formatOptions);
@ -231,11 +254,9 @@ export const FormatMixin = dedupeMixin(
*/
_syncValueUpwards() {
// Downwards syncing should only happen for <lion-field>.value changes from 'above'
this.__preventDownwardsSync = true;
// This triggers _onModelValueChanged and connects user input to the
// parsing/formatting/serializing loop
this.modelValue = this.__callParser(this.value);
this.__preventDownwardsSync = false;
}
/**
@ -243,9 +264,9 @@ export const FormatMixin = dedupeMixin(
*/
_reflectBackFormattedValueToUser() {
// Downwards syncing 'back and forth' prevents change event from being fired in IE.
// So only sync when the source of new <lion-field>.value change was not the 'input' event of
// inputElement
if (!this.__preventDownwardsSync) {
// So only sync when the source of new <lion-field>.value change was not the 'input' event
// of inputElement
if (!this.__isHandlingUserInput) {
// Text 'undefined' should not end up in <input>
this.value = typeof this.formattedValue !== 'undefined' ? this.formattedValue : '';
}
@ -266,9 +287,12 @@ export const FormatMixin = dedupeMixin(
}
_onUserInputChanged() {
// Upwards syncing. Most properties are delegated right away, value is synced to <lion-field>,
// to be able to act on (imperatively set) value changes
// Upwards syncing. Most properties are delegated right away, value is synced to
// <lion-field>, to be able to act on (imperatively set) value changes
this.__isHandlingUserInput = true;
this._syncValueUpwards();
this.__isHandlingUserInput = false;
}
constructor() {
@ -289,11 +313,11 @@ export const FormatMixin = dedupeMixin(
this.inputElement.addEventListener(this.formatOn, this._reflectBackFormattedValueDebounced);
this.inputElement.addEventListener('input', this._proxyInputEvent);
this.addEventListener('user-input-changed', this._onUserInputChanged);
// Connect the value found in <input> to the formatting/parsing/serializing loop as a fallback
// mechanism. Assume the user uses the value property of the <lion-field>(recommended api) as
// the api (this is a downwards sync).
// However, when no value is specified on <lion-field>, have support for sync of the real input
// to the <lion-field> (upwards sync).
// Connect the value found in <input> to the formatting/parsing/serializing loop as a
// fallback mechanism. Assume the user uses the value property of the
// <lion-field>(recommended api) as the api (this is a downwards sync).
// However, when no value is specified on <lion-field>, have support for sync of the real
// input to the <lion-field> (upwards sync).
if (typeof this.modelValue === 'undefined') {
this._syncValueUpwards();
}

View file

@ -157,6 +157,25 @@ describe('FormatMixin', () => {
expect(formatEl.inputElement.value).to.equal('foo: test');
});
it('reflects back .formattedValue immediately when .modelValue changed imperatively', async () => {
const el = await fixture(html`
<${elem} .formatter="${value => `foo: ${value}`}">
<input slot="input" />
</${elem}>
`);
// The FormatMixin can be used in conjunction with the ValidateMixin, in which case
// it can hold errorState (affecting the formatting)
el.errorState = true;
// users types value 'test'
mimicUserInput(el, 'test');
expect(el.inputElement.value).to.not.equal('foo: test');
// Now see the difference for an imperative change
el.modelValue = 'test2';
expect(el.inputElement.value).to.equal('foo: test2');
});
describe('parsers/formatters/serializers', () => {
it('should call the parser|formatter|serializer provided by user', async () => {
const formatterSpy = sinon.spy(value => `foo: ${value}`);
@ -206,7 +225,7 @@ describe('FormatMixin', () => {
expect(parserSpy.callCount).to.equal(1);
});
it('will only call the formatter for valid values', async () => {
it('will only call the formatter for valid values on `user-input-changed` ', async () => {
const formatterSpy = sinon.spy(value => `foo: ${value}`);
const el = await fixture(html`
<${elem} .formatter=${formatterSpy}>
@ -216,12 +235,12 @@ describe('FormatMixin', () => {
expect(formatterSpy.callCount).to.equal(1);
el.errorState = true;
el.modelValue = 'bar';
mimicUserInput(el, 'bar');
expect(formatterSpy.callCount).to.equal(1);
expect(el.formattedValue).to.equal('foo: init-string');
expect(el.formattedValue).to.equal('bar');
el.errorState = false;
el.modelValue = 'bar2';
mimicUserInput(el, 'bar2');
expect(formatterSpy.callCount).to.equal(2);
expect(el.formattedValue).to.equal('foo: bar2');

View file

@ -19,6 +19,11 @@ const tag = unsafeStatic(tagString);
const inputSlotString = '<input slot="input" />';
const inputSlot = unsafeHTML(inputSlotString);
function mimicUserInput(formControl, newViewValue) {
formControl.value = newViewValue; // eslint-disable-line no-param-reassign
formControl.inputElement.dispatchEvent(new CustomEvent('input', { bubbles: true }));
}
beforeEach(() => {
localizeTearDown();
});
@ -328,7 +333,7 @@ describe('<lion-field>', () => {
expect(lionField.error.required).to.be.undefined;
});
it('will only update formattedValue the value when valid', async () => {
it('will only update formattedValue when valid on `user-input-changed`', async () => {
const formatterSpy = sinon.spy(value => `foo: ${value}`);
function isBarValidator(value) {
return { isBar: value === 'bar' };
@ -348,9 +353,9 @@ describe('<lion-field>', () => {
expect(formatterSpy.callCount).to.equal(1);
expect(lionField.formattedValue).to.equal('foo: bar');
lionField.modelValue = 'foo';
mimicUserInput(lionField, 'foo');
expect(formatterSpy.callCount).to.equal(1);
expect(lionField.formattedValue).to.equal('foo: bar');
expect(lionField.formattedValue).to.equal('foo');
});
});