Skip to content

Commit

Permalink
- Update test dependencies
Browse files Browse the repository at this point in the history
- Added better way to find change range which fixes  issue around duplicate subsequent characters giving incorrect change range.
- Remove findChangedIndex util, as it wasn't used anywhere.
- Fix simulateKeyInput util
- Fix incorrect test
- Fix issue when complete text is selected and replaced with new value in pattern format. It was it was incorrectly pasting value.
  • Loading branch information
s-yadav committed May 12, 2024
1 parent 8c0b056 commit a44af92
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 413 deletions.
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"editor.formatOnSave": true
}
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@
"@mdx-js/react": "^1.6.22",
"@rollup/plugin-buble": "^0.21.3",
"@rollup/plugin-typescript": "^8.3.0",
"@testing-library/jest-dom": "^6.0.0",
"@testing-library/react": "^14.0.0",
"@testing-library/user-event": "^14.0.0",
"@testing-library/jest-dom": "^6.4.5",
"@testing-library/react": "^15.0.7",
"@testing-library/user-event": "^14.5.2",
"@types/react": "^18.0.6",
"@typescript-eslint/parser": "^5.15.0",
"babel-eslint": "^10.0.3",
Expand Down Expand Up @@ -92,7 +92,7 @@
"rollup-plugin-uglify": "^6.0.3",
"ts-loader": "^9.2.6",
"typescript": "^4.6.3",
"vitest": "^1.0.0",
"vitest": "^1.6.0",
"webpack": "^5.69.1",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "^4.7.4"
Expand Down
38 changes: 30 additions & 8 deletions src/number_format_base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
noop,
caretUnknownFormatBoundary,
getCaretPosInBoundary,
findChangedRangeFromCaretPositions,
} from './utils';

function defaultRemoveFormatting(value: string) {
Expand Down Expand Up @@ -63,6 +64,8 @@ export default function NumberFormatBase<BaseType = InputAttributes>(
onValueChange,
);

const caretPositionBeforeChange = useRef<{ selectionStart: number; selectionEnd: number }>();

const lastUpdatedValue = useRef({ formattedValue, numAsString });

const _onValueChange: NumberFormatBaseProps['onValueChange'] = (values, source) => {
Expand Down Expand Up @@ -240,7 +243,12 @@ export default function NumberFormatBase<BaseType = InputAttributes>(
| React.KeyboardEvent<HTMLInputElement>,
source: SourceType,
) => {
const changeRange = findChangeRange(formattedValue, inputValue);
const input = event.target as HTMLInputElement;

const changeRange = caretPositionBeforeChange.current
? findChangedRangeFromCaretPositions(caretPositionBeforeChange.current, input.selectionEnd)
: findChangeRange(formattedValue, inputValue);

const changeMeta = {
...changeRange,
lastValue: formattedValue,
Expand Down Expand Up @@ -274,13 +282,21 @@ export default function NumberFormatBase<BaseType = InputAttributes>(
return true;
};

const setCaretPositionInfoBeforeChange = (el: HTMLInputElement, endOffset: number = 0) => {
const { selectionStart, selectionEnd } = el;
caretPositionBeforeChange.current = { selectionStart, selectionEnd: selectionEnd + endOffset };
};

const _onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const el = e.target;
const inputValue = el.value;

const changed = formatInputValue(inputValue, e, SourceType.event);

if (changed) onChange(e);

// reset the position, as we have already handled the caret position
caretPositionBeforeChange.current = undefined;
};

const _onKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
Expand All @@ -299,10 +315,20 @@ export default function NumberFormatBase<BaseType = InputAttributes>(
expectedCaretPosition = selectionStart;
}

// if key is delete and text is not selected keep the end offset to 1, as it deletes one character
// this is required as selection is not changed on delete case, which changes the change range calculation
let endOffset = 0;
if (key === 'Delete' && selectionStart === selectionEnd) {
endOffset = 1;
}

//if expectedCaretPosition is not set it means we don't want to Handle keyDown
// also if multiple characters are selected don't handle
if (expectedCaretPosition === undefined || selectionStart !== selectionEnd) {
onKeyDown(e);
// keep information of what was the caret position before keyDown
// set it after onKeyDown, in case parent updates the position manually
setCaretPositionInfoBeforeChange(el, endOffset);
return;
}

Expand All @@ -327,14 +353,9 @@ export default function NumberFormatBase<BaseType = InputAttributes>(
setPatchedCaretPosition(el, newCaretPosition, value);
}

/* NOTE: this is just required for unit test as we need to get the newCaretPosition,
Remove this when you find different solution */
/* @ts-ignore */
if (e.isUnitTestRun) {
setPatchedCaretPosition(el, newCaretPosition, value);
}

onKeyDown(e);

setCaretPositionInfoBeforeChange(el, endOffset);
};

/** required to handle the caret position when click anywhere within the input **/
Expand Down Expand Up @@ -365,6 +386,7 @@ export default function NumberFormatBase<BaseType = InputAttributes>(
});

onMouseUp(e);
setCaretPositionInfoBeforeChange(el);
};

