fix(ui): [form-core] enhance formatter and parser meta; reset _isPasting on task instead of microtask

Co-authored-by: gerjanvangeest <Gerjan.van.Geest@ing.com>
This commit is contained in:
Thijs Louisse 2025-01-15 16:33:02 +01:00 committed by GitHub
parent 45f06668f9
commit d5258d56fd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 123 additions and 30 deletions

View file

@ -2,4 +2,4 @@
'@lion/ui': minor '@lion/ui': minor
--- ---
[form-core] add "user-edit" mode to formatOptions while editing existing value of a form control [form-core] add "user-edited" mode to formatOptions while editing existing value of a form control

View file

@ -0,0 +1,5 @@
---
'@lion/ui': patch
---
[form-core] enhance formatter and parser meta with viewValueStates

View file

@ -1,14 +1,15 @@
/* eslint-disable class-methods-use-this */ /* eslint-disable class-methods-use-this */
import { dedupeMixin } from '@open-wc/dedupe-mixin'; import { dedupeMixin } from '@open-wc/dedupe-mixin';
import { ValidateMixin } from './validate/ValidateMixin.js';
import { FormControlMixin } from './FormControlMixin.js'; import { FormControlMixin } from './FormControlMixin.js';
import { Unparseable } from './validate/Unparseable.js'; import { Unparseable } from './validate/Unparseable.js';
import { ValidateMixin } from './validate/ValidateMixin.js';
/** /**
* @typedef {import('../types/FormatMixinTypes.js').FormatMixin} FormatMixin
* @typedef {import('../types/FormatMixinTypes.js').FormatOptions} FormatOptions
* @typedef {import('../types/FormControlMixinTypes.js').ModelValueEventDetails} ModelValueEventDetails * @typedef {import('../types/FormControlMixinTypes.js').ModelValueEventDetails} ModelValueEventDetails
* @typedef {import('../types/FormatMixinTypes.js').FormatOptions} FormatOptions
* @typedef {import('../types/FormatMixinTypes.js').FormatMixin} FormatMixin
*/ */
// For a future breaking release: // For a future breaking release:
@ -69,6 +70,16 @@ const FormatMixinImplementation = superclass =>
}; };
} }
/**
* During format/parse, we sometimes need to know how the current viewValue the user
* interacts with "came to be". Was it already formatted before for instance?
* If so, in case of an amount input, we might want to stick to the curent interpretation of separators.
*/
#viewValueState = {
didFormatterOutputSyncToView: false,
didFormatterRun: false,
};
/** /**
* @param {string} [name] * @param {string} [name]
* @param {unknown} [oldValue] * @param {unknown} [oldValue]
@ -93,7 +104,7 @@ const FormatMixinImplementation = superclass =>
* The view value. Will be delegated to `._inputNode.value` * The view value. Will be delegated to `._inputNode.value`
*/ */
get value() { get value() {
return (this._inputNode && this._inputNode.value) || this.__value || ''; return this._inputNode?.value || this.__value || '';
} }
/** @param {string} value */ /** @param {string} value */
@ -249,7 +260,11 @@ const FormatMixinImplementation = superclass =>
// Apparently, the parser was not able to produce a satisfactory output for the desired // 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 // 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. // states (for instance from a lost user session), since it saves the current viewValue.
const result = this.parser(value, { ...this.formatOptions, mode: this.#getFormatMode() }); const result = this.parser(value, {
...this.formatOptions,
mode: this.#getFormatOptionsMode(),
viewValueStates: this.#getFormatOptionsViewValueStates(),
});
return result !== undefined ? result : new Unparseable(value); return result !== undefined ? result : new Unparseable(value);
} }
@ -258,6 +273,8 @@ const FormatMixinImplementation = superclass =>
* @private * @private
*/ */
_callFormatter() { _callFormatter() {
this.#viewValueState.didFormatterRun = false;
// - Why check for this.hasError? // - Why check for this.hasError?
// We only want to format values that are considered valid. For best UX, // We only want to format values that are considered valid. For best UX,
// we only 'reward' valid inputs. // we only 'reward' valid inputs.
@ -280,9 +297,12 @@ const FormatMixinImplementation = superclass =>
return this.modelValue.viewValue; return this.modelValue.viewValue;
} }
this.#viewValueState.didFormatterRun = true;
return this.formatter(this.modelValue, { return this.formatter(this.modelValue, {
...this.formatOptions, ...this.formatOptions,
mode: this.#getFormatMode(), mode: this.#getFormatOptionsMode(),
viewValueStates: this.#getFormatOptionsViewValueStates(),
}); });
} }
@ -389,6 +409,8 @@ const FormatMixinImplementation = superclass =>
if (this._reflectBackOn()) { if (this._reflectBackOn()) {
// Text 'undefined' should not end up in <input> // Text 'undefined' should not end up in <input>
this.value = typeof this.formattedValue !== 'undefined' ? this.formattedValue : ''; this.value = typeof this.formattedValue !== 'undefined' ? this.formattedValue : '';
this.#viewValueState.didFormatterOutputSyncToView =
Boolean(this.formattedValue) && this.#viewValueState.didFormatterRun;
} }
} }
@ -539,7 +561,7 @@ const FormatMixinImplementation = superclass =>
*/ */
__onPaste() { __onPaste() {
this._isPasting = true; this._isPasting = true;
queueMicrotask(() => { setTimeout(() => {
this._isPasting = false; this._isPasting = false;
}); });
} }
@ -583,16 +605,37 @@ const FormatMixinImplementation = superclass =>
} }
} }
#getFormatMode() { /**
* This method will be called right before a parser or formatter gets called and computes
* the formatOptions.mode. In short, it gives meta info about how the user interacted with
* the form control, as these user interactions can influence the formatting/parsing behavior.
*
* It's important that the behavior of these can be different during a paste or user edit,
* but not during programmatic setting of values (like setting modelValue).
* @returns {'auto'|'pasted'|'user-edited'}
*/
#getFormatOptionsMode() {
if (this._isPasting) { if (this._isPasting) {
return 'pasted'; return 'pasted';
} }
const isUserEditing = this._isHandlingUserInput && this.__prevViewValue; const isUserEditing = this._isHandlingUserInput && this.__prevViewValue;
if (isUserEditing) { if (isUserEditing) {
return 'user-edit'; return 'user-edited';
} }
return 'auto'; return 'auto';
} }
/**
* @returns {FormatOptions['viewValueStates']}
*/
#getFormatOptionsViewValueStates() {
/** @type {FormatOptions['viewValueStates']}} */
const states = [];
if (this.#viewValueState.didFormatterOutputSyncToView) {
states.push('formatted');
}
return states;
}
}; };
export const FormatMixin = dedupeMixin(FormatMixinImplementation); export const FormatMixin = dedupeMixin(FormatMixinImplementation);

