-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix Japanese IME input handling in message composer #30324
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: develop
Are you sure you want to change the base?
Fix Japanese IME input handling in message composer #30324
Conversation
This commit addresses input issues with Japanese IME (Input Method Editor) where pressing Enter to confirm IME suggestions would create unwanted line breaks instead of properly confirming the input. Changes include: - Enhanced platform detection for proper cross-platform IME support - Improved composition event handling for Japanese input - Strengthened isComposing() method with platform-specific checks - Added proper handling for composition confirmation events - Platform-specific timing adjustments for input processing Fixes IME input issues on: - macOS Safari - Windows/Electron environments - Other modern browsers 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
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.
Hi! Thanks for the contribution.
I'm really conflicted about the quality of the PR generated by an LLM....
private detectPlatform(): IPlatformInfo { | ||
const ua = navigator.userAgent.toLowerCase(); | ||
const platform = navigator.platform || ""; | ||
|
||
return { | ||
isElectron: !!(window as any).electron || ua.includes('electron'), | ||
isWindows: ua.includes('win') || platform.includes('Win'), | ||
isMac: ua.includes('mac') || platform.includes('Mac'), | ||
isSafari: ua.includes("safari/") && !ua.includes("chrome/"), | ||
}; | ||
} | ||
|
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.
This is mixing platform, browser etc
Why do we need to detect windows and macos?
// IME確定後の入力イベントをプラットフォームに応じて処理 | ||
|
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.
Please keep the previous comment. Also the comments should be in english
} else if (this.platformInfo.isElectron) { | ||
// Electronでは確実に非同期実行 | ||
setTimeout(triggerInput, 0); |
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.
Why do we need to do that?
// IME composition状態の統合的な判定 | ||
const nativeEvent = event.nativeEvent; | ||
const isNativeComposing = nativeEvent && nativeEvent.isComposing; | ||
|
||
// プラットフォーム固有の追加チェック | ||
const isPlatformSpecificComposing = ( | ||
(this.platformInfo.isMac && this.platformInfo.isSafari && event.which === 229) || | ||
((this.platformInfo.isWindows || this.platformInfo.isElectron) && event.which === 229) | ||
); | ||
|
||
return !!(this.isIMEComposing || isNativeComposing || isPlatformSpecificComposing); |
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.
Keep the original comments and please explain why is it needed
// IME composition中は確定イベント以外を無視 | ||
if (this.isIMEComposing && event.inputType !== "insertCompositionText") { |
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.
ditto
return; | ||
} | ||
|
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.
Unneeded space change
// IME composition中は全てのキーイベントを無視 | ||
if (this.isComposing(event)) { | ||
return; | ||
} | ||
|
||
// プラットフォーム固有のIME処理 | ||
if (this.platformInfo.isMac && this.platformInfo.isSafari && event.which === 229) { | ||
// Safariの追加のkeyDownを無視 | ||
event.stopPropagation(); | ||
return; | ||
} | ||
|
||
// Windows/ElectronでのIMEキーコード229処理 | ||
if ((this.platformInfo.isWindows || this.platformInfo.isElectron) && event.which === 229) { | ||
// Windows IMEのキーコード229を適切に処理 | ||
return; | ||
} |
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.
ditto
This commit addresses input issues with Japanese IME (Input Method Editor) where pressing Enter to confirm IME suggestions would create unwanted line breaks instead of properly confirming the input.
Changes include:
Fixes IME input issues on:
🤖 Generated with Claude Code
Checklist
public
/exported
symbols have accurate TSDoc documentation.