From a40445cf4d852328a9310a55cf38801e17525476 Mon Sep 17 00:00:00 2001 From: Nikita Barsukov Date: Thu, 17 Oct 2024 15:32:58 +0300 Subject: [PATCH] fix(kit): `Number` fails to prevent user insertion of extra spaces on invalid positions (#1789) --- .../number/alone-decimal-separator.cy.ts | 6 +- .../number/number-thousand-separator.cy.ts | 69 +++++++++++++++++-- .../src/tests/kit/number/utils.ts | 2 +- .../src/app/modules/logo/logo.template.html | 6 -- projects/demo/src/assets/icons/by.svg | 35 ---------- .../kit/src/lib/masks/number/number-mask.ts | 8 ++- .../number/processors/empty-postprocessor.ts | 6 +- .../thousand-separator-postprocessor.ts | 37 +++++++--- .../masks/number/tests/number-mask.spec.ts | 8 +++ .../utils/tests/to-number-parts.spec.ts | 44 +++++------- .../lib/masks/number/utils/to-number-parts.ts | 8 +-- 11 files changed, 139 insertions(+), 90 deletions(-) delete mode 100644 projects/demo/src/assets/icons/by.svg diff --git a/projects/demo-integrations/src/tests/component-testing/number/alone-decimal-separator.cy.ts b/projects/demo-integrations/src/tests/component-testing/number/alone-decimal-separator.cy.ts index 0b0525a88..fd5f77614 100644 --- a/projects/demo-integrations/src/tests/component-testing/number/alone-decimal-separator.cy.ts +++ b/projects/demo-integrations/src/tests/component-testing/number/alone-decimal-separator.cy.ts @@ -42,13 +42,17 @@ describe('Number | should drop decimal separator if all digits are erased', () = .type(`{moveToStart}${'{rightArrow}'.repeat(minusSign.length)}`) .type('{del}') .should('have.value', minusSign) + .should('have.prop', 'selectionStart', minusSign.length) + .should('have.prop', 'selectionEnd', minusSign.length) // and then repeat everything in reversed order .type('0.12') .type(`{moveToStart}${'{rightArrow}'.repeat(minusSign.length)}`) .type('{del}') .type('{moveToEnd}') .type('{backspace}'.repeat(2)) - .should('have.value', minusSign); + .should('have.value', minusSign) + .should('have.prop', 'selectionStart', minusSign.length) + .should('have.prop', 'selectionEnd', minusSign.length); }); }); }); diff --git a/projects/demo-integrations/src/tests/kit/number/number-thousand-separator.cy.ts b/projects/demo-integrations/src/tests/kit/number/number-thousand-separator.cy.ts index bb30dafbb..e594a717e 100644 --- a/projects/demo-integrations/src/tests/kit/number/number-thousand-separator.cy.ts +++ b/projects/demo-integrations/src/tests/kit/number/number-thousand-separator.cy.ts @@ -1,11 +1,11 @@ import {openNumberPage} from './utils'; describe('Number | thousandSeparator', () => { - beforeEach(() => { - openNumberPage('thousandSeparator=-'); - }); - describe('adds thousand separator after pressing new digit', () => { + beforeEach(() => { + openNumberPage('thousandSeparator=-'); + }); + const tests = [ // [Typed value, Masked value] ['1', '1'], @@ -30,6 +30,7 @@ describe('Number | thousandSeparator', () => { describe('move thousand separator after deleting a digit (initial: 1-000-000)', () => { beforeEach(() => { + openNumberPage('thousandSeparator=-'); cy.get('@input').type('1000000'); }); @@ -55,6 +56,10 @@ describe('Number | thousandSeparator', () => { }); describe('Editing somewhere in the middle of a value (NOT the last character)', () => { + beforeEach(() => { + openNumberPage('thousandSeparator=-'); + }); + it('1-00|0-000 => Backspace => 10|0-000 => Type "5" => 1-05|0-000', () => { cy.get('@input') .type('1000000') @@ -138,6 +143,8 @@ describe('Number | thousandSeparator', () => { }); it('allows to set empty string as thousand separator', () => { + openNumberPage('thousandSeparator=-'); + cy.get('tr').contains('[thousandSeparator]').parents('tr').find('input').clear(); cy.get('@input') @@ -146,4 +153,58 @@ describe('Number | thousandSeparator', () => { .should('have.prop', 'selectionStart', '1000000'.length) .should('have.prop', 'selectionEnd', '1000000'.length); }); + + describe('prevent insertion of extra spaces (thousand separator is equal to non-breaking space) on invalid positions', () => { + beforeEach(() => openNumberPage()); + + it('paste value with extra leading and trailing spaces', () => { + cy.get('@input') + .paste(' 123456 ') + .should('have.value', '123 456') + .should('have.prop', 'selectionStart', '123 456'.length) + .should('have.prop', 'selectionEnd', '123 456'.length); + }); + + it('|123 => Press space => |123', () => { + cy.get('@input') + .type('123') + .type('{moveToStart}') + .type(' ') + .should('have.value', '123') + .should('have.prop', 'selectionStart', 0) + .should('have.prop', 'selectionEnd', 0); + }); + + it('1|23 => Press space => 1|23', () => { + cy.get('@input') + .type('123') + .type('{moveToStart}') + .type('{rightArrow}') + .type(' ') + .should('have.value', '123') + .should('have.prop', 'selectionStart', 1) + .should('have.prop', 'selectionEnd', 1); + }); + + it('12|3 => Press space => 12|3', () => { + cy.get('@input') + .type('123') + .type('{moveToStart}') + .type('{rightArrow}'.repeat(2)) + .type(' ') + .should('have.value', '123') + .should('have.prop', 'selectionStart', 2) + .should('have.prop', 'selectionEnd', 2); + }); + + it('123| => Press space => 123|', () => { + cy.get('@input') + .type('123') + .type('{moveToEnd}') + .type(' ') + .should('have.value', '123') + .should('have.prop', 'selectionStart', 3) + .should('have.prop', 'selectionEnd', 3); + }); + }); }); diff --git a/projects/demo-integrations/src/tests/kit/number/utils.ts b/projects/demo-integrations/src/tests/kit/number/utils.ts index e2f55602b..940252e6c 100644 --- a/projects/demo-integrations/src/tests/kit/number/utils.ts +++ b/projects/demo-integrations/src/tests/kit/number/utils.ts @@ -1,6 +1,6 @@ import {DemoPath} from '@demo/constants'; -export function openNumberPage(queryParams: string): void { +export function openNumberPage(queryParams = ''): void { cy.visit(`/${DemoPath.Number}/API?${queryParams}`); cy.get('#demo-content input') .should('be.visible') diff --git a/projects/demo/src/app/modules/logo/logo.template.html b/projects/demo/src/app/modules/logo/logo.template.html index aa0d44116..0aae17594 100644 --- a/projects/demo/src/app/modules/logo/logo.template.html +++ b/projects/demo/src/app/modules/logo/logo.template.html @@ -12,9 +12,3 @@ Maskito - -by T-Bank diff --git a/projects/demo/src/assets/icons/by.svg b/projects/demo/src/assets/icons/by.svg deleted file mode 100644 index 68dec7215..000000000 --- a/projects/demo/src/assets/icons/by.svg +++ /dev/null @@ -1,35 +0,0 @@ - - - - - - - - - - - - - - - - - diff --git a/projects/kit/src/lib/masks/number/number-mask.ts b/projects/kit/src/lib/masks/number/number-mask.ts index 40f774ebf..29c0418c6 100644 --- a/projects/kit/src/lib/masks/number/number-mask.ts +++ b/projects/kit/src/lib/masks/number/number-mask.ts @@ -144,6 +144,7 @@ export function maskitoNumberOptionsGenerator({ thousandSeparator, prefix, postfix, + minusSign, }), createDecimalZeroPaddingPostprocessor({ decimalSeparator, @@ -152,7 +153,12 @@ export function maskitoNumberOptionsGenerator({ prefix, postfix, }), - emptyPostprocessor({prefix, postfix, decimalSeparator, thousandSeparator}), + emptyPostprocessor({ + prefix, + postfix, + decimalSeparator, + minusSign, + }), ], plugins: [ createLeadingZeroesValidationPlugin({ diff --git a/projects/kit/src/lib/masks/number/processors/empty-postprocessor.ts b/projects/kit/src/lib/masks/number/processors/empty-postprocessor.ts index 6e3f1b8dd..1c4f7b1a4 100644 --- a/projects/kit/src/lib/masks/number/processors/empty-postprocessor.ts +++ b/projects/kit/src/lib/masks/number/processors/empty-postprocessor.ts @@ -13,12 +13,12 @@ export function emptyPostprocessor({ prefix, postfix, decimalSeparator, - thousandSeparator, + minusSign, }: { prefix: string; postfix: string; decimalSeparator: string; - thousandSeparator: string; + minusSign: string; }): MaskitoPostprocessor { return ({value, selection}) => { const [caretIndex] = selection; @@ -28,7 +28,7 @@ export function emptyPostprocessor({ }); const {minus, integerPart, decimalPart} = toNumberParts(cleanValue, { decimalSeparator, - thousandSeparator, + minusSign, }); const aloneDecimalSeparator = !integerPart && !decimalPart && cleanValue.includes(decimalSeparator); diff --git a/projects/kit/src/lib/masks/number/processors/thousand-separator-postprocessor.ts b/projects/kit/src/lib/masks/number/processors/thousand-separator-postprocessor.ts index b53eb1e40..85ddd29cf 100644 --- a/projects/kit/src/lib/masks/number/processors/thousand-separator-postprocessor.ts +++ b/projects/kit/src/lib/masks/number/processors/thousand-separator-postprocessor.ts @@ -12,11 +12,13 @@ export function createThousandSeparatorPostprocessor({ decimalSeparator, prefix, postfix, + minusSign, }: { thousandSeparator: string; decimalSeparator: string; prefix: string; postfix: string; + minusSign: string; }): MaskitoPostprocessor { if (!thousandSeparator) { return identity; @@ -25,6 +27,9 @@ export function createThousandSeparatorPostprocessor({ const isAllSpaces = (...chars: string[]): boolean => chars.every((x) => /\s/.test(x)); return ({value, selection}) => { + const [initialFrom, initialTo] = selection; + let [from, to] = selection; + const {cleanValue, extractedPostfix, extractedPrefix} = extractAffixes(value, { prefix, postfix, @@ -32,27 +37,41 @@ export function createThousandSeparatorPostprocessor({ const {minus, integerPart, decimalPart} = toNumberParts(cleanValue, { decimalSeparator, - thousandSeparator, + minusSign, }); - const [initialFrom, initialTo] = selection; - let [from, to] = selection; + const deletedChars = + cleanValue.length - + ( + minus + + integerPart + + (cleanValue.includes(decimalSeparator) + ? decimalSeparator + decimalPart + : '') + ).length; + + if (deletedChars > 0 && initialFrom && initialFrom <= deletedChars) { + from -= deletedChars; + } + + if (deletedChars > 0 && initialTo && initialTo <= deletedChars) { + to -= deletedChars; + } const processedIntegerPart = Array.from(integerPart).reduceRight( (formattedValuePart, char, i) => { const isLeadingThousandSeparator = !i && char === thousandSeparator; const isPositionForSeparator = !isLeadingThousandSeparator && - formattedValuePart.length && + Boolean(formattedValuePart.length) && (formattedValuePart.length + 1) % 4 === 0; + const isSeparator = + char === thousandSeparator || isAllSpaces(char, thousandSeparator); - if ( - isPositionForSeparator && - (char === thousandSeparator || isAllSpaces(char, thousandSeparator)) - ) { + if (isPositionForSeparator && isSeparator) { return thousandSeparator + formattedValuePart; } - if (char === thousandSeparator && !isPositionForSeparator) { + if (!isPositionForSeparator && isSeparator) { if (i && i <= initialFrom) { from--; } diff --git a/projects/kit/src/lib/masks/number/tests/number-mask.spec.ts b/projects/kit/src/lib/masks/number/tests/number-mask.spec.ts index 57a3989ff..00023ae12 100644 --- a/projects/kit/src/lib/masks/number/tests/number-mask.spec.ts +++ b/projects/kit/src/lib/masks/number/tests/number-mask.spec.ts @@ -397,4 +397,12 @@ describe('Number (maskitoTransform)', () => { }); }); }); + + it('autofill value with extra leading and trailing whitespace (thousand separator is equal to whitespace too)', () => { + const options = maskitoNumberOptionsGenerator({ + thousandSeparator: ' ', + }); + + expect(maskitoTransform(' 123456 ', options)).toBe('123 456'); + }); }); diff --git a/projects/kit/src/lib/masks/number/utils/tests/to-number-parts.spec.ts b/projects/kit/src/lib/masks/number/utils/tests/to-number-parts.spec.ts index ed1ca5c1a..0a65e69b7 100644 --- a/projects/kit/src/lib/masks/number/utils/tests/to-number-parts.spec.ts +++ b/projects/kit/src/lib/masks/number/utils/tests/to-number-parts.spec.ts @@ -12,10 +12,10 @@ import {toNumberParts} from '../to-number-parts'; describe('toNumberParts', () => { [',', '.'].forEach((decimalSeparator) => { describe(`decimalSeparator = ${decimalSeparator}`, () => { - const thousandSeparator = '_'; + const minusSign = '-'; it('empty string => empty parts', () => { - expect(toNumberParts('', {decimalSeparator, thousandSeparator})).toEqual({ + expect(toNumberParts('', {decimalSeparator, minusSign})).toEqual({ minus: '', integerPart: '', decimalPart: '', @@ -26,7 +26,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`123${decimalSeparator}45`, { decimalSeparator, - thousandSeparator, + minusSign, }), ).toEqual({ minus: '', @@ -39,7 +39,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`-123${decimalSeparator}45`, { decimalSeparator, - thousandSeparator, + minusSign, }), ).toEqual({ minus: '-', @@ -49,9 +49,7 @@ describe('toNumberParts', () => { }); it('123 => {minus: "", integerPart: "123", decimalPart: ""}', () => { - expect( - toNumberParts('123', {decimalSeparator, thousandSeparator}), - ).toEqual({ + expect(toNumberParts('123', {decimalSeparator, minusSign})).toEqual({ minus: '', integerPart: '123', decimalPart: '', @@ -59,9 +57,7 @@ describe('toNumberParts', () => { }); it('-123 => {minus: "-", integerPart: "123", decimalPart: ""}', () => { - expect( - toNumberParts('-123', {decimalSeparator, thousandSeparator}), - ).toEqual({ + expect(toNumberParts('-123', {decimalSeparator, minusSign})).toEqual({ minus: '-', integerPart: '123', decimalPart: '', @@ -72,7 +68,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`${decimalSeparator}45`, { decimalSeparator, - thousandSeparator, + minusSign, }), ).toEqual({ minus: '', @@ -85,7 +81,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`-${decimalSeparator}45`, { decimalSeparator, - thousandSeparator, + minusSign, }), ).toEqual({ minus: '-', @@ -95,13 +91,11 @@ describe('toNumberParts', () => { }); it('- => {minus: "-", integerPart: "", decimalPart: ""}', () => { - expect(toNumberParts('-', {decimalSeparator, thousandSeparator})).toEqual( - { - minus: '-', - integerPart: '', - decimalPart: '', - }, - ); + expect(toNumberParts('-', {decimalSeparator, minusSign})).toEqual({ + minus: '-', + integerPart: '', + decimalPart: '', + }); }); }); }); @@ -112,7 +106,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`${minus}1,234,567.89`, { decimalSeparator: '.', - thousandSeparator: ',', + minusSign: minus, }), ).toEqual({ minus, @@ -129,10 +123,10 @@ describe('toNumberParts', () => { it('only thousand separator sign', () => { expect( - toNumberParts(thousandSeparator, {decimalSeparator, thousandSeparator}), + toNumberParts(thousandSeparator, {decimalSeparator, minusSign: '-'}), ).toEqual({ minus: '', - integerPart: thousandSeparator, + integerPart: '', decimalPart: '', }); }); @@ -141,7 +135,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`-${thousandSeparator}`, { decimalSeparator, - thousandSeparator, + minusSign: '-', }), ).toEqual({ minus: '-', @@ -154,7 +148,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`-1${thousandSeparator}234.45`, { decimalSeparator, - thousandSeparator, + minusSign: '-', }), ).toEqual({ minus: '-', @@ -167,7 +161,7 @@ describe('toNumberParts', () => { expect( toNumberParts(`-${thousandSeparator}234.45`, { decimalSeparator, - thousandSeparator, + minusSign: '-', }), ).toEqual({ minus: '-', diff --git a/projects/kit/src/lib/masks/number/utils/to-number-parts.ts b/projects/kit/src/lib/masks/number/utils/to-number-parts.ts index 3a793f51d..fd38efeb8 100644 --- a/projects/kit/src/lib/masks/number/utils/to-number-parts.ts +++ b/projects/kit/src/lib/masks/number/utils/to-number-parts.ts @@ -2,14 +2,12 @@ import {escapeRegExp} from '../../../utils'; export function toNumberParts( value: string, - { - decimalSeparator, - thousandSeparator, - }: {decimalSeparator: string; thousandSeparator: string}, + {decimalSeparator, minusSign}: {decimalSeparator: string; minusSign: string}, ): {minus: string; integerPart: string; decimalPart: string} { const [integerWithMinus = '', decimalPart = ''] = value.split(decimalSeparator); + const escapedMinus = escapeRegExp(minusSign); const [, minus = '', integerPart = ''] = - new RegExp(`([^\\d${escapeRegExp(thousandSeparator)}]+)?(.*)`).exec( + new RegExp(`^(?:[^\\d${escapedMinus}])?(${escapedMinus})?(.*)`).exec( integerWithMinus, ) || [];