Skip to content

Commit

Permalink
fix(ui): [form-core] enhance formatter and parser meta; reset _isPast…
Browse files Browse the repository at this point in the history
…ing on task instead of microtask

Co-authored-by: gerjanvangeest <[email protected]>
  • Loading branch information
tlouisse and gerjanvangeest authored Jan 15, 2025
1 parent 45f0666 commit d5258d5
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .changeset/bright-snakes-smash.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions .changeset/twenty-planes-push.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[form-core] enhance formatter and parser meta with viewValueStates
61 changes: 52 additions & 9 deletions packages/ui/components/form-core/src/FormatMixin.js
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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]
Expand All @@ -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 */
Expand Down Expand Up @@ -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);
}

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

Expand Down Expand Up @@ -389,6 +409,8 @@ const FormatMixinImplementation = superclass =>
if (this._reflectBackOn()) {
// Text 'undefined' should not end up in <input>
this.value = typeof this.formattedValue !== 'undefined' ? this.formattedValue : '';
this.#viewValueState.didFormatterOutputSyncToView =
Boolean(this.formattedValue) && this.#viewValueState.didFormatterRun;
}
}

Expand Down Expand Up @@ -539,7 +561,7 @@ const FormatMixinImplementation = superclass =>
*/
__onPaste() {
this._isPasting = true;
queueMicrotask(() => {
setTimeout(() => {
this._isPasting = false;
});
}
Expand Down Expand Up @@ -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);
36 changes: 34 additions & 2 deletions packages/ui/components/form-core/test-suites/FormatMixin.suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}><input slot="input"></${tag}>`)
);
Expand All @@ -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}><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;

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');
});
});
});
Expand Down
5 changes: 4 additions & 1 deletion packages/ui/components/form-core/types/FormatMixinTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('<lion-input-amount>', () => {
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`<lion-input-amount
Expand All @@ -149,8 +149,8 @@ describe('<lion-input-amount>', () => {

// 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');

Expand Down
10 changes: 7 additions & 3 deletions packages/ui/components/input-amount/test/parsers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
12 changes: 7 additions & 5 deletions packages/ui/components/localize/src/number/parseNumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 7 additions & 3 deletions packages/ui/components/localize/test/number/parseNumber.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/ui/components/localize/types/LocalizeMixinTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, DatePostProcessor>;
}
Expand All @@ -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<string, NumberPostProcessor>;
}

Expand Down

0 comments on commit d5258d5

Please sign in to comment.