Skip to content

Commit

Permalink
chore: enhance code readability and robustness
Browse files Browse the repository at this point in the history
  • Loading branch information
tlouisse committed Jan 14, 2025
1 parent dc4744e commit 7e8df69
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 50 deletions.
43 changes: 16 additions & 27 deletions packages/ui/components/form-core/src/FormatMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,6 @@ const FormatMixinImplementation = superclass =>
return undefined;
}

this.#setFormatModeForParseOrFormat('user-edit');

// B) parse the view value

// - if result:
Expand All @@ -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);
}

Expand All @@ -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) {
Expand All @@ -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(),
});
}

/**
Expand Down Expand Up @@ -540,7 +539,7 @@ const FormatMixinImplementation = superclass =>
*/
__onPaste() {
this._isPasting = true;
this.#setFormatModeForParseOrFormat('pasted', () => {
queueMicrotask(() => {
this._isPasting = false;
});
}
Expand Down Expand Up @@ -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';
}
};

Expand Down
34 changes: 17 additions & 17 deletions packages/ui/components/form-core/test-suites/FormatMixin.suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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)}
><input slot="input"></${tag}>`,
)
await fixture(html`<${tag}><input slot="input"></${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');
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,23 +142,24 @@ describe('<lion-input-amount>', () => {
)
);
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 () => {
Expand Down

0 comments on commit 7e8df69

Please sign in to comment.