View file

@ -551,7 +551,7 @@ export function runFormatMixinSuite(customConfig) {
}); });
describe('On user input', () => { describe('On user input', () => {
it('adjusts formatOptions.mode to "user-edit" for parser when user changes value', async () => { it('adjusts formatOptions.mode to "user-edited" for parser when user changes value', async () => {
const el = /** @type {FormatClass} */ ( const el = /** @type {FormatClass} */ (
await fixture(html`<${tag}><input slot="input"></${tag}>`) await fixture(html`<${tag}><input slot="input"></${tag}>`)
); );
@ -567,9 +567,41 @@ export function runFormatMixinSuite(customConfig) {
mimicUserInput(el, 'some other val'); mimicUserInput(el, 'some other val');
expect(/** @type {{mode: string}} */ (parserSpy.lastCall.args[1]).mode).to.equal( expect(/** @type {{mode: string}} */ (parserSpy.lastCall.args[1]).mode).to.equal(
'user-edit', 'user-edited',
); );
});
it('adjusts formatOptions.viewValueStates when user already formatted value', async () => {
const el = /** @type {FormatClass} */ (
await fixture(html`<${tag}><input slot="input"></${tag}>`)
);
// Neutralize config, so that we know for sure that the user has formatted the value
// when this suite is run on extended classes
// always reflect back formattedVlaue to view (instead of on blur or else)
// @ts-expect-error [allow-protected] in test
el._reflectBackOn = () => true;
// avoid non-formatted values due to validators in extended classes
el.defaultValidators = [];
// avoid Unparseable due to parsers in extended classes
el.parser = v => v;
// @ts-expect-error
el.formatter = v => v;
const parserSpy = sinon.spy(el, 'parser');
const formatterSpy = sinon.spy(el, 'formatter');
mimicUserInput(el, 'some val');
expect(parserSpy.lastCall.args[1].viewValueStates).to.not.include('formatted');
// @ts-expect-error
expect(formatterSpy.lastCall.args[1].viewValueStates).to.not.include('formatted');
await el.updateComplete; await el.updateComplete;
mimicUserInput(el, 'some other val');
expect(parserSpy.lastCall.args[1].viewValueStates).to.include('formatted');
// @ts-expect-error
expect(formatterSpy.lastCall.args[1].viewValueStates).to.include('formatted');
}); });
}); });
}); });

