diff --git a/.changeset/bright-snakes-smash.md b/.changeset/bright-snakes-smash.md index e52963303..27100a548 100644 --- a/.changeset/bright-snakes-smash.md +++ b/.changeset/bright-snakes-smash.md @@ -2,4 +2,4 @@ '@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 diff --git a/.changeset/twenty-planes-push.md b/.changeset/twenty-planes-push.md new file mode 100644 index 000000000..e1404e8b7 --- /dev/null +++ b/.changeset/twenty-planes-push.md @@ -0,0 +1,5 @@ +--- +'@lion/ui': patch +--- + +[form-core] enhance formatter and parser meta with viewValueStates diff --git a/packages/ui/components/form-core/src/FormatMixin.js b/packages/ui/components/form-core/src/FormatMixin.js index da1a4c55b..ffe7fc6f6 100644 --- a/packages/ui/components/form-core/src/FormatMixin.js +++ b/packages/ui/components/form-core/src/FormatMixin.js @@ -1,14 +1,15 @@ /* eslint-disable class-methods-use-this */ import { dedupeMixin } from '@open-wc/dedupe-mixin'; + +import { ValidateMixin } from './validate/ValidateMixin.js'; import { FormControlMixin } from './FormControlMixin.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/FormatMixinTypes.js').FormatOptions} FormatOptions + * @typedef {import('../types/FormatMixinTypes.js').FormatMixin} FormatMixin */ // 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 {unknown} [oldValue] @@ -93,7 +104,7 @@ const FormatMixinImplementation = superclass => * The view value. Will be delegated to `._inputNode.value` */ get value() { - return (this._inputNode && this._inputNode.value) || this.__value || ''; + return this._inputNode?.value || this.__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 // 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, mode: this.#getFormatMode() }); + const result = this.parser(value, { + ...this.formatOptions, + mode: this.#getFormatOptionsMode(), + viewValueStates: this.#getFormatOptionsViewValueStates(), + }); return result !== undefined ? result : new Unparseable(value); } @@ -258,6 +273,8 @@ const FormatMixinImplementation = superclass => * @private */ _callFormatter() { + this.#viewValueState.didFormatterRun = false; + // - Why check for this.hasError? // We only want to format values that are considered valid. For best UX, // we only 'reward' valid inputs. @@ -280,9 +297,12 @@ const FormatMixinImplementation = superclass => return this.modelValue.viewValue; } + this.#viewValueState.didFormatterRun = true; + return this.formatter(this.modelValue, { ...this.formatOptions, - mode: this.#getFormatMode(), + mode: this.#getFormatOptionsMode(), + viewValueStates: this.#getFormatOptionsViewValueStates(), }); } @@ -389,6 +409,8 @@ const FormatMixinImplementation = superclass => if (this._reflectBackOn()) { // Text 'undefined' should not end up in 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() { this._isPasting = true; - queueMicrotask(() => { + setTimeout(() => { 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) { return 'pasted'; } const isUserEditing = this._isHandlingUserInput && this.__prevViewValue; if (isUserEditing) { - return 'user-edit'; + return 'user-edited'; } 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); diff --git a/packages/ui/components/form-core/test-suites/FormatMixin.suite.js b/packages/ui/components/form-core/test-suites/FormatMixin.suite.js index 93151d024..46784d627 100644 --- a/packages/ui/components/form-core/test-suites/FormatMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/FormatMixin.suite.js @@ -551,7 +551,7 @@ export function runFormatMixinSuite(customConfig) { }); 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} */ ( await fixture(html`<${tag}>`) ); @@ -567,9 +567,41 @@ export function runFormatMixinSuite(customConfig) { mimicUserInput(el, 'some other val'); 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}>`) + ); + + // 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; + + 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'); }); }); }); diff --git a/packages/ui/components/form-core/types/FormatMixinTypes.ts b/packages/ui/components/form-core/types/FormatMixinTypes.ts index 929cfe1cd..117bdeb8f 100644 --- a/packages/ui/components/form-core/types/FormatMixinTypes.ts +++ b/packages/ui/components/form-core/types/FormatMixinTypes.ts @@ -3,7 +3,10 @@ import { LitElement } from 'lit'; import { ValidateHost } from './validate/ValidateMixinTypes.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 { /** * Converts viewValue to modelValue diff --git a/packages/ui/components/input-amount/test/lion-input-amount.test.js b/packages/ui/components/input-amount/test/lion-input-amount.test.js index 0acfd8783..cfaaf0762 100644 --- a/packages/ui/components/input-amount/test/lion-input-amount.test.js +++ b/packages/ui/components/input-amount/test/lion-input-amount.test.js @@ -131,7 +131,7 @@ describe('', () => { 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} */ ( await fixture( html`', () => { // When editing an already existing value, we interpet the separators as they are mimicUserInput(el, '123.456'); - expect(parserSpy.lastCall.args[1]?.mode).to.equal('user-edit'); - expect(formatterSpy.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-edited'); expect(el.modelValue).to.equal(123456); expect(el.formattedValue).to.equal('123.456,00'); diff --git a/packages/ui/components/input-amount/test/parsers.test.js b/packages/ui/components/input-amount/test/parsers.test.js index 20af3ae3d..e8a990db3 100644 --- a/packages/ui/components/input-amount/test/parsers.test.js +++ b/packages/ui/components/input-amount/test/parsers.test.js @@ -61,10 +61,14 @@ describe('parseAmount()', async () => { 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: '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: 'user-edit' })).to.equal(123.45678); + expect( + parseAmount('123.456,78', { mode: 'user-edited', viewValueStates: ['formatted'] }), + ).to.equal(123.45678); }); }); diff --git a/packages/ui/components/localize/src/number/parseNumber.js b/packages/ui/components/localize/src/number/parseNumber.js index dc2fa350d..1a3ae3f9e 100644 --- a/packages/ui/components/localize/src/number/parseNumber.js +++ b/packages/ui/components/localize/src/number/parseNumber.js @@ -16,16 +16,18 @@ import { getDecimalSeparator } from './getDecimalSeparator.js'; * parseNumber('1,234.56'); // method: heuristic => 1234.56 * * @param {string} value Clean number (only [0-9 ,.]) to be parsed - * @param {object} options - * @param {string?} [options.mode] auto|pasted|user-edit + * @param {{mode?:'auto'|'pasted'|'user-edited'; viewValueStates?:string[]}} options * @return {string} unparseable|withLocale|heuristic */ -function getParseMode(value, { mode = 'auto' } = {}) { +function getParseMode(value, { mode = 'auto', viewValueStates } = {}) { 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 - if (!separators || mode === 'user-edit') { + const shouldAlignWithExistingSeparators = + viewValueStates?.includes('formatted') && mode === 'user-edited'; + + if (!separators || shouldAlignWithExistingSeparators) { return 'withLocale'; } if (mode === 'auto' && separators.length === 1) { diff --git a/packages/ui/components/localize/test/number/parseNumber.test.js b/packages/ui/components/localize/test/number/parseNumber.test.js index e58648409..4b806794e 100644 --- a/packages/ui/components/localize/test/number/parseNumber.test.js +++ b/packages/ui/components/localize/test/number/parseNumber.test.js @@ -74,11 +74,15 @@ describe('parseNumber()', () => { 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: '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: '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', () => { diff --git a/packages/ui/components/localize/types/LocalizeMixinTypes.ts b/packages/ui/components/localize/types/LocalizeMixinTypes.ts index 86697f549..08f6ab6d2 100644 --- a/packages/ui/components/localize/types/LocalizeMixinTypes.ts +++ b/packages/ui/components/localize/types/LocalizeMixinTypes.ts @@ -21,7 +21,7 @@ export declare interface FormatDateOptions extends Intl.DateTimeFormatOptions { roundMode?: string; returnIfNaN?: string; - mode?: 'pasted' | 'auto' | 'user-edit'; + mode?: 'pasted' | 'auto' | 'user-edited'; postProcessors?: Map; } @@ -41,8 +41,8 @@ export declare interface FormatNumberOptions extends Intl.NumberFormatOptions { // https://en.wikipedia.org/wiki/Decimal_separator#Digit_grouping // note the half space in there as well groupSeparator?: ',' | '.' | ' ' | '_' | ' ' | "'"; - mode?: 'pasted' | 'auto' | 'user-edit'; - + mode?: 'pasted' | 'auto' | 'user-edited'; + viewValueStates?: 'formatted'[]; postProcessors?: Map; }