From 7e8df69a9ae907a82b9316ebbd38cae3fe338e5a Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Tue, 14 Jan 2025 18:15:35 +0100 Subject: [PATCH] chore: enhance code readability and robustness --- .../components/form-core/src/FormatMixin.js | 43 +++++++------------ .../test-suites/FormatMixin.suite.js | 34 +++++++-------- .../test/lion-input-amount.test.js | 13 +++--- 3 files changed, 40 insertions(+), 50 deletions(-) diff --git a/packages/ui/components/form-core/src/FormatMixin.js b/packages/ui/components/form-core/src/FormatMixin.js index 3c468951a..da1a4c55b 100644 --- a/packages/ui/components/form-core/src/FormatMixin.js +++ b/packages/ui/components/form-core/src/FormatMixin.js @@ -241,8 +241,6 @@ const FormatMixinImplementation = superclass => return undefined; } - this.#setFormatModeForParseOrFormat('user-edit'); - // B) parse the view value // - if result: @@ -251,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); } @@ -272,7 +270,7 @@ const FormatMixinImplementation = superclass => // input into `._inputNode` with modelValue as input) if (this._isHandlingUserInput && this.hasFeedbackFor?.includes('error') && this._inputNode) { - return this._inputNode ? this.value : undefined; + return this.value; } if (this.modelValue instanceof Unparseable) { @@ -282,9 +280,10 @@ const FormatMixinImplementation = superclass => return this.modelValue.viewValue; } - this.#setFormatModeForParseOrFormat('user-edit'); - - return this.formatter(this.modelValue, this.formatOptions); + return this.formatter(this.modelValue, { + ...this.formatOptions, + mode: this.#getFormatMode(), + }); } /** @@ -540,7 +539,7 @@ const FormatMixinImplementation = superclass => */ __onPaste() { this._isPasting = true; - this.#setFormatModeForParseOrFormat('pasted', () => { + queueMicrotask(() => { this._isPasting = false; }); } @@ -584,25 +583,15 @@ const FormatMixinImplementation = superclass => } } - /** - * @param {'user-edit'|'pasted'} newMode - * @param {() => void} [callbackFn] - * @returns {void} - */ - #setFormatModeForParseOrFormat(newMode, callbackFn) { - const isUserEditingOrPasting = Boolean( - this._isPasting || (this._isHandlingUserInput && this.__prevViewValue), - ); - if (!isUserEditingOrPasting) return; - // an in-progress mode of 'pasted' takes precedence over 'user-edit' - if (this.formatOptions.mode === 'pasted') return; - - this.formatOptions.mode = newMode; - // - queueMicrotask(() => { - this.formatOptions.mode = 'auto'; - callbackFn?.(); - }); + #getFormatMode() { + if (this._isPasting) { + return 'pasted'; + } + const isUserEditing = this._isHandlingUserInput && this.__prevViewValue; + if (isUserEditing) { + return 'user-edit'; + } + return 'auto'; } }; 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 b5629c032..93151d024 100644 --- a/packages/ui/components/form-core/test-suites/FormatMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/FormatMixin.suite.js @@ -498,9 +498,15 @@ export function runFormatMixinSuite(customConfig) { const formatterSpy = sinon.spy(el, 'formatter'); 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); - 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 () => { @@ -547,29 +553,23 @@ export function runFormatMixinSuite(customConfig) { 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} - .parser=${sinon.spy(v => v)} - >`, - ) + await fixture(html`<${tag}>`) ); - // We start with auto mode - expect(el.formatOptions.mode).to.equal('auto'); + 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(el.formatOptions.mode).to.equal('auto'); + expect(/** @type {{mode: string}} */ (parserSpy.lastCall.args[1]).mode).to.equal( + 'auto', + ); await el.updateComplete; mimicUserInput(el, 'some other val'); - expect(el.formatOptions.mode).to.equal('user-edit'); + expect(/** @type {{mode: string}} */ (parserSpy.lastCall.args[1]).mode).to.equal( + 'user-edit', + ); await el.updateComplete; - - // 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 progrmatically set modelValue, that should behave idempotent, regardless of when it is set) - await aTimeout(0); - expect(el.formatOptions.mode).to.equal('auto'); }); }); }); 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 10a831975..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 @@ -142,23 +142,24 @@ describe('', () => { ) ); 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.args[0][1]?.mode).to.equal('user-edit'); - expect(el.formatOptions.mode).to.equal('user-edit'); + 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) - await aTimeout(0); - expect(el.modelValue).to.equal(123456); - expect(el.formattedValue).to.equal('123.456,00'); - expect(el.formatOptions.mode).to.equal('auto'); + 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 () => {