diff --git a/.changeset/bright-snakes-smash.md b/.changeset/bright-snakes-smash.md new file mode 100644 index 000000000..e52963303 --- /dev/null +++ b/.changeset/bright-snakes-smash.md @@ -0,0 +1,5 @@ +--- +'@lion/ui': minor +--- + +[form-core] add "user-edit" mode to formatOptions while editing existing value of a form control diff --git a/packages/ui/components/form-core/src/FormatMixin.js b/packages/ui/components/form-core/src/FormatMixin.js index d80636db0..da1a4c55b 100644 --- a/packages/ui/components/form-core/src/FormatMixin.js +++ b/packages/ui/components/form-core/src/FormatMixin.js @@ -249,7 +249,7 @@ 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); + const result = this.parser(value, { ...this.formatOptions, mode: this.#getFormatMode() }); return result !== undefined ? result : new Unparseable(value); } @@ -269,13 +269,8 @@ const FormatMixinImplementation = superclass => // imperatively, we DO want to format a value (it is the only way to get meaningful // input into `._inputNode` with modelValue as input) - if ( - this._isHandlingUserInput && - this.hasFeedbackFor?.length && - this.hasFeedbackFor.includes('error') && - this._inputNode - ) { - return this._inputNode ? this.value : undefined; + if (this._isHandlingUserInput && this.hasFeedbackFor?.includes('error') && this._inputNode) { + return this.value; } if (this.modelValue instanceof Unparseable) { @@ -285,7 +280,10 @@ const FormatMixinImplementation = superclass => return this.modelValue.viewValue; } - return this.formatter(this.modelValue, this.formatOptions); + return this.formatter(this.modelValue, { + ...this.formatOptions, + mode: this.#getFormatMode(), + }); } /** @@ -348,7 +346,6 @@ const FormatMixinImplementation = superclass => * @private */ __handlePreprocessor() { - const unprocessedValue = this.value; let currentCaretIndex = this.value.length; // Be gentle with Safari if ( @@ -364,7 +361,6 @@ const FormatMixinImplementation = superclass => prevViewValue: this.__prevViewValue, }); - this.__prevViewValue = unprocessedValue; if (preprocessedValue === undefined) { // Make sure we do no set back original value, so we preserve // caret index (== selectionStart/selectionEnd) @@ -459,7 +455,7 @@ const FormatMixinImplementation = superclass => /** * Configuration object that will be available inside the formatter function */ - this.formatOptions = /** @type {FormatOptions} */ ({}); + this.formatOptions = /** @type {FormatOptions} */ ({ mode: 'auto' }); /** * The view value is the result of the formatter function (when available). @@ -543,10 +539,8 @@ const FormatMixinImplementation = superclass => */ __onPaste() { this._isPasting = true; - this.formatOptions.mode = 'pasted'; - setTimeout(() => { + queueMicrotask(() => { this._isPasting = false; - this.formatOptions.mode = 'auto'; }); } @@ -588,6 +582,17 @@ const FormatMixinImplementation = superclass => this._inputNode.removeEventListener('compositionend', this.__onCompositionEvent); } } + + #getFormatMode() { + if (this._isPasting) { + return 'pasted'; + } + const isUserEditing = this._isHandlingUserInput && this.__prevViewValue; + if (isUserEditing) { + return 'user-edit'; + } + return 'auto'; + } }; 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 751a59e4f..93151d024 100644 --- a/packages/ui/components/form-core/test-suites/FormatMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/FormatMixin.suite.js @@ -5,6 +5,8 @@ import sinon from 'sinon'; import { Unparseable, Validator, FormatMixin } from '@lion/ui/form-core.js'; import { getFormControlMembers, mimicUserInput } from '@lion/ui/form-core-test-helpers.js'; +const isLionInputStepper = (/** @type {FormatClass} */ el) => 'valueTextMapping' in el; + /** * @typedef {import('../types/FormControlMixinTypes.js').FormControlHost} FormControlHost * @typedef {ArrayConstructor | ObjectConstructor | NumberConstructor | BooleanConstructor | StringConstructor | DateConstructor | 'iban' | 'email'} modelValueType @@ -480,7 +482,7 @@ export function runFormatMixinSuite(customConfig) { /** * @param {FormatClass} el */ - function paste(el, val = 'lorem') { + function mimicPaste(el, val = 'lorem') { const { _inputNode } = getFormControlMembers(el); _inputNode.value = val; _inputNode.dispatchEvent(new ClipboardEvent('paste', { bubbles: true })); @@ -494,12 +496,17 @@ export function runFormatMixinSuite(customConfig) { `) ); const formatterSpy = sinon.spy(el, 'formatter'); - paste(el); + mimicPaste(el); expect(formatterSpy).to.be.called; - expect(/** @type {{mode: string}} */ (formatterSpy.args[0][1]).mode).to.equal('pasted'); + expect(/** @type {{mode: string}} */ (formatterSpy.lastCall.args[1]).mode).to.equal( + 'pasted', + ); + // give microtask of _isPasting chance to reset await aTimeout(0); - mimicUserInput(el, ''); - expect(/** @type {{mode: string}} */ (formatterSpy.args[0][1]).mode).to.equal('auto'); + el.modelValue = 'foo'; + expect(/** @type {{mode: string}} */ (formatterSpy.lastCall.args[1]).mode).to.equal( + 'auto', + ); }); it('sets protected value "_isPasting" for Subclassers', async () => { @@ -509,7 +516,7 @@ export function runFormatMixinSuite(customConfig) { `) ); const formatterSpy = sinon.spy(el, 'formatter'); - paste(el); + mimicPaste(el); expect(formatterSpy).to.have.been.called; // @ts-ignore [allow-protected] in test expect(el._isPasting).to.be.true; @@ -526,7 +533,7 @@ export function runFormatMixinSuite(customConfig) { ); // @ts-ignore [allow-protected] in test const reflectBackSpy = sinon.spy(el, '_reflectBackOn'); - paste(el); + mimicPaste(el); expect(reflectBackSpy).to.have.been.called; }); @@ -538,10 +545,33 @@ export function runFormatMixinSuite(customConfig) { ); // @ts-ignore [allow-protected] in test const reflectBackSpy = sinon.spy(el, '_reflectBackOn'); - paste(el); + mimicPaste(el); expect(reflectBackSpy).to.have.been.called; }); }); + + describe('On user input', () => { + it('adjusts formatOptions.mode to "user-edit" for parser when user changes value', async () => { + const el = /** @type {FormatClass} */ ( + await fixture(html`<${tag}>`) + ); + + const parserSpy = sinon.spy(el, 'parser'); + + // Here we get auto as we start from '' (so there was no value to edit) + mimicUserInput(el, 'some val'); + expect(/** @type {{mode: string}} */ (parserSpy.lastCall.args[1]).mode).to.equal( + 'auto', + ); + await el.updateComplete; + + mimicUserInput(el, 'some other val'); + expect(/** @type {{mode: string}} */ (parserSpy.lastCall.args[1]).mode).to.equal( + 'user-edit', + ); + await el.updateComplete; + }); + }); }); describe('Parser', () => { @@ -597,6 +627,43 @@ export function runFormatMixinSuite(customConfig) { expect(_inputNode.value).to.equal(val); }); + it('does only calculate derived values as consequence of user input when preprocessed value is different from previous view value', async () => { + const val = generateValueBasedOnType({ viewValue: true }) || 'init-value'; + if (typeof val !== 'string') return; + + const preprocessorSpy = sinon.spy(v => v.replace(/\$$/g, '')); + const el = /** @type {FormatClass} */ ( + await fixture(html` + <${tag} .preprocessor=${preprocessorSpy}> + + + `) + ); + + // TODO: find out why we need to skip this for lion-input-stepper + if (isLionInputStepper(el)) return; + + /** + * The _calculateValues method is called inside _onUserInputChanged w/o providing args + * @param {sinon.SinonSpyCall} call + * @returns {boolean} + */ + const isCalculateCallAfterUserInput = call => call.args[0]?.length === 0; + + const didRecalculateAfterUserInput = (/** @type {sinon.SinonSpy} */ spy) => + spy.callCount > 1 && !spy.getCalls().find(isCalculateCallAfterUserInput); + + // @ts-expect-error [allow-protected] in test + const calcValuesSpy = sinon.spy(el, '_calculateValues'); + // this value gets preprocessed to 'val' + mimicUserInput(el, `${val}$`); + expect(didRecalculateAfterUserInput(calcValuesSpy)).to.be.false; + + // this value gets preprocessed to 'value' (and thus differs from previous) + mimicUserInput(el, `${val}ue$`); + expect(didRecalculateAfterUserInput(calcValuesSpy)).to.be.true; + }); + it('does not preprocess during composition', async () => { const el = /** @type {FormatClass} */ ( await fixture(html` diff --git a/packages/ui/components/form-core/types/FormatMixinTypes.ts b/packages/ui/components/form-core/types/FormatMixinTypes.ts index 2c09f91ce..929cfe1cd 100644 --- a/packages/ui/components/form-core/types/FormatMixinTypes.ts +++ b/packages/ui/components/form-core/types/FormatMixinTypes.ts @@ -3,7 +3,7 @@ import { LitElement } from 'lit'; import { ValidateHost } from './validate/ValidateMixinTypes.js'; import { FormControlHost } from './FormControlMixinTypes.js'; -export type FormatOptions = { mode: 'pasted' | 'auto' } & object; +export type FormatOptions = { mode: 'pasted' | 'auto' | 'user-edit'} & object; export declare class FormatHost { /** * Converts viewValue to modelValue diff --git a/packages/ui/components/input-amount/src/parsers.js b/packages/ui/components/input-amount/src/parsers.js index ecd80dcc1..4ed7ea65f 100644 --- a/packages/ui/components/input-amount/src/parsers.js +++ b/packages/ui/components/input-amount/src/parsers.js @@ -1,7 +1,7 @@ import { parseNumber, getFractionDigits } from '@lion/ui/localize-no-side-effects.js'; /** - * @typedef {import('../../localize/types/LocalizeMixinTypes.js').FormatNumberOptions} FormatOptions + * @typedef {import('../../localize/types/LocalizeMixinTypes.js').FormatNumberOptions} FormatNumberOptions */ /** @@ -28,7 +28,7 @@ function round(value, decimals) { * parseAmount('1,234.56', {currency: 'JOD'}); => 1234.560 * * @param {string} value Number to be parsed - * @param {FormatOptions} [givenOptions] Locale Options + * @param {FormatNumberOptions} [givenOptions] Locale Options */ export function parseAmount(value, givenOptions) { const unmatchedInput = value.match(/[^0-9,.\- ]/g); @@ -44,7 +44,7 @@ export function parseAmount(value, givenOptions) { return undefined; } - /** @type {FormatOptions} */ + /** @type {FormatNumberOptions} */ const options = { ...givenOptions, }; 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 6e37394e2..0acfd8783 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 @@ -1,10 +1,11 @@ import { aTimeout, expect, fixture } from '@open-wc/testing'; import { html } from 'lit'; -import { localize } from '@lion/ui/localize.js'; +import { getLocalizeManager } from '@lion/ui/localize-no-side-effects.js'; import { localizeTearDown } from '@lion/ui/localize-test-helpers.js'; import { getInputMembers } from '@lion/ui/input-test-helpers.js'; import { LionInputAmount, formatAmount, parseAmount } from '@lion/ui/input-amount.js'; - +import { mimicUserInput } from '@lion/ui/form-core-test-helpers.js'; +import sinon from 'sinon'; import '@lion/ui/define/lion-input-amount.js'; /** @@ -12,6 +13,8 @@ import '@lion/ui/define/lion-input-amount.js'; */ describe('', () => { + const localize = getLocalizeManager(); + beforeEach(() => { localizeTearDown(); }); @@ -128,6 +131,37 @@ describe('', () => { expect(_inputNode.value).to.equal('100.12'); }); + it('adjusts formats with locale when formatOptions.mode is "user-edit"', async () => { + const el = /** @type {LionInputAmount} */ ( + await fixture( + html``, + ) + ); + const parserSpy = sinon.spy(el, 'parser'); + const formatterSpy = sinon.spy(el, 'formatter'); + + // @ts-expect-error [allow-protected] in test + expect(el._inputNode.value).to.equal('123.456,78'); + + // 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(el.modelValue).to.equal(123456); + expect(el.formattedValue).to.equal('123.456,00'); + + // Formatting should only affect values that should be formatted / parsed as a consequence of user input. + // When a user finished editing, the default should be restored. + // (think of a programmatically set modelValue, that should behave idempotent, regardless of when it is set) + el.modelValue = 1234; + expect(el.formattedValue).to.equal('1.234,00'); + expect(formatterSpy.lastCall.args[1]?.mode).to.equal('auto'); + }); + it('sets inputmode attribute to decimal', async () => { const el = /** @type {LionInputAmount} */ ( await fixture(``) diff --git a/packages/ui/components/input-amount/test/parsers.test.js b/packages/ui/components/input-amount/test/parsers.test.js index 6cd58449c..20af3ae3d 100644 --- a/packages/ui/components/input-amount/test/parsers.test.js +++ b/packages/ui/components/input-amount/test/parsers.test.js @@ -60,4 +60,11 @@ describe('parseAmount()', async () => { expect(parseAmount('foo1', { mode: 'pasted' })).to.equal(1); expect(parseAmount('EUR 1,50', { mode: 'pasted' })).to.equal(1.5); }); + + it('parses based on locale when "user-edit" mode used', 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: 'auto' })).to.equal(123456.78); + expect(parseAmount('123.456,78', { mode: 'user-edit' })).to.equal(123.45678); + }); }); diff --git a/packages/ui/components/localize/src/number/getDecimalSeparator.js b/packages/ui/components/localize/src/number/getDecimalSeparator.js index 90808306b..7b21dc60e 100644 --- a/packages/ui/components/localize/src/number/getDecimalSeparator.js +++ b/packages/ui/components/localize/src/number/getDecimalSeparator.js @@ -8,7 +8,7 @@ import { getLocale } from '../utils/getLocale.js'; * @returns {string} The separator */ export function getDecimalSeparator(locale, options) { - if (options && options.decimalSeparator) { + if (options?.decimalSeparator) { return options.decimalSeparator; } const computedLocale = getLocale(locale); diff --git a/packages/ui/components/localize/src/number/parseNumber.js b/packages/ui/components/localize/src/number/parseNumber.js index c0be7bb25..dc2fa350d 100644 --- a/packages/ui/components/localize/src/number/parseNumber.js +++ b/packages/ui/components/localize/src/number/parseNumber.js @@ -17,13 +17,15 @@ import { getDecimalSeparator } from './getDecimalSeparator.js'; * * @param {string} value Clean number (only [0-9 ,.]) to be parsed * @param {object} options - * @param {string?} [options.mode] auto|pasted + * @param {string?} [options.mode] auto|pasted|user-edit * @return {string} unparseable|withLocale|heuristic */ function getParseMode(value, { mode = 'auto' } = {}) { const separators = value.match(/[., ]/g); - if (!separators) { + // When a user edits an existin value, we already formatted it with a certain locale. + // For best UX, we stick with this locale + if (!separators || mode === 'user-edit') { return 'withLocale'; } if (mode === 'auto' && separators.length === 1) { @@ -52,8 +54,7 @@ function getParseMode(value, { mode = 'auto' } = {}) { * @param {import('../../types/LocalizeMixinTypes.js').FormatNumberOptions} options Locale Options */ function parseWithLocale(value, options) { - const locale = options && options.locale ? options.locale : undefined; - const separator = getDecimalSeparator(locale, options); + const separator = getDecimalSeparator(options?.locale, options); const regexNumberAndLocaleSeparator = new RegExp(`[0-9${separator}-]`, 'g'); let numberAndLocaleSeparator = value.match(regexNumberAndLocaleSeparator)?.join(''); if (separator === ',') { diff --git a/packages/ui/components/localize/test/number/parseNumber.test.js b/packages/ui/components/localize/test/number/parseNumber.test.js index ac0dbeec8..e58648409 100644 --- a/packages/ui/components/localize/test/number/parseNumber.test.js +++ b/packages/ui/components/localize/test/number/parseNumber.test.js @@ -74,6 +74,13 @@ 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 () => { + 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: 'auto' })).to.equal(123456.78); + expect(parseNumber('123.456,78', { mode: 'user-edit' })).to.equal(123.45678); + }); + it('detects separators unparseable when there are 2 same ones e.g. 1.234.56', () => { expect(parseNumber('1.234.56')).to.equal(123456); expect(parseNumber('1,234,56')).to.equal(123456); diff --git a/packages/ui/components/localize/types/LocalizeMixinTypes.ts b/packages/ui/components/localize/types/LocalizeMixinTypes.ts index 1257acbb7..86697f549 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'; + mode?: 'pasted' | 'auto' | 'user-edit'; postProcessors?: Map; } @@ -41,7 +41,7 @@ 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'; + mode?: 'pasted' | 'auto' | 'user-edit'; postProcessors?: Map; }