Modal: Fix unexpected modal closing in IME Composition#46453
Merged
Conversation
t-hamano
added
[Type] Bug
t-hamano
changed the title
t-hamano
changed the title
t-hamano
force-pushed
the
fix/components-modal-ime
branch
from
8a0a42d to
a663594
Compare
t-hamano
commented
Dec 11, 2022
Comment on lines
+79
to
+80
| <InputControl style={ { marginBottom: '20px' } } /> | ||
|
|
Contributor
Author
There was a problem hiding this comment.
I added this so that we can check it in the storybook, is this necessary?
Member
There was a problem hiding this comment.
It isn't necessary, but I think it's ok to keep around for a couple good reasons:
- It has to be tested manually (can't be put in an automated test)
- It's useful for this to be easily testable on many different platforms, since the behavior can vary between them
- It doesn't clutter the story file
|
Size Change: +16 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
mirka
approved these changes
Dec 12, 2022
mirka
left a comment
mirka
left a comment
Member
There was a problem hiding this comment.
Looking good! I've tested this on Mac Chrome/Safari/Firefox and it works as expected.
And thanks for adding the test!
Comment on lines
+79
to
+80
| <InputControl style={ { marginBottom: '20px' } } /> | ||
|
|
Member
There was a problem hiding this comment.
It isn't necessary, but I think it's ok to keep around for a couple good reasons:
- It has to be tested manually (can't be put in an automated test)
- It's useful for this to be easily testable on many different platforms, since the behavior can vary between them
- It doesn't clutter the story file
Contributor
Author
|
Thank you for the review, @mirka !
Okay, then I would like to merge it as is. |
7 tasks
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
2 participants
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.
Part of #45605
What?
This PR fixes unexpected modal closing in IME Composition.
Why?
In IME composition, the
escapekey is used to cancel character committing. This event is detected as a modal close action, causing unexpected modal close processing.580c94495e38743a61a44c71a3656b31.mp4
How?
Added determination of whether or not the compose is in progress, as was done with #45607 and #45626.
At the same time, I added a unit test to confirm that the modal should be closed with the
escapekey.Testing Instructions
In addition, I have confirmed that the following browsers work as expected in Windows 11:
Screenshots or screencast
997b62efd399bb8a9876bbba8d860436.mp4