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] Broken "stop editing" integration with IME e.g. Japanese #2338

Closed
2 tasks done
oliviertassinari opened this issue Aug 14, 2021 · 8 comments · Fixed by #5257
Closed
2 tasks done

[data grid] Broken "stop editing" integration with IME e.g. Japanese #2338

oliviertassinari opened this issue Aug 14, 2021 · 8 comments · Fixed by #5257
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Editing Related to the data grid Editing feature good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 14, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

IME keys are not processed correctly

Expected Behavior 🤔

IME keys are processed correctly

Steps to Reproduce 🕹

Steps:

  1. Open https://material-ui.com/components/data-grid/editing/#cell-editing
  2. See how pressing Enter shouldn't exit the edit mode. The end-user isn't done yet.
Aug-14-2021.11-30-31.mp4

The same happens with Escape.

version 4.0.0-alpha.38

Context 🔦

We saw this request by email:

I bought and use xgrid and am trying to figure out Japanese IME composition in x-grid.

When I type Japanese in a cell, typing Japanese letters and converting them into Chinese character (Kanji) for example, it ends composition and goes to next cell.

Is there any good way not to finish the composition session. I believe people use onCompositionEnd, onCompositionStart and onCompositionUpdate but you are only able to use them in , not

mui/material-ui#23044
mui/material-ui#19435
mui/material-ui#19499
mui/material-ui#23050

Sometimes, I feel that we should kill the x@mui.com email to force all communications to happen on GitHub cc @mui-org/x & @mbrookes

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Aug 14, 2021
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 14, 2021

This is an easy one to fix:

diff --git a/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts b/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
index 9de65f42..a6f97119 100644
--- a/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
+++ b/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
@@ -250,6 +250,12 @@ export function useGridEditRows(
         apiRef.current.commitCellChange({ id, field }, event);
         apiRef.current.publishEvent(GridEvents.cellEditStop, params, event);
       }
+
+      // Wait until IME is settled.
+      if (event.which === 229) {
+        return;
+      }
+
       if (isEditMode && isCellEditCommitKeys(event.key)) {
         const commitParams = { id, field };
         if (!apiRef.current.commitCellChange(commitParams, event)) {

However, I didn't work on it because it would likely create a git merge conflict with #2098. For the test cases, a quick simple one is probably enough. It's also in the realm of open-source, so we could leave it up to the community to work on it.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 14, 2021
@taeslee
Copy link

taeslee commented Sep 16, 2021

Hi @oliviertassinari,

I would like to work on this issue. Can you assign me to this issue?

@flaviendelangle flaviendelangle added good first issue Great for first contributions. Enable to learn the contribution process. and removed good first issue Great for first contributions. Enable to learn the contribution process. labels Sep 16, 2021
@flaviendelangle
Copy link
Member

Here you are 👍

@taeslee
Copy link

taeslee commented Sep 22, 2021

It seems like this issue was already resolved.
https://user-images.githubusercontent.com/64485716/134408162-295b170c-e053-4857-be7e-484d71473264.mp4

IMEJapanese.mp4

@oliviertassinari
Copy link
Member Author

I have just tried on https://mui.com/components/data-grid/editing/#cell-editing, it's still broken for me on macOS.

@guoyunhe
Copy link

I developed a similar React component in Alibaba. The way I find to support IME is to always mount an <input> with focus. When it is inactive, just set opacity to hide it.

In DataGrid, when users type "wo" in IME, "w" is always inserted as it is because here is no <input/> when users type "w", so the IME cannot spell "我". Mounting an invisible <input/> will solve the issue.

@alepuente
Copy link

@guoyunhe Is there any code example you can show me?

@oliviertassinari
Copy link
Member Author

I have just tested this on AG Grid's live demo, they don't support it. The bug was reported in ag-grid/ag-grid#4435.

@oliviertassinari oliviertassinari changed the title [DataGrid] Broken IME support [DataGrid] Broken "stop editing" integration with IME e.g. Japanese Jun 25, 2022
@oliviertassinari oliviertassinari changed the title [DataGrid] Broken "stop editing" integration with IME e.g. Japanese [data grid] Broken "stop editing" integration with IME e.g. Japanese Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Editing Related to the data grid Editing feature good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants