[DataGrid] Backspace stopped working in inputs in cells

[piotrdytkowski]

I have an Input component inside a cell, I can type text in it, but pressing backspace freezes the input.
This was working in version 4.0.0-alpha.22, stopped working in 4.0.0-alpha.23.

• 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 😯

Pressing backspace in an input freezes it

Expected Behavior 🤔

Pressing backspace in an input should remove last character in an input.

Steps to Reproduce 🕹

Steps:

1. Focus in some input.
2. Type some text
3. Hit backspace
   Sandbox : https://codesandbox.io/s/data-grid-with-broken-input-fw5dx

To see expected behaviour change data grid version to 4.0.0-alpha.22 and repeat steps.

Your Environment 🌎

System:
OS: Linux 5.4 Ubuntu 18.04.5 LTS (Bionic Beaver)
Binaries:
Node: 12.21.0 - /usr/bin/node
Yarn: 1.22.5 - /usr/bin/yarn
npm: 6.14.11 - /usr/bin/npm
Browsers:
Chrome: 89.0.4389.82
Firefox: 86.0.1
npmPackages:
@material-ui/core: 4.11.3 => 4.11.3
@material-ui/data-grid: 4.0.0-alpha.23 => 4.0.0-alpha.23
@material-ui/styles: 4.11.3
@material-ui/system: 4.11.3
@material-ui/types: 5.1.0
@material-ui/utils: 4.11.2
@types/react: ^16.9.35 => 16.14.4
react: ^16.13.1 => 16.14.0
react-dom: ^16.13.1 => 16.14.0
styled-components: ^5.1.1 => 5.2.1
typescript: 4.0.5 => 4.0.5

Order id 💳

[oliviertassinari]

Backspace is not the only regression with cells in read mode and containing a textbox. See this one too. It's basically impossible to focus something inside any cells as soon as one cell is focused. It's not a light regression:

It was reported in #1295 too.

[oliviertassinari]

I have noticed a couple of bugs with the logic. It seems that we have a couple of new e2e tests to add :D.

1. First, if the cell is not editable, it doesn't make sense to handle the GRID_CELL_EXIT_EDIT event:

diff --git a/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts b/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.
ts
index 4f31e22f..6299c7bf 100644
--- a/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
+++ b/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
@@ -208,6 +208,10 @@ export function useGridEditRows(apiRef: GridApiRef) {

   const handleExitEdit = React.useCallback(
     (params: GridCellParams, event: React.SyntheticEvent) => {
+      if (!params.isEditable) {
+        return;
+      }
+
       setCellMode(params.id, params.field, 'view');

2. Second, firing GRID_CELL_EXIT_EDIT on a Backspace on a nested input is simply wrong. I propose we go even further, ignoring all events:

diff --git a/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts b/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.
ts
index 4f31e22f..392d7dee 100644
--- a/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
+++ b/packages/grid/_modules_/grid/hooks/features/rows/useGridEditRows.ts
@@ -13,6 +13,7 @@ import {
   GRID_CELL_KEYDOWN,
   GRID_CELL_VALUE_CHANGE,
 } from '../../../constants/eventsConstants';
+import { isGridCellRoot } from '../../../utils/domUtils';
 import { GridApiRef } from '../../../models/api/gridApiRef';
 import { GridEditRowApi } from '../../../models/api/gridEditRowApi';
 import { GridCellMode } from '../../../models/gridCell';
@@ -265,12 +266,13 @@ export function useGridEditRows(apiRef: GridApiRef) {

   const handleCellKeyDown = React.useCallback(
     (params: GridCellParams, event) => {
+      const fromCellRoot = isGridCellRoot(event.target)
       const isEditMode = params.cellMode === 'edit';

-      if (!isEditMode && isCellEnterEditModeKeys(event.key)) {
+      if (!isEditMode && fromCellRoot && isCellEnterEditModeKeys(event.key)) {
         apiRef.current.publishEvent(GRID_CELL_ENTER_EDIT, params, event);
       }
-      if (!isEditMode && isDeleteKeys(event.key)) {
+      if (!isEditMode && fromCellRoot && isDeleteKeys(event.key)) {
         const commitParams: GridEditCellPropsParams = apiRef.current.getEditCellPropsParams(
           params.id,
           params.field,

3. Third, I noticed that we can hedge for future iframe, popup support. ⚠ there are multiple document:

diff --git a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboard.ts b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridK
eyboard.ts
index 40feeccb..41fd49e8 100644
--- a/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboard.ts
+++ b/packages/grid/_modules_/grid/hooks/features/keyboard/useGridKeyboard.ts
@@ -150,7 +150,7 @@ export const useGridKeyboard = (

   const handleCellKeyDown = React.useCallback(
     (params: GridCellParams, event: React.KeyboardEvent) => {
-      if (!isGridCellRoot(document.activeElement)) {
+      if (!isGridCellRoot(event.target as Element)) {
         return;
       }

4. Fourth, the event name is wrong:

diff --git a/packages/grid/_modules_/grid/constants/eventsConstants.ts b/packages/grid/_modules_/grid/constants/eventsConstants.ts
index 0d2a6634..8f4ddb6d 100644
--- a/packages/grid/_modules_/grid/constants/eventsConstants.ts
+++ b/packages/grid/_modules_/grid/constants/eventsConstants.ts
@@ -28,7 +28,7 @@ export const GRID_CELL_VALUE_CHANGE = 'cellValueChange';
 export const GRID_CELL_ENTER_EDIT = 'cellEnterEdit';
 export const GRID_CELL_EXIT_EDIT = 'cellExitEdit';
 export const GRID_CELL_NAVIGATION_KEYDOWN = 'cellNavigationKeyDown';
-export const GRID_CELL_FOCUS = 'cellCellFocus';
+export const GRID_CELL_FOCUS = 'cellFocus';

 export const GRID_ROW_CLICK = 'rowClick';
 export const GRID_ROW_DOUBLE_CLICK = 'rowDoubleClick';

5. Lastly, for the click issue, it's the one I spent the most time on. This commit is wrong 5169c94. I didn't look into a solution. But I already left a comment about it, so it's not a surprise: 5169c94#r596918212.

[ZeeshanTamboli]

This works with latest version 4.0.0-alpha.27- https://codesandbox.io/s/data-grid-with-broken-input-forked-4qudh. So perhaps we can close this.

[oliviertassinari]

@ZeeshanTamboli Before we close, do you want to take care of the small side bugs found in #1300 (comment)? :)

[ZeeshanTamboli]

@ZeeshanTamboli Before we close, do you want to take care of the small side bugs found in #1300 (comment)? :)

I could but I am not understanding what those are. Writing/fixing code that I don't understand is not good.
Here are some:

First, if the cell is not editable, it doesn't make sense to handle the GRID_CELL_EXIT_EDIT event:

Why would we want to check isEditable here if the exit event won't even get triggered in the first place if the cell is not editable and which won't go into edit state?

Second, firing GRID_CELL_EXIT_EDIT on a Backspace on a nested input is simply wrong. I propose we go even further, ignoring all events:

Isn't it a view mode, !isEditMode?

4th can be fixed by renaming event.

3rd and 5th I did not understand.

[oliviertassinari]

@ZeeshanTamboli

• 1 and 2. I have created a reproduction to illustrate my point: https://codesandbox.io/s/material-demo-forked-0teqd?file=/demo.tsx. Notice how the exit event is fired when pressing Backspace or how it turns into edit mode when trying to type into the input. At this point. I think that we could ignore the behavior, and wait for a developer to raise a real pain point.
• 3 I explore it in [core] Help support different documents #1754.
• 4 already fixed
• 5 we have already made 2-3 PRs to handle the side effects of this approach, we can ignore it.