View file

@ -3,7 +3,10 @@ import { LitElement } from 'lit';
import { ValidateHost } from './validate/ValidateMixinTypes.js'; import { ValidateHost } from './validate/ValidateMixinTypes.js';
import { FormControlHost } from './FormControlMixinTypes.js'; import { FormControlHost } from './FormControlMixinTypes.js';
export type FormatOptions = { mode: 'pasted' | 'auto' | 'user-edit'} & object; export type FormatOptions = {
mode: 'pasted' | 'auto' | 'user-edited';
viewValueStates?: 'formatted'[];
};
export declare class FormatHost { export declare class FormatHost {
/** /**
* Converts viewValue to modelValue * Converts viewValue to modelValue

View file

@ -131,7 +131,7 @@ describe('<lion-input-amount>', () => {
expect(_inputNode.value).to.equal('100.12'); expect(_inputNode.value).to.equal('100.12');
}); });
it('adjusts formats with locale when formatOptions.mode is "user-edit"', async () => { it('adjusts formats with locale when formatOptions.mode is "user-edited"', async () => {
const el = /** @type {LionInputAmount} */ ( const el = /** @type {LionInputAmount} */ (
await fixture( await fixture(
html`<lion-input-amount html`<lion-input-amount
@ -149,8 +149,8 @@ describe('<lion-input-amount>', () => {
// When editing an already existing value, we interpet the separators as they are // When editing an already existing value, we interpet the separators as they are
mimicUserInput(el, '123.456'); mimicUserInput(el, '123.456');
expect(parserSpy.lastCall.args[1]?.mode).to.equal('user-edit'); expect(parserSpy.lastCall.args[1]?.mode).to.equal('user-edited');
expect(formatterSpy.lastCall.args[1]?.mode).to.equal('user-edit'); expect(formatterSpy.lastCall.args[1]?.mode).to.equal('user-edited');
expect(el.modelValue).to.equal(123456); expect(el.modelValue).to.equal(123456);
expect(el.formattedValue).to.equal('123.456,00'); expect(el.formattedValue).to.equal('123.456,00');

View file

@ -61,10 +61,14 @@ describe('parseAmount()', async () => {
expect(parseAmount('EUR 1,50', { mode: 'pasted' })).to.equal(1.5); expect(parseAmount('EUR 1,50', { mode: 'pasted' })).to.equal(1.5);
}); });
it('parses based on locale when "user-edit" mode used', async () => { it('parses based on locale when "user-edited" mode used combined with viewValueStates "formatted"', async () => {
expect(parseAmount('123,456.78', { mode: 'auto' })).to.equal(123456.78); expect(parseAmount('123,456.78', { mode: 'auto' })).to.equal(123456.78);
expect(parseAmount('123,456.78', { mode: 'user-edit' })).to.equal(123456.78); expect(
parseAmount('123,456.78', { mode: 'user-edited', viewValueStates: ['formatted'] }),
).to.equal(123456.78);
expect(parseAmount('123.456,78', { mode: 'auto' })).to.equal(123456.78); expect(parseAmount('123.456,78', { mode: 'auto' })).to.equal(123456.78);
expect(parseAmount('123.456,78', { mode: 'user-edit' })).to.equal(123.45678); expect(
parseAmount('123.456,78', { mode: 'user-edited', viewValueStates: ['formatted'] }),
).to.equal(123.45678);
}); });
}); });

View file

@ -16,16 +16,18 @@ import { getDecimalSeparator } from './getDecimalSeparator.js';
* parseNumber('1,234.56'); // method: heuristic => 1234.56 * parseNumber('1,234.56'); // method: heuristic => 1234.56
* *
* @param {string} value Clean number (only [0-9 ,.]) to be parsed * @param {string} value Clean number (only [0-9 ,.]) to be parsed
* @param {object} options * @param {{mode?:'auto'|'pasted'|'user-edited'; viewValueStates?:string[]}} options
* @param {string?} [options.mode] auto|pasted|user-edit
* @return {string} unparseable|withLocale|heuristic * @return {string} unparseable|withLocale|heuristic
*/ */
function getParseMode(value, { mode = 'auto' } = {}) { function getParseMode(value, { mode = 'auto', viewValueStates } = {}) {
const separators = value.match(/[., ]/g); const separators = value.match(/[., ]/g);
// When a user edits an existin value, we already formatted it with a certain locale. // When a user edits an existing value, we already formatted it with a certain locale.
// For best UX, we stick with this locale // For best UX, we stick with this locale
if (!separators || mode === 'user-edit') { const shouldAlignWithExistingSeparators =
viewValueStates?.includes('formatted') && mode === 'user-edited';
if (!separators || shouldAlignWithExistingSeparators) {
return 'withLocale'; return 'withLocale';
} }
if (mode === 'auto' && separators.length === 1) { if (mode === 'auto' && separators.length === 1) {

View file

@ -74,11 +74,15 @@ describe('parseNumber()', () => {
expect(parseNumber('123456.78', { mode: 'pasted' })).to.equal(123456.78); expect(parseNumber('123456.78', { mode: 'pasted' })).to.equal(123456.78);
}); });
it('detects separators withLocale when "user-edit" mode used e.g. 123.456,78', async () => { it('detects separators withLocale when "user-edited" mode combined with viewValueStates "formatted" used e.g. 123.456,78', async () => {
expect(parseNumber('123,456.78', { mode: 'auto' })).to.equal(123456.78); expect(parseNumber('123,456.78', { mode: 'auto' })).to.equal(123456.78);
expect(parseNumber('123,456.78', { mode: 'user-edit' })).to.equal(123456.78); expect(
parseNumber('123,456.78', { mode: 'user-edited', viewValueStates: ['formatted'] }),
).to.equal(123456.78);
expect(parseNumber('123.456,78', { mode: 'auto' })).to.equal(123456.78); expect(parseNumber('123.456,78', { mode: 'auto' })).to.equal(123456.78);
expect(parseNumber('123.456,78', { mode: 'user-edit' })).to.equal(123.45678); expect(
parseNumber('123.456,78', { mode: 'user-edited', viewValueStates: ['formatted'] }),
).to.equal(123.45678);
}); });
it('detects separators unparseable when there are 2 same ones e.g. 1.234.56', () => { it('detects separators unparseable when there are 2 same ones e.g. 1.234.56', () => {

View file

@ -21,7 +21,7 @@ export declare interface FormatDateOptions extends Intl.DateTimeFormatOptions {
roundMode?: string; roundMode?: string;
returnIfNaN?: string; returnIfNaN?: string;
mode?: 'pasted' | 'auto' | 'user-edit'; mode?: 'pasted' | 'auto' | 'user-edited';
postProcessors?: Map<string, DatePostProcessor>; postProcessors?: Map<string, DatePostProcessor>;
} }
@ -41,8 +41,8 @@ export declare interface FormatNumberOptions extends Intl.NumberFormatOptions {
// https://en.wikipedia.org/wiki/Decimal_separator#Digit_grouping // https://en.wikipedia.org/wiki/Decimal_separator#Digit_grouping
// note the half space in there as well // note the half space in there as well
groupSeparator?: ',' | '.' | '' | '_' | ' ' | "'"; groupSeparator?: ',' | '.' | '' | '_' | ' ' | "'";
mode?: 'pasted' | 'auto' | 'user-edit'; mode?: 'pasted' | 'auto' | 'user-edited';
viewValueStates?: 'formatted'[];
postProcessors?: Map<string, NumberPostProcessor>; postProcessors?: Map<string, NumberPostProcessor>;
} }