fix(ui): guard Enter key handlers against IME composition#66392
Open
okxint wants to merge 1 commit into
Open
Conversation
…t components LemonInput was overriding its internal onKeyDown (which had the isComposing guard) with the consumer's onKeyDown via props spread, so the guard never applied when consumers used the lower-level onKeyDown prop. fix: destructure consumer's onKeyDown and call it from within the merged handler, skipping Enter during composition. LemonTextArea already called consumer's onKeyDown after the guard, but did not early-return on composition so the consumer handler still fired on Enter during IME. fixed to return early. also add the guard to LemonInputSelect (tag-add on Enter) and EditableField (save on Enter), two of the highest-impact call sites listed in PostHog#60044.
Contributor
|
Reviews (1): Last reviewed commit: "fix(ui): guard Enter key handlers agains..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+561
to
+563
| if (e.key === 'Enter' && e.nativeEvent.isComposing) { | ||
| return // IME candidate confirmation — don't add a tag | ||
| } |
Contributor
There was a problem hiding this comment.
Because _onKeyDown is passed as the onKeyDown prop to LemonInput, and LemonInput's merged handler now returns early on Enter + isComposing before it calls consumerOnKeyDown?.(event), this guard in _onKeyDown is dead code: LemonInput will always swallow the IME-Enter event before _onKeyDown is ever invoked. The protection the comment describes is already provided one layer up.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Pressing Enter to confirm an IME candidate (Japanese, Chinese, Korean, etc.) also fires the surrounding action — creating an org, adding a tag, saving a field title, etc. Reported in #60044.
Root cause in
LemonInput: the internalonKeyDownhandler has theisComposingguard but is overridden by{...props}spread immediately after, so consumers using theonKeyDownprop bypass the guard entirely.Changes
LemonInput— destructuresonKeyDownfrompropsand calls it from inside the merged handler. Enter events during IME composition return early before eitheronPressEnteror the consumer's handler fires.LemonTextArea— already merged its consumeronKeyDown, but didn't early-return on composition, so the consumer handler still fired. Now returns early on Enter+isComposing.LemonInputSelect— adds the guard before the "add tag on Enter" path.EditableField— adds the guard before the "save on Enter" path.Closes #60044
How did you test this code?
Agent-authored. The fix is mechanically straightforward —
event.nativeEvent.isComposingis the standard web API guard for this. Verified the four changed files match the pattern used in the existing working path (onPressEnterinLemonInput).Automatic notifications
Docs update
N/A
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Traced the root cause to the
{...props}spread pattern overriding the internalonKeyDown, consistent with the diagnosis in the issue. Fixed by merging handlers rather than replacing them, and added early-return guards at the specific high-impact call sites listed in the issue.