Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ const Input = forwardRef<InputRef, InputProps>((props, ref) => {
props.defaultValue,
props.value,
);
// Count-related state should reflect the committed value during IME composition
// instead of the intermediate phonetic text.
const [countValue, setCountValue] = useState(formatValue);
Comment on lines +63 to +65

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Instead of maintaining a separate countValue state and syncing it with formatValue via useEffect, we can derive countValue directly during render. By tracking only the compositionStartValue (the value when IME composition starts), we can simplify the state logic, eliminate the useEffect entirely, and avoid potential double-renders or out-of-sync issues.

Suggested change
// Count-related state should reflect the committed value during IME composition
// instead of the intermediate phonetic text.
const [countValue, setCountValue] = useState(formatValue);
// Track the value at the start of IME composition to reflect the committed value
// during composition instead of the intermediate phonetic text.
const [compositionStartValue, setCompositionStartValue] = useState<string | null>(null);
const countValue = compositionStartValue !== null ? compositionStartValue : formatValue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

看了一下原 issue,感觉这并不是 bug。IME 状态下也是要统计字数的,原生的 validity.tooLong 也是按照输入长度来的。

我把结论 ref 过去,这个不用修。

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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


const countConfig = useCount(count, showCount);
const { isOutOfRange, dataCount } = useCountDisplay({
countConfig,
value: formatValue,
value: countValue,
maxLength,
});
const getExceedValue = useCountExceed({
Expand Down Expand Up @@ -99,6 +102,12 @@ const Input = forwardRef<InputRef, InputProps>((props, ref) => {
setFocused((prev) => (prev && disabled ? false : prev));
}, [disabled]);

useEffect(() => {
if (!compositionRef.current) {
setCountValue(formatValue);
}
}, [formatValue]);
Comment on lines +105 to +109

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

With the derived countValue approach, this useEffect is no longer needed, which prevents unnecessary double-renders when formatValue changes.


const triggerChange = (
e:
| React.ChangeEvent<HTMLInputElement>
Expand All @@ -114,6 +123,9 @@ const Input = forwardRef<InputRef, InputProps>((props, ref) => {
return;
}
setValue(cutValue);
if (!compositionRef.current) {
setCountValue(cutValue);
}
Comment on lines 123 to +128

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Update triggerChange to reset compositionStartValue to null when composition ends. Note that we also reset it before the early return to ensure the count display is correctly updated even if the change event is skipped to avoid duplicate triggers.

Suggested change
return;
}
setValue(cutValue);
if (!compositionRef.current) {
setCountValue(cutValue);
}
setCompositionStartValue(null);
return;
}
setValue(cutValue);
if (!compositionRef.current) {
setCompositionStartValue(null);
}


if (inputRef.current) {
resolveOnChange(inputRef.current, e, onChange, cutValue);
Expand Down Expand Up @@ -227,6 +239,7 @@ const Input = forwardRef<InputRef, InputProps>((props, ref) => {
type={type}
onCompositionStart={(e) => {
compositionRef.current = true;
setCountValue(formatValue);
onCompositionStart?.(e);
}}
Comment on lines 240 to 244

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Set compositionStartValue to the current formatValue when composition starts.

Suggested change
onCompositionStart={(e) => {
compositionRef.current = true;
setCountValue(formatValue);
onCompositionStart?.(e);
}}
onCompositionStart={(e) => {
compositionRef.current = true;
setCompositionStartValue(formatValue);
onCompositionStart?.(e);
}}

onCompositionEnd={onInternalCompositionEnd}
Expand Down
15 changes: 14 additions & 1 deletion src/TextArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,13 @@ const TextArea = React.forwardRef<TextAreaRef, TextAreaProps>(
const getTextArea = () => resizableTextAreaRef.current?.textArea || null;

const { setValue, formatValue } = useMergedValue(defaultValue, customValue);
// Count-related state should reflect the committed value during IME composition
// instead of the intermediate phonetic text.
const [countValue, setCountValue] = React.useState(formatValue);
Comment on lines +63 to +65

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similarly to Input.tsx, we can derive countValue directly during render by tracking only the compositionStartValue (the value when IME composition starts). This simplifies the state logic, eliminates the useEffect entirely, and avoids potential double-renders or out-of-sync issues.

Suggested change
// Count-related state should reflect the committed value during IME composition
// instead of the intermediate phonetic text.
const [countValue, setCountValue] = React.useState(formatValue);
// Track the value at the start of IME composition to reflect the committed value
// during composition instead of the intermediate phonetic text.
const [compositionStartValue, setCompositionStartValue] = React.useState<string | null>(null);
const countValue = compositionStartValue !== null ? compositionStartValue : formatValue;

const countConfig = useCount(count, showCount);
const { isOutOfRange, dataCount } = useCountDisplay({
countConfig,
value: formatValue,
value: countValue,
maxLength,
});
const getExceedValue = useCountExceed({
Expand All @@ -88,6 +91,12 @@ const TextArea = React.forwardRef<TextAreaRef, TextAreaProps>(
setFocused((prev) => !disabled && prev);
}, [disabled]);

useEffect(() => {
if (!compositionRef.current) {
setCountValue(formatValue);
}
}, [formatValue]);
Comment on lines +94 to +98

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

With the derived countValue approach, this useEffect is no longer needed, which prevents unnecessary double-renders when formatValue changes.

Comment on lines +94 to +98

// ============================== Count ===============================
// ============================== Change ==============================
const triggerChange = (
Expand All @@ -98,6 +107,9 @@ const TextArea = React.forwardRef<TextAreaRef, TextAreaProps>(
) => {
const cutValue = getExceedValue(currentValue, compositionRef.current);
setValue(cutValue);
if (!compositionRef.current) {
setCountValue(cutValue);
}
Comment on lines +110 to +112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Update triggerChange to reset compositionStartValue to null when composition ends.

Suggested change
if (!compositionRef.current) {
setCountValue(cutValue);
}
if (!compositionRef.current) {
setCompositionStartValue(null);
}


resolveOnChange(e.currentTarget, e, onChange, cutValue);
};
Expand All @@ -107,6 +119,7 @@ const TextArea = React.forwardRef<TextAreaRef, TextAreaProps>(
HTMLTextAreaElement
> = (e) => {
compositionRef.current = true;
setCountValue(formatValue);
onCompositionStart?.(e);
Comment on lines 121 to 123

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Set compositionStartValue to the current formatValue when composition starts.

Suggested change
compositionRef.current = true;
setCountValue(formatValue);
onCompositionStart?.(e);
compositionRef.current = true;
setCompositionStartValue(formatValue);
onCompositionStart?.(e);

};

Expand Down
22 changes: 22 additions & 0 deletions tests/TextArea.count.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,28 @@ describe('TextArea.Count', () => {
expect(container.querySelector('.rc-textarea-out-of-range')).toBeTruthy();
});

it('should base showCount on committed value during IME composition', () => {
const { container } = render(
<TextArea count={{ show: true, max: 5 }} defaultValue="" />,
);
const textarea = container.querySelector('textarea')!;
const count = container.querySelector('.rc-textarea-data-count');

expect(count?.textContent).toEqual('0 / 5');

fireEvent.compositionStart(textarea);
fireEvent.change(textarea, { target: { value: 'zhu' } });

expect(textarea.value).toEqual('zhu');
expect(count?.textContent).toEqual('0 / 5');

fireEvent.compositionEnd(textarea, { target: { value: '竹' } });
fireEvent.input(textarea, { target: { value: '竹' } });

expect(textarea.value).toEqual('竹');
expect(count?.textContent).toEqual('1 / 5');
});

describe('cls', () => {
it('raw', () => {
const { container } = render(
Expand Down
22 changes: 22 additions & 0 deletions tests/count.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,28 @@ describe('Input.Count', () => {
expect(onChange).toHaveBeenCalledTimes(1);
});

it('should base showCount on committed value during IME composition', () => {
const { container } = render(
<Input count={{ show: true, max: 5 }} defaultValue="" />,
);
const input = container.querySelector('input')!;
const count = container.querySelector('.rc-input-show-count-suffix');

expect(count?.textContent).toEqual('0 / 5');

fireEvent.compositionStart(input);
fireEvent.change(input, { target: { value: 'zhu' } });

expect(input.value).toEqual('zhu');
expect(count?.textContent).toEqual('0 / 5');

fireEvent.compositionEnd(input, { target: { value: '竹' } });
fireEvent.input(input, { target: { value: '竹' } });

expect(input.value).toEqual('竹');
expect(count?.textContent).toEqual('1 / 5');
});

it('using the input method to enter the cropped content should trigger onChange twice', () => {
const onChange = jest.fn();
const onCompositionEnd = jest.fn();
Expand Down
Loading