fix: resolve IME enter to complete composition bug #4182
Conversation
alexisareyn
requested review from
Harsh-Anand-Singh
and removed request for
a team
Fix race condition where compositionend fires before onKeyDown handler, causing premature search submission when users press Enter to complete character composition. - Add useIMEComposition hook to track composition state with requestAnimationFrame - Modify autosuggest to use isComposing() check - Add comprehensive tests for IME composition scenarios - Ensure Enter key during composition completion doesn't trigger search
Harsh-Anand-Singh
force-pushed
the
autosuggest-ime-fix
branch
from
728039b to
908a46a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4182 +/- ##
=======================================
Coverage 97.15% 97.15%
=======================================
Files 874 875 +1
Lines 25660 25683 +23
Branches 9285 9286 +1
=======================================
+ Hits 24929 24952 +23
- Misses 684 725 +41
+ Partials 47 6 -41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for the contribution! Great PR description the problem & solution breakdown made this easy to review. LGTM 👍 |
Co-authored-by: alexisareyn <[email protected]>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
| break; | ||
| } | ||
| case KeyCode.enter: { | ||
| if (isComposing()) { |
There was a problem hiding this comment.
Did you try to check event.detail.shiftKey and event.detail.isComposing instead of introducing this hook?
There was a problem hiding this comment.
I initially tried event.detail.isComposing but it doesn't solve the race condition. The core issue is that compositionend fires immediately when Enter is pressed, clearing the composition state before onKeyDown executes. So by the time we check event.detail.isComposing, it's already false even for composition-ending Enter keys.
The useIMEComposition hook bridges this timing gap by using requestAnimationFrame to delay clearing the composition flag, ensuring onKeyDown can reliably detect composition-related Enter presses.
| { value: '_gak_word_', label: '각도 (Angle)', description: 'Starts with 각' }, | ||
| { value: '_gan_word_', label: '간단 (Simple)', description: 'Starts with 간' }, | ||
| { value: '_gar_word_', label: '가방 (Bag)', description: 'Korean word starting with 가' }, | ||
| { value: '_gae_word_', label: '개념 (Concept)', description: 'Starts with 개' }, |
There was a problem hiding this comment.
[non-blocking ] Could also be useful to introduce an integ test in the __integ__ folder to ensure a real browser behavior
There was a problem hiding this comment.
will add one with other fixes for comments below, thanks
| const { wrapper, container } = renderAutosuggest( | ||
| <Autosuggest {...defaultProps} onChange={onChange} onSelect={onSelect} /> | ||
| ); | ||
| const input = container.querySelector('input')!; |
There was a problem hiding this comment.
You can use wrapper.findNativeInput().getElement() instead
| expect(wrapper.findDropdown().findOpenDropdown()).not.toBe(null); | ||
| }); | ||
|
|
||
| test('selects option on enter keydown when not composing', () => { |
There was a problem hiding this comment.
This scenario is already covered here: https://github.com/cloudscape-design/components/blob/main/src/autosuggest/__tests__/autosuggest.test.tsx#L325
| const { wrapper, container } = renderAutosuggest( | ||
| <Autosuggest {...defaultProps} onChange={onChange} onSelect={onSelect} /> | ||
| ); | ||
| const input = container.querySelector('input')!; |
| expect(onChange).not.toHaveBeenCalled(); | ||
| expect(onSelect).not.toHaveBeenCalled(); | ||
|
|
||
| jest.restoreAllMocks(); |
There was a problem hiding this comment.
Could be moved to beforeEach
| const { wrapper, container } = renderAutosuggest( | ||
| <Autosuggest {...defaultProps} onChange={onChange} onSelect={onSelect} /> | ||
| ); | ||
| const input = container.querySelector('input')!; |
| getInputElement: () => HTMLInputElement | null; | ||
| } | ||
|
|
||
| const Component = React.forwardRef((props, ref: React.Ref<Ref>) => { |
There was a problem hiding this comment.
You can use our internal renderHook to achieve pretty much the same https://github.com/cloudscape-design/components/blob/main/src/__tests__/render-hook.tsx#L18
| }; | ||
| }, [elementRef, handleCompositionStart, handleCompositionEnd]); | ||
|
|
||
| const isComposing = useCallback(() => wasComposingRef.current, []); |
There was a problem hiding this comment.
Here you don't get benefits of using useCallback as the ref is a mutable object
| wasComposingRef.current = true; | ||
| }, []); | ||
|
|
||
| const handleCompositionEnd = useCallback(() => { |
There was a problem hiding this comment.
Here you don't get benefits of using useCallback as the ref is a mutable object
3 participants
Fix race condition where compositionend fires before onKeyDown handler, causing premature search submission when users press Enter to complete character composition.
Description
This PR fixes a critical accessibility issue affecting Korean (and other IME) users where pressing Enter to complete character composition would accidentally trigger search submission instead of just completing the character.
Problem: When Korean users type characters like 가 (ga), pressing Enter to complete the character would incorrectly trigger search submission because the browser's
compositionendevent fires before theonKeyDownhandler executes, creating a race condition.Root Cause: The timing issue meant there was no reliable way to distinguish between "Enter pressed to complete character composition" vs "Enter pressed to trigger search".
Solution: Created a useIMEComposition hook that tracks composition state and uses requestAnimationFrame to maintain the composition flag briefly after compositionend, allowing the onKeyDown handler to correctly identify composition-related Enter keypresses and prevent inappropriate search triggering.
How has this been tested?
Automated Testing:
useIMECompositionhook covering character formation, multiple composition cycles, edge cases, and proper cleanupnpm testManual Testing:
Start the dev server:
npm startNavigate to the autosuggest search page at http://localhost:8080
Switch to Korean 2-set keyboard input method
Type Korean characters (e.g., ㄱ + ㅏ to form 가) and press Enter to complete - should NOT trigger search
Type regular text and press Enter - should trigger search normally
Run automated tests:
npm testReview checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.