Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Validator callback and provide prevValue in Parser callback #644

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
43 changes: 29 additions & 14 deletions src/InputNumber.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,14 @@ export interface InputNumberProps<T extends ValueType = ValueType>
changeOnWheel?: boolean;

/** Parse display value to validate number */
parser?: (displayValue: string | undefined) => T;
parser?: (displayValue: string | undefined, info: { prevValue: string }) => T;
/** Transform `value` to display value show in input */
formatter?: (value: T | undefined, info: { userTyping: boolean; input: string }) => string;
formatter?: (
value: T | undefined,
info: { userTyping: boolean; input: string; prevValue: string },
) => string;
/** Validate an input string before processing */
validator?: (input: string) => boolean;
Comment on lines +90 to +97
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

注意:parserformatter 方法签名的更改可能引入破坏性变更

InputNumberProps 接口中为 parserformatter 方法添加新的参数 info,可能会导致现有使用这些属性的代码出现 TypeScript 类型错误,因为现有实现可能未预期新的 info 参数。建议将新的 info 参数设为可选,以保持向后兼容性。

应用以下代码修改:

- parser?: (displayValue: string | undefined, info: { prevValue: string }) => T;
+ parser?: (displayValue: string | undefined, info?: { prevValue: string }) => T;
- formatter?: (
-   value: T | undefined,
-   info: { userTyping: boolean; input: string; prevValue: string },
- ) => string;
+ formatter?: (
+   value: T | undefined,
+   info?: { userTyping: boolean; input: string; prevValue: string },
+ ) => string;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
parser?: (displayValue: string | undefined, info: { prevValue: string }) => T;
/** Transform `value` to display value show in input */
formatter?: (value: T | undefined, info: { userTyping: boolean; input: string }) => string;
formatter?: (
value: T | undefined,
info: { userTyping: boolean; input: string; prevValue: string },
) => string;
/** Validate an input string before processing */
validator?: (input: string) => boolean;
parser?: (displayValue: string | undefined, info?: { prevValue: string }) => T;
/** Transform `value` to display value show in input */
formatter?: (
value: T | undefined,
info?: { userTyping: boolean; input: string; prevValue: string },
) => string;
/** Validate an input string before processing */
validator?: (input: string) => boolean;

/** Syntactic sugar of `formatter`. Config precision of display. */
precision?: number;
/** Syntactic sugar of `formatter`. Config decimal separator of display. */
Expand Down Expand Up @@ -128,24 +133,18 @@ const InternalInputNumber = React.forwardRef(
keyboard,
changeOnWheel = false,
controls = true,

classNames,
stringMode,

validator,
parser,
formatter,
precision,
decimalSeparator,

onChange,
onInput,
onPressEnter,
onStep,

changeOnBlur = true,

domRef,

...inputProps
} = props;

Expand All @@ -171,7 +170,10 @@ const InternalInputNumber = React.forwardRef(
}
}

// ====================== Parser & Formatter ======================
const prevValueRef = React.useRef<string | number>('');
const inputValueRef = React.useRef<string | number>('');
bombillazo marked this conversation as resolved.
Show resolved Hide resolved

// ====================== Formatter ======================
/**
* `precision` is used for formatter & onChange.
* It will auto generate by `value` & `step`.
Expand Down Expand Up @@ -204,7 +206,7 @@ const InternalInputNumber = React.forwardRef(
const numStr = String(num);

if (parser) {
return parser(numStr);
return parser(numStr, { prevValue: String(prevValueRef.current ?? '') });
}

let parsedStr = numStr;
Expand All @@ -219,11 +221,14 @@ const InternalInputNumber = React.forwardRef(
);

// >>> Formatter
const inputValueRef = React.useRef<string | number>('');
const mergedFormatter = React.useCallback(
(number: string, userTyping: boolean) => {
if (formatter) {
return formatter(number, { userTyping, input: String(inputValueRef.current) });
return formatter(number, {
userTyping,
input: String(inputValueRef.current),
prevValue: String(prevValueRef.current ?? ''),
});
}

let str = typeof number === 'number' ? num2str(number) : number;
Expand Down Expand Up @@ -262,7 +267,11 @@ const InternalInputNumber = React.forwardRef(
}
return mergedFormatter(decimalValue.toString(), false);
});
inputValueRef.current = inputValue;

React.useEffect(() => {
prevValueRef.current = inputValueRef.current;
inputValueRef.current = inputValue;
}, [inputValue]);

// Should always be string
function setInputValue(newValue: DecimalClass, userTyping: boolean) {
Expand Down Expand Up @@ -380,10 +389,16 @@ const InternalInputNumber = React.forwardRef(

// >>> Collect input value
const collectInputValue = (inputStr: string) => {
// validate string
if (validator) {
if (!validator(inputStr)) return;
}
bombillazo marked this conversation as resolved.
Show resolved Hide resolved

recordCursor();

// Update inputValue in case input can not parse as number
// Refresh ref value immediately since it may used by formatter
prevValueRef.current = inputValueRef.current;
inputValueRef.current = inputStr;
setInternalInputValue(inputStr);

Expand Down
2 changes: 1 addition & 1 deletion tests/formatter.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe('InputNumber.Formatter', () => {

fireEvent.change(container.querySelector('input'), { target: { value: '1' } });
expect(formatter).toHaveBeenCalledTimes(1);
expect(formatter).toHaveBeenCalledWith('1', { userTyping: true, input: '1' });
expect(formatter).toHaveBeenCalledWith('1', { userTyping: true, input: '1', prevValue: '' });
});

describe('dynamic formatter', () => {
Expand Down
84 changes: 84 additions & 0 deletions tests/validator.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import KeyCode from 'rc-util/lib/KeyCode';
import InputNumber from '../src';
import { fireEvent, render } from './util/wrapper';

describe('InputNumber.validator', () => {
it('validator on direct input', () => {
const onChange = jest.fn();
const { container } = render(
<InputNumber
defaultValue={0}
validator={(num) => {
return /^[0-9]*$/.test(num);
}}
onChange={onChange}
/>,
);
const input = container.querySelector('input');
fireEvent.focus(input);

fireEvent.change(input, { target: { value: 'a' } });
expect(input.value).toEqual('0');
fireEvent.change(input, { target: { value: '5' } });
expect(input.value).toEqual('5');
expect(onChange).toHaveBeenCalledWith(5);
fireEvent.change(input, { target: { value: '10e' } });
expect(input.value).toEqual('5');
fireEvent.change(input, { target: { value: '_' } });
expect(input.value).toEqual('5');
fireEvent.change(input, { target: { value: '10' } });
expect(input.value).toEqual('10');
expect(onChange).toHaveBeenCalledWith(10);
});

it('validator and formatter', () => {
const onChange = jest.fn();
const { container } = render(
<InputNumber
defaultValue={1}
formatter={(num) => `$ ${num} boeing 737`}
validator={(num) => {
return /^[0-9]*$/.test(num);
}}
onChange={onChange}
/>,
);
const input = container.querySelector('input');
fireEvent.focus(input);

expect(input.value).toEqual('$ 1 boeing 737');
fireEvent.change(input, { target: { value: '5' } });
expect(input.value).toEqual('$ 5 boeing 737');

fireEvent.keyDown(input, {
which: KeyCode.UP,
key: 'ArrowUp',
code: 'ArrowUp',
keyCode: KeyCode.UP,
});

expect(input.value).toEqual('$ 6 boeing 737');
expect(onChange).toHaveBeenLastCalledWith(6);

fireEvent.change(input, { target: { value: '#' } });
expect(input.value).toEqual('$ 6 boeing 737');

fireEvent.keyDown(input, {
which: KeyCode.DOWN,
key: 'ArrowDown',
code: 'ArrowDown',
keyCode: KeyCode.DOWN,
});

expect(input.value).toEqual('$ 5 boeing 737');
expect(onChange).toHaveBeenLastCalledWith(5);

fireEvent.mouseDown(container.querySelector('.rc-input-number-handler-up'), {
which: KeyCode.DOWN,
});
expect(input.value).toEqual('$ 6 boeing 737');
expect(onChange).toHaveBeenLastCalledWith(6);
fireEvent.change(input, { target: { value: 'a' } });
expect(input.value).toEqual('$ 6 boeing 737');
});
bombillazo marked this conversation as resolved.
Show resolved Hide resolved
});