Skip to content
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

[data-grid]: use event.type for detecting IME key press #14146

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

k-rajat19
Copy link
Contributor

Currently event.which is used for detecting IME composition event which is working fine but it is deprecated so we should find some other alternative for detecting IME key press. After some research, I found that we can use event.type property to check if the event is a composition event.
there is also a event.isComposing property which can be used but it is only supported in modern browsers.

@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Aug 12, 2024
@k-rajat19
Copy link
Contributor Author

@flaviendelangle would you like to give your input on this 😊

@flaviendelangle
Copy link
Member

Hi, I'm not working on the data grid anymore
@romgrk should be able to help 👍

@romgrk
Copy link
Contributor

romgrk commented Sep 19, 2024

The code LGTM but I don't know how to test this or if we have existing automated tests. Have you tested this change?

@mui-bot
Copy link

mui-bot commented Sep 19, 2024

Deploy preview: https://deploy-preview-14146--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against ae92fbd

@romgrk
Copy link
Contributor

romgrk commented Sep 19, 2024

Failing CI, that answers my question about automated tests.

@k-rajat19
Copy link
Contributor Author

@romgrk I've updated tests to reflect current behavior👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants