-
Notifications
You must be signed in to change notification settings - Fork 8.4k
fix: Support IME composition for accented characters in Input component #11340
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
base: main
Are you sure you want to change the base?
fix: Support IME composition for accented characters in Input component #11340
Conversation
Fixes langflow-ai#11293 Changed error response key from 'message' to 'detail' in SideBarFoldersButtonComponent to properly display error messages from the backend API when project upload fails. Co-Authored-By: Claude <noreply@anthropic.com>
Fixes langflow-ai#10460 Changed CustomLink to a button with onClick handler that properly: 1. Closes the modal 2. Navigates to the MCP Servers settings page This resolves the issue where clicking the "settings" hyperlink in the Add MCP Server dialog did nothing. Co-Authored-By: Claude <noreply@anthropic.com>
Fixes langflow-ai#9096 On Linux with IBus/dead keys, the controlled input was updating its value on every onChange, which interrupted character composition (where the accent precedes the letter). This caused accents to appear separately. Changes: - Added composition state tracking (isComposing) - Use local text buffer during composition - Only propagate value when not composing and in onCompositionEnd - Normalize text to NFC at end of composition This allows proper accent input on Linux systems using IME. Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughChanges address three areas: IME composition event handling in the Input component to fix accent input behavior, error field name correction in folder upload error handling, and refactoring modal navigation from CustomLink to programmatic routing via useNavigate hook. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 4 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/src/components/core/folderSidebarComponent/components/sideBarFolderButtons/index.tsx (1)
157-162: Provide a safe fallback whendetailis missing.If the backend doesn’t include
detail, the alert can showundefinedor throw if the shape differs. Consider a safe fallback tomessageor a generic string.✅ Suggested guard
- setErrorData({ - title: `Error on uploading your project, try dragging it into an existing project.`, - list: [err["response"]["data"]["detail"]], - }); + setErrorData({ + title: `Error on uploading your project, try dragging it into an existing project.`, + list: [ + err?.response?.data?.detail ?? + err?.response?.data?.message ?? + "Upload failed.", + ], + });
🤖 Fix all issues with AI agents
In `@src/frontend/src/components/ui/input.tsx`:
- Around line 24-42: The component only syncs string controlled values into
internalValue causing numeric controlled inputs to appear blank; update the
initialization and the React.useEffect sync to normalize non-null/non-undefined
values to strings (use String(value) when typeof value !== "string" and value !=
null) and keep using isComposing to avoid clobbering in-progress IME input;
specifically adjust the internalValue initial state and the effect that calls
setInternalValue (referencing value, setInternalValue, internalValue,
React.useEffect, and isComposing) so numbers and other primitive controlled
values render correctly as text.
- Around line 44-77: The IME composition flow can cause onChange to fire twice
because isComposing state may not update in time; add a ref-based guard (e.g.,
skipNextOnChangeRef) and use it in handleChange and handleCompositionEnd: in
handleCompositionEnd set skipNextOnChangeRef.current = true before calling the
synthetic onChange, then in handleChange return early if
skipNextOnChangeRef.current is true and reset it to false; keep existing
setIsComposing calls in handleCompositionStart/handleCompositionEnd but rely on
the ref for the immediate skip of the next onChange to avoid duplicate
invocations.
In `@src/frontend/src/modals/addMcpServerModal/index.tsx`:
- Around line 292-300: The "settings" button inside the Form.Root is currently
acting as a submit because it lacks an explicit type; update the button rendered
in the addMcpServerModal (the element that calls setOpen(false) and
navigate("/settings/mcp-servers")) to include type="button" so it does not
trigger the Form.Root onSubmit (submitForm). This change should be made in the
callback that closes the modal and navigates to settings to prevent unintended
submitForm execution.
| value, | ||
| onChange, | ||
| ...props | ||
| }, | ||
| ref, | ||
| ) => { | ||
| const [isComposing, setIsComposing] = React.useState(false); | ||
| const [internalValue, setInternalValue] = React.useState( | ||
| typeof value === "string" ? value : "", | ||
| ); | ||
| const inputRef = React.useRef<HTMLInputElement>(null); | ||
| const resolvedRef = (ref as React.RefObject<HTMLInputElement>) || inputRef; | ||
|
|
||
| // Update internal value when external value changes and not composing | ||
| React.useEffect(() => { | ||
| if (!isComposing && typeof value === "string") { | ||
| setInternalValue(value); | ||
| } | ||
| }, [value, isComposing]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd /tmp/repo && find . -type f -name "input.tsx" | head -5Repository: langflow-ai/langflow
Length of output: 123
🏁 Script executed:
cd /tmp/repo && cat -n src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 123
🏁 Script executed:
find . -type f -name "input.tsx" | head -10Repository: langflow-ai/langflow
Length of output: 106
🏁 Script executed:
cat -n src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 5014
Handle non-string controlled values to avoid blank inputs.
value can be a number (per React.InputHTMLAttributes); the current logic only syncs string values to internalValue, causing numeric controlled inputs to render empty.
✅ Suggested normalization
- const [internalValue, setInternalValue] = React.useState(
- typeof value === "string" ? value : "",
- );
+ const normalizeValue = (v: InputProps["value"]) =>
+ v === undefined || v === null ? "" : String(v);
+ const [internalValue, setInternalValue] = React.useState(
+ normalizeValue(value),
+ );
@@
- React.useEffect(() => {
- if (!isComposing && typeof value === "string") {
- setInternalValue(value);
- }
- }, [value, isComposing]);
+ React.useEffect(() => {
+ if (!isComposing && value !== undefined) {
+ setInternalValue(normalizeValue(value));
+ }
+ }, [value, isComposing]);📝 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.
| value, | |
| onChange, | |
| ...props | |
| }, | |
| ref, | |
| ) => { | |
| const [isComposing, setIsComposing] = React.useState(false); | |
| const [internalValue, setInternalValue] = React.useState( | |
| typeof value === "string" ? value : "", | |
| ); | |
| const inputRef = React.useRef<HTMLInputElement>(null); | |
| const resolvedRef = (ref as React.RefObject<HTMLInputElement>) || inputRef; | |
| // Update internal value when external value changes and not composing | |
| React.useEffect(() => { | |
| if (!isComposing && typeof value === "string") { | |
| setInternalValue(value); | |
| } | |
| }, [value, isComposing]); | |
| value, | |
| onChange, | |
| ...props | |
| }, | |
| ref, | |
| ) => { | |
| const normalizeValue = (v: InputProps["value"]) => | |
| v === undefined || v === null ? "" : String(v); | |
| const [isComposing, setIsComposing] = React.useState(false); | |
| const [internalValue, setInternalValue] = React.useState( | |
| normalizeValue(value), | |
| ); | |
| const inputRef = React.useRef<HTMLInputElement>(null); | |
| const resolvedRef = (ref as React.RefObject<HTMLInputElement>) || inputRef; | |
| // Update internal value when external value changes and not composing | |
| React.useEffect(() => { | |
| if (!isComposing && value !== undefined) { | |
| setInternalValue(normalizeValue(value)); | |
| } | |
| }, [value, isComposing]); |
🤖 Prompt for AI Agents
In `@src/frontend/src/components/ui/input.tsx` around lines 24 - 42, The component
only syncs string controlled values into internalValue causing numeric
controlled inputs to appear blank; update the initialization and the
React.useEffect sync to normalize non-null/non-undefined values to strings (use
String(value) when typeof value !== "string" and value != null) and keep using
isComposing to avoid clobbering in-progress IME input; specifically adjust the
internalValue initial state and the effect that calls setInternalValue
(referencing value, setInternalValue, internalValue, React.useEffect, and
isComposing) so numbers and other primitive controlled values render correctly
as text.
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| setInternalValue(e.target.value); | ||
| // Only call onChange if not composing (IME input in progress) | ||
| if (!isComposing && onChange) { | ||
| onChange(e); | ||
| } | ||
| }; | ||
|
|
||
| const handleCompositionStart = () => { | ||
| setIsComposing(true); | ||
| }; | ||
|
|
||
| const handleCompositionEnd = ( | ||
| e: React.CompositionEvent<HTMLInputElement>, | ||
| ) => { | ||
| setIsComposing(false); | ||
| // Normalize to NFC and trigger onChange after composition ends | ||
| const normalizedValue = e.currentTarget.value.normalize("NFC"); | ||
| setInternalValue(normalizedValue); | ||
| if (onChange) { | ||
| // Create a synthetic event with the normalized value | ||
| const syntheticEvent = { | ||
| ...e, | ||
| target: { | ||
| ...e.target, | ||
| value: normalizedValue, | ||
| }, | ||
| currentTarget: { | ||
| ...e.currentTarget, | ||
| value: normalizedValue, | ||
| }, | ||
| } as unknown as React.ChangeEvent<HTMLInputElement>; | ||
| onChange(syntheticEvent); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 5014
🏁 Script executed:
# Search for composition event handling tests or related code
rg -i "composition|compositionend" --type tsx --type ts src/frontend/ -A 3 -B 1Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
# Check if there are any tests for this input component
fd "input\.test\|input\.spec" src/frontend/ 2>/dev/nullRepository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
# Search for usage patterns of this Input component to understand expected behavior
rg "onChange.*input\|Input.*onChange" src/frontend/src --type tsx --type ts -l | head -10Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
# Search for composition event handling patterns
rg -i "composition|compositionend" src/frontend/ -A 3 -B 1Repository: langflow-ai/langflow
Length of output: 1438
🏁 Script executed:
# Check if there are any tests for the input component
fd "input" src/frontend/ -type f | grep -i testRepository: langflow-ai/langflow
Length of output: 234
🏁 Script executed:
# Search for onChange handler patterns to understand expected behavior
rg "onChange" src/frontend/src/components/ui/input.tsx -B 2 -A 2Repository: langflow-ai/langflow
Length of output: 1092
🌐 Web query:
React compositionend onChange double firing browser behavior
💡 Result:
Short answer
This is a known browser/React interaction: IME composition (compositionstart/compositionend) + React’s synthetic onChange (which wraps the native input event) can produce duplicate/extra change events in some browsers (Chrome) — especially when the input is re-rendered/removed during event handling. To avoid the double fire, track IME composition and ignore input/onChange while composing, or avoid unmounting/replacing the input inside the handler. [1][2][3][4]
What’s happening (concise)
- React’s onChange is a cross-browser wrapper around the native input event; IME input uses compositionstart/compositionend and the final text can emit input/change events. React will therefore receive these native events too. [4]
- Chrome has documented/observed odd behavior where an input’s change/input event may fire again if the element is destroyed or replaced during the event handling (which can look like a “double” onChange). See reproductions and a Chromium bug. [2]
- IME compositionend can also cause an extra event boundary that looks like a duplicate onChange unless you explicitly handle composition state. [1]
How to fix (practical options)
- Track composition and ignore onChange while composing:
- onCompositionStart => set isComposing = true
- onCompositionEnd => set isComposing = false and handle the final value there (or allow onChange after clearing the flag).
- Don’t remove/replace the input during the onChange/onInput handler — postpone DOM changes that unmount the input until after the handler (setTimeout or state flow). This avoids the Chrome double-fire bug. [2]
- Alternatively, use onInput if your logic suits it and you want the native input event semantics, but still guard composition. [4]
- Debounce or deduplicate handlers (compare previous value) as a last-resort defense.
References
- MDN: compositionend event (IME composition behaviour). [1] (developer.mozilla.org)
- StackOverflow discussion and Chromium bug / reproduction about input/change firing again when input is removed during event handling. [2] (stackoverflow.com)
- React GitHub issue(s) about input/dispatch/dispatchEvent and event handling differences. [3] (github.com)
- Article explaining React onChange vs onInput and React’s event mapping. [4] (medium.com)
If you want, paste a minimal snippet of your component and I’ll show an exact isComposing-based fix.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/EditContext/compositionend_event?utm_source=openai
- 2: https://stackoverflow.com/questions/19577815/replacing-an-input-element-causes-onchange-event-in-chrome-only?utm_source=openai
- 3: dispatchEvent on input/textarea is ignored facebook/react#10135
- 4: https://medium.com/%40vladperyazev14/why-you-should-avoid-using-onchange-in-react-44400122b879?utm_source=openai
🌐 Web query:
React composition events and onChange firing sequence Chrome Firefox
💡 Result:
Short answer
- Chrome & Safari: input (or React's onInput/onChange) fires before compositionend.
- Firefox (Gecko) & older Edge: compositionend fires before input.
This difference means IME composition can cause onChange/onInput order to differ across browsers. (github.com)
Why it matters for React
- React’s onChange is a synthetic wrapper around the browser input/input-like events, so the native event ordering affects when React handlers run during IME composition. React also exposes onCompositionStart/onCompositionUpdate/onCompositionEnd to detect composition state. React issues/history discuss this exact cross‑browser problem. (github.com)
Practical workaround
- Track composition state: set a flag on onCompositionStart and clear it on onCompositionEnd, and ignore onChange/onInput while isComposing is true (or handle compositionend specially). This is the common recommended approach for controlled inputs that must not act on intermediate IME input. (github.com)
If you want, I can show a minimal React pattern (code) that implements the isComposing guard.
Citations:
- 1: Event order between "compositionend" and "input" w3c/uievents#202
- 2: Composition Events(Chinese, Japanese IME) problem in controlled components(input, textarea) facebook/react#8683
- 3: Composition Events(Chinese, Japanese IME) problem in controlled components(input, textarea) facebook/react#8683
🏁 Script executed:
# Check if there are any issues or tests related to composition in the codebase
rg -i "composing\|composition" src/frontend/src/components --type-list | head -20Repository: langflow-ai/langflow
Length of output: 453
🏁 Script executed:
# Search for similar input handling patterns in the codebase
rg "isComposing\|skipNext" src/frontend/src -A 2 -B 1Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
# Look for any GitHub issues or comments about this specific concern in the repo
rg -i "double.fire\|double.onChange" --type-listRepository: langflow-ai/langflow
Length of output: 453
🏁 Script executed:
# Check the actual event flow - look at handleCompositionEnd more carefully
sed -n '56,78p' src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 815
🏁 Script executed:
# Check if there are any React version constraints or other context
cat package.json | grep -A 5 -B 5 "react"Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
# Search for how this Input component is tested or used
rg "Input\>" src/frontend/src/components --max-count 5Repository: langflow-ai/langflow
Length of output: 142
🏁 Script executed:
# Check the React version to understand batching behavior
rg '"react"' . --max-count 1 | head -5Repository: langflow-ai/langflow
Length of output: 441
🏁 Script executed:
# Let's verify the exact event sequence with React 19
# Check if there are any comments or issues about event ordering
rg "setIsComposing\|handleChange\|handleCompositionEnd" src/frontend/src/components/ui/input.tsx -B 1 -A 1Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
# One more check: verify that onChange is being called directly in handleCompositionEnd
sed -n '63,77p' src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 501
Add a ref-based guard to prevent onChange from firing twice after IME composition.
After handleCompositionEnd explicitly calls onChange, the browser's native input event will fire and trigger handleChange again. Although the current isComposing state guard attempts to prevent this, React 19's event batching makes the state update timing unpredictable—isComposing may not have updated before the second onChange fires, causing duplicate handler invocations.
Use a ref to immediately flag and skip the next onChange call, which is more reliable than depending on state timing:
Suggested fix
const Input = React.forwardRef<HTMLInputElement, InputProps>(
(props, ref) => {
const [isComposing, setIsComposing] = React.useState(false);
const [internalValue, setInternalValue] = React.useState(
typeof value === "string" ? value : "",
);
const inputRef = React.useRef<HTMLInputElement>(null);
+ const skipNextChangeRef = React.useRef(false);
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
setInternalValue(e.target.value);
+ if (skipNextChangeRef.current) {
+ skipNextChangeRef.current = false;
+ return;
+ }
// Only call onChange if not composing (IME input in progress)
if (!isComposing && onChange) {
onChange(e);
}
};
const handleCompositionEnd = (
e: React.CompositionEvent<HTMLInputElement>,
) => {
setIsComposing(false);
// Normalize to NFC and trigger onChange after composition ends
const normalizedValue = e.currentTarget.value.normalize("NFC");
setInternalValue(normalizedValue);
if (onChange) {
+ skipNextChangeRef.current = true;
// Create a synthetic event with the normalized value
const syntheticEvent = {
...e,
target: {
...e.target,
value: normalizedValue,
},
currentTarget: {
...e.currentTarget,
value: normalizedValue,
},
} as unknown as React.ChangeEvent<HTMLInputElement>;
onChange(syntheticEvent);
}
};🤖 Prompt for AI Agents
In `@src/frontend/src/components/ui/input.tsx` around lines 44 - 77, The IME
composition flow can cause onChange to fire twice because isComposing state may
not update in time; add a ref-based guard (e.g., skipNextOnChangeRef) and use it
in handleChange and handleCompositionEnd: in handleCompositionEnd set
skipNextOnChangeRef.current = true before calling the synthetic onChange, then
in handleChange return early if skipNextOnChangeRef.current is true and reset it
to false; keep existing setIsComposing calls in
handleCompositionStart/handleCompositionEnd but rely on the ref for the
immediate skip of the next onChange to avoid duplicate invocations.
| <button | ||
| className="underline cursor-pointer bg-transparent border-0 p-0 text-inherit" | ||
| onClick={() => { | ||
| setOpen(false); | ||
| navigate("/settings/mcp-servers"); | ||
| }} | ||
| > | ||
| settings | ||
| </CustomLink> | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
head -n 310 src/frontend/src/modals/addMcpServerModal/index.tsx | tail -n 30Repository: langflow-ai/langflow
Length of output: 1222
🏁 Script executed:
wc -l src/frontend/src/modals/addMcpServerModal/index.tsxRepository: langflow-ai/langflow
Length of output: 119
🏁 Script executed:
cat -n src/frontend/src/modals/addMcpServerModal/index.tsx | grep -A 50 -B 50 "settings"Repository: langflow-ai/langflow
Length of output: 4474
🏁 Script executed:
fd BaseModal src/frontend/src --type f -e ts -e tsx | head -5Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
rg "export.*BaseModal" src/frontend/src --type ts --type tsx -A 5 | head -30Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
fd BaseModal src/frontend/src --type fRepository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
rg "export.*BaseModal" src/frontend/src -A 5 | head -40Repository: langflow-ai/langflow
Length of output: 133
🏁 Script executed:
cat -n src/frontend/src/modals/baseModal/index.tsx | head -100Repository: langflow-ai/langflow
Length of output: 3306
🏁 Script executed:
rg "form\|submit" src/frontend/src/modals/baseModal/index.tsx -i -B 2 -A 2Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
cat -n src/frontend/src/modals/baseModal/index.tsx | sed -n '100,250p'Repository: langflow-ai/langflow
Length of output: 4979
🏁 Script executed:
cat -n src/frontend/src/modals/baseModal/index.tsx | tail -150Repository: langflow-ai/langflow
Length of output: 5103
Add type="button" to prevent form submission.
The button is nested inside a <Form.Root> (from BaseModal with onSubmit={submitForm}). Without the type attribute, it defaults to type="submit" and triggers form submission, unintentionally calling submitForm which modifies MCP servers. The button should navigate to settings without submitting the form.
✅ Proposed fix
<button
+ type="button"
className="underline cursor-pointer bg-transparent border-0 p-0 text-inherit"
onClick={() => {
setOpen(false);
navigate("/settings/mcp-servers");
}}
>
settings
</button>📝 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.
| <button | |
| className="underline cursor-pointer bg-transparent border-0 p-0 text-inherit" | |
| onClick={() => { | |
| setOpen(false); | |
| navigate("/settings/mcp-servers"); | |
| }} | |
| > | |
| settings | |
| </CustomLink> | |
| </button> | |
| <button | |
| type="button" | |
| className="underline cursor-pointer bg-transparent border-0 p-0 text-inherit" | |
| onClick={() => { | |
| setOpen(false); | |
| navigate("/settings/mcp-servers"); | |
| }} | |
| > | |
| settings | |
| </button> |
🤖 Prompt for AI Agents
In `@src/frontend/src/modals/addMcpServerModal/index.tsx` around lines 292 - 300,
The "settings" button inside the Form.Root is currently acting as a submit
because it lacks an explicit type; update the button rendered in the
addMcpServerModal (the element that calls setOpen(false) and
navigate("/settings/mcp-servers")) to include type="button" so it does not
trigger the Form.Root onSubmit (submitForm). This change should be made in the
callback that closes the modal and navigates to settings to prevent unintended
submitForm execution.
…in IME composition - Add normalizeValue function to convert non-string values to strings - Add skipNextOnChangeRef to prevent duplicate onChange calls during IME composition - Add type="button" to settings button in addMcpServerModal to prevent form submit Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Changes
src/frontend/src/components/ui/input.tsxisComposingRoot Cause
On Linux with IBus/dead keys, the controlled input was updating its value on every onChange, which interrupted character composition (where the accent precedes the letter).
Issue
Fixes #9096
Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.