Skip to content

Commit

Permalink
fix(ui): [input-amount] make sure that previous locale is not used fo…
Browse files Browse the repository at this point in the history
…r parsing on user-edit with <= 2 decimals
  • Loading branch information
tlouisse committed Jan 30, 2025
1 parent b983379 commit cbc225b
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-feet-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[input-amount] make sure that previous locale is not used for parsing on user-edit with <= 2 decimals
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-edited"', async () => {
it('formats with locale when formatOptions.mode is "user-edited" and value has three decimal places', async () => {
const el = /** @type {LionInputAmount} */ (
await fixture(
html`<lion-input-amount
Expand All @@ -154,6 +154,14 @@ describe('<lion-input-amount>', () => {
expect(el.modelValue).to.equal(123456);
expect(el.formattedValue).to.equal('123.456,00');

// When editing an already existing value, we interpet the separators as they are.
// However, only when the decimal places are 3 or more.
mimicUserInput(el, '123.456.00');
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');

// 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)
Expand All @@ -162,6 +170,45 @@ describe('<lion-input-amount>', () => {
expect(formatterSpy.lastCall.args[1]?.mode).to.equal('auto');
});

it('formats with heuristic when formatOptions.mode is "user-edited" and value has two decimal places', async () => {
const el = /** @type {LionInputAmount} */ (
await fixture(
html`<lion-input-amount
.modelValue=${64}
currency="EUR"
.formatOptions="${{ locale: 'en-GB' }}"
></lion-input-amount>`,
)
);
const parserSpy = sinon.spy(el, 'parser');
const formatterSpy = sinon.spy(el, 'formatter');

expect(el.formattedValue).to.equal('64.00');
// @ts-expect-error [allow-protected] in test
expect(el._inputNode.value).to.equal('64.00');

// When editing an already existing value, we interpret the separators based on decimal places when there's 1 or
// less separator in total (otherwise we would accidentally multiply or divide by 1000)
mimicUserInput(el, '64,00');
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(64);
expect(el.formattedValue).to.equal('64.00');

mimicUserInput(el, '64,0');
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(64);
expect(el.formattedValue).to.equal('64.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(`<lion-input-amount></lion-input-amount>`)
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/components/input-amount/test/parsers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,6 @@ describe('parseAmount()', async () => {
expect(parseAmount('123.456,78', { mode: 'auto' })).to.equal(123456.78);
expect(
parseAmount('123.456,78', { mode: 'user-edited', viewValueStates: ['formatted'] }),
).to.equal(123.45678);
).to.equal(123456.78);
});
});
33 changes: 21 additions & 12 deletions packages/ui/components/localize/src/number/parseNumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,34 @@ import { getDecimalSeparator } from './getDecimalSeparator.js';
* @return {string} unparseable|withLocale|heuristic
*/
function getParseMode(value, { mode = 'auto', viewValueStates } = {}) {
const separators = value.match(/[., ]/g);
const separatorsRe = /[., ]/g;
const separators = value.match(separatorsRe) || [];
// @ts-expect-error [wait-for-platform-types]
const decimalPlaces = (value.split(separatorsRe) || []).at(-1).length;
const isCurViewFormattedAndUserIsEditing =
viewValueStates?.includes('formatted') && mode === 'user-edited';

// When a user edits an existing value, we already formatted it with a certain locale.
// For best UX, we stick with this locale
const shouldAlignWithExistingSeparators =
viewValueStates?.includes('formatted') && mode === 'user-edited';
// For best UX, we stick with this locale. However, we only do this when the user has
// entered at least 3 decimal places.
const shouldUseCurLocale = isCurViewFormattedAndUserIsEditing && decimalPlaces > 2;

if (!separators || shouldAlignWithExistingSeparators) {
const shouldUseLocale =
shouldUseCurLocale ||
!separators.length ||
(mode === 'auto' && separators.length === 1 && decimalPlaces >= 3);
if (shouldUseLocale) {
return 'withLocale';
}
if (mode === 'auto' && separators.length === 1) {
const decimalLength = value.split(`${separators}`)[1].length;
if (decimalLength >= 3) {
return 'withLocale';
}
}
if (separators.length === 1 || separators[0] !== separators[separators.length - 1]) {

const shouldUseHeuristic =
(isCurViewFormattedAndUserIsEditing && decimalPlaces <= 2) ||
separators.length === 1 ||
separators[0] !== separators[separators.length - 1];
if (shouldUseHeuristic) {
return 'heuristic';
}

return 'unparseable';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('parseNumber()', () => {
expect(parseNumber('123.456,78', { mode: 'auto' })).to.equal(123456.78);
expect(
parseNumber('123.456,78', { mode: 'user-edited', viewValueStates: ['formatted'] }),
).to.equal(123.45678);
).to.equal(123456.78);
});

it('detects separators unparseable when there are 2 same ones e.g. 1.234.56', () => {
Expand Down

0 comments on commit cbc225b

Please sign in to comment.