const _onFocus = (e: React.FocusEvent<HTMLInputElement>) => {
Expand Down
6 changes: 6 additions & 0 deletions src/numeric_format.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,12 @@ export function useNumericFormat<BaseType = InputAttributes>(
const { key } = e;
const { selectionStart, selectionEnd, value = '' } = el;

// if user tries to delete partial prefix then ignore it
if ((key === 'Backspace' || key === 'Delete') && selectionEnd < prefix.length) {
e.preventDefault();
return;
}

// if multiple characters are selected and user hits backspace, no need to handle anything manually
if (selectionStart !== selectionEnd) {
onKeyDown(e);
Expand Down
7 changes: 5 additions & 2 deletions src/pattern_format.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@ export function removeFormatting<BaseType = InputAttributes>(
}

/**
* if user paste the whole formatted text in an empty input, check if matches to the pattern
* if user paste the whole formatted text in an empty input or doing select all and paste, check if matches to the pattern
* and remove the format characters, if there is a mismatch on the pattern, do plane number extract
*/
if (lastValue === '' && value.length === format.length) {
if (
(lastValue === '' || from.end - from.start === lastValue.length) &&
value.length === format.length
) {
let str = '';
for (let i = 0; i < value.length; i++) {
if (isNumericSlot(i)) {
Expand Down
41 changes: 18 additions & 23 deletions src/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -246,29 +246,13 @@ export function setCaretPosition(el: HTMLInputElement, caretPos: number) {
}

/**
Given previous value and newValue it returns the index
start - end to which values have changed.
This function makes assumption about only consecutive
characters are changed which is correct assumption for caret input.
*/
export function findChangedIndex(prevValue: string, newValue: string) {
let i = 0,
j = 0;
const prevLength = prevValue.length;
const newLength = newValue.length;
while (prevValue[i] === newValue[i] && i < prevLength) i++;

//check what has been changed from last
while (
prevValue[prevLength - 1 - j] === newValue[newLength - 1 - j] &&
newLength - j > i &&
prevLength - j > i
) {
j++;
}

return { start: i, end: prevLength - j };
}
* TODO: remove dependency of findChangeRange, findChangedRangeFromCaretPositions is better way to find what is changed
* currently this is mostly required by test and isCharacterSame util
* Given previous value and newValue it returns the index
* start - end to which values have changed.
* This function makes assumption about only consecutive
* characters are changed which is correct assumption for caret input.
*/

export const findChangeRange = memoizeOnce((prevValue: string, newValue: string) => {
let i = 0,
Expand All @@ -292,6 +276,17 @@ export const findChangeRange = memoizeOnce((prevValue: string, newValue: string)
};
});

export const findChangedRangeFromCaretPositions = (
lastCaretPositions: { selectionStart: number; selectionEnd: number },
currentCaretPosition: number,
) => {
const startPosition = Math.min(lastCaretPositions.selectionStart, currentCaretPosition);
return {
from: { start: startPosition, end: lastCaretPositions.selectionEnd },
to: { start: startPosition, end: currentCaretPosition },
};
};

/*
Returns a number whose value is limited to the given range
*/
Expand Down
4 changes: 2 additions & 2 deletions test/library/input.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -521,10 +521,10 @@ describe('NumberFormat as input', () => {
source: 'prop',
});

await simulateKeyInput(user, input, '5', 0, 0, { eventType: 'keyboard' });
await simulateKeyInput(user, input, '5', 0, 0);

const { event, source } = spy.mock.lastCall[1];
expect(event.nativeEvent.data).toEqual('5');
expect(event.type).toEqual('change');
expect(source).toEqual('event');
});

Expand Down
72 changes: 29 additions & 43 deletions test/library/input_numeric_format.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
simulateKeyInput,
clearInput,
simulateDblClick,
simulateDragMouseToSelect,
simulatePaste,
} from '../test_util';

/**
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('Test NumberFormat as input with numeric format options', () => {
});

it('should allow floating/integer numbers as values and do proper formatting', async () => {
const { input, user, rerender } = await render(<NumericFormat value={12345.67} />);
const { input, rerender } = await render(<NumericFormat value={12345.67} />);
expect(input).toHaveValue('12345.67');

rerender(<NumericFormat value={12345.67} thousandSeparator />);
Expand Down Expand Up @@ -372,7 +372,7 @@ describe('Test NumberFormat as input with numeric format options', () => {
thousandSeparator={true}
decimalScale={2}
fixedDecimalScale={true}
value={'1,234.00'}
value={'$1,234.00'}
/>,
);
await simulateKeyInput(user, input, '56', 1, 9);
Expand All @@ -388,7 +388,7 @@ describe('Test NumberFormat as input with numeric format options', () => {
value={'98.76%'}
/>,
);
await simulateKeyInput(user, input, '1', 0, 5);
await simulateKeyInput(user, input, '1', 0, 6);
expect(input).toHaveValue('1.00%');

rerender(
Expand Down Expand Up @@ -658,16 +658,12 @@ describe('Test NumberFormat as input with numeric format options', () => {
});

//Issue #145
it.todo(
'should give correct formatted value when pasting a number with decimal and decimal scale is set to zero, issue #145',
async () => {
// TODO: Incorrect assertion
const { input, user } = await render(<NumericFormat decimalScale={0} />);
await simulateKeyInput(user, input, '9.55');
simulateBlurEvent(input);
expect(input).toHaveValue('9');
},
);
it('should give correct formatted value when pasting a number with decimal and decimal scale is set to zero, issue #145', async () => {
const { input, user } = await render(<NumericFormat decimalScale={0} />);
await simulatePaste(user, input, '9.55');
simulateBlurEvent(input);
expect(input).toHaveValue('9');
});

it('should format the number correctly when thousandSeparator is true and decimal scale is 0. Issue #178', async () => {
const { input, user } = await render(
Expand All @@ -694,27 +690,24 @@ describe('Test NumberFormat as input with numeric format options', () => {
expect(input).toHaveValue('-');
});

it.todo(
'should allow typing . as decimal separator when some other decimal separator is given',
async () => {
const { input, user } = await render(
<NumericFormat
decimalSeparator=","
thousandSeparator="."
value="234.456"
suffix=" Sq. ft"
/>,
);
await simulateKeyInput(user, input, '.', 5);
expect(input).toHaveValue('2.344,56 Sq. ft');
//check if caret position is maintained
expect(input.selectionStart).toEqual(6);
it('should allow typing . as decimal separator when some other decimal separator is given', async () => {
const { input, user } = await render(
<NumericFormat
decimalSeparator=","
thousandSeparator="."
value="234.456 Sq. ft"
suffix=" Sq. ft"
/>,
);
await simulateKeyInput(user, input, '.', 5, 5);
expect(input).toHaveValue('2.344,56 Sq. ft');
//check if caret position is maintained
expect(input.selectionStart).toEqual(6);

//it should allow typing actual separator
await simulateKeyInput(user, input, ',', 3, 3);
expect(input).toHaveValue('23,4456 Sq. ft');
},
);
//it should allow typing actual separator
await simulateKeyInput(user, input, ',', 3, 3);
expect(input).toHaveValue('23,4456 Sq. ft');
});

it('should not break if suffix/prefix has negation sign. Issue #245', async () => {
const { input, user } = await render(<NumericFormat suffix="-" />);
Expand Down Expand Up @@ -828,23 +821,16 @@ describe('Test NumberFormat as input with numeric format options', () => {
expect(input).toHaveValue('100-1000 USD');
});

it.todo('while deleting prefix', async () => {
it('while deleting prefix', async () => {
const { input, user } = await render(
<NumericFormat prefix="100-" value={1} suffix="000 USD" />,
);

// TODO: simulateKeyInput isn't actually typing anything into the input.
// It computes the expected value of the input and updates it.
// However, for what it's worth, this assertion also fails in the browser as
// of now.

// await simulateDragMouseToSelect(user, input, 0, 4)
// await simulateKeyInput(user, input, '{Backspace}');
await simulateKeyInput(user, input, '{Backspace}', 0, 4);
expect(input).toHaveValue('100-1000 USD');
});

it.todo('while deleting part of the prefix', async () => {
it('while deleting part of the prefix', async () => {
const { input, user } = await render(
<NumericFormat prefix="100-" value={1} suffix="000 USD" />,
);
Expand Down
6 changes: 3 additions & 3 deletions test/library/keypress_and_caret.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ describe('Test keypress and caret position changes', () => {
/>,
);

// backspace after thousand separator separator
// // backspace after thousand separator separator
await simulateKeyInput(user, input, '{Backspace}', 7, 7);
expect(input).toHaveValue('Rs. 1,345.50 /sq.feet');
expect(input.selectionStart).toEqual(5);
Expand All @@ -431,8 +431,8 @@ describe('Test keypress and caret position changes', () => {
expect(input.selectionStart).toEqual(8);

// delete before decimal separator
await simulateKeyInput(user, input, '.', 8, 8, { eventType: 'keyboard' });
await simulateKeyInput(user, input, '{Delete}', 7, 7, { eventType: 'keyboard' });
await simulateKeyInput(user, input, '.', 8, 8);
await simulateKeyInput(user, input, '{Delete}', 7, 7);
expect(input).toHaveValue('Rs. 14,550 /sq.feet');
expect(input.selectionStart).toEqual(8);
});
Expand Down
Loading

0 comments on commit a44af92

Please sign in to comment.