-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
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
[material-ui][Dialog] Should not close until the IME is cancelled #39713
Conversation
Netlify deploy previewhttps://deploy-preview-39713--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
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.
Hey @megos, thanks for working on this 🎉
I left a question:
if (event.key !== 'Escape' || !isTopModal()) { | ||
if ( | ||
event.key !== 'Escape' || | ||
event.which === 229 || // Wait until IME is settled. |
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.
.which
is deprecated. Could we use .isComposing
instead?
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.
FYI I found in an old issue about isComposing
with an odd behavior in macOS Safari: #19435 (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.
As per the issue comment above, it needs to use which
instead of isComposing
for Safari.
This behavior is not fixed on the latest (17.1) Safari version.
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.
Looks good ~ appreciate that you added a test as well ✨ @megos
We use the escape key to cancel IME input.
This PR changed a dialog that keeps open when you cancel IME input in a dialog with the escape key.
This is similar to #19435