[DataGrid] Tab should update state.focus on edit mode

[yaredtsy]

Fixes #5952

Before
on Edit mode Tab key was not updating the state.focus. even if the focus of the component changes the state is not updating.
Demo: https://codesandbox.io/s/updaterowsapiref-demo-mui-x-forked-buy7rq?file=/demo.tsx

Now
https://codesandbox.io/s/updaterowsapiref-demo-mui-x-forked-kcn7ft?file=/demo.tsx

[mui-bot]

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	483.2	733	664.6	620.86	100.513
Sort 100k rows	ms	554.5	1,109.8	667.6	832.5	200.112
Select 100k rows	ms	141.8	254.3	222.6	208.82	37.919
Deselect 100k rows	ms	154.1	201	187.9	178.92	18.153

Generated by 🚫 dangerJS against b1543c7

[cherniavskii]

Sorry for taking so long to review this PR.
While it fixes the linked issue, I think it's a bit broader topic - we will need to somehow handle the cells with more than one focusable element, like date pickers. Ideally, it should be possible to Tab from input to the icon button.

This PR might be an intermediate solution if it doesn't break anything.

[cherniavskii]

Suggested change

	event.key === 'Tab'
	(cellParams.cellMode !== GridCellModes.Edit && event.key === 'Tab')

This would fix failed e2e test

[yaredtsy]

This would fix failed e2e test

but this was the problem useGridKeyboardNavigation won't publish the event in the edit mode.

[cherniavskii]

Sorry, I forgot to reverse the check.
I meant this:

- event.key === 'Tab'
+ (cellParams.cellMode === GridCellModes.Edit  && event.key === 'Tab')

[yaredtsy]

hey @cherniavskii

I have adapted @m4theushw solution and I think everything is working now.

[m4theushw]

I was not aware of this PR. I opened #7098 with a different approach that also works with Portals. IMO the keyboard navigation for cells in edit mode should be handled inside the editing hook. When Tab is pressed, we can't only move to the column, it should move to the next editable column. The code below is buggy:

mui-x/packages/grid/x-data-grid/src/hooks/features/keyboardNavigation/useGridKeyboardNavigation.ts

Lines 383 to 389 in d2cf267

	case 'Tab': {
	// "Tab" is only triggered by the row / cell editing feature
	if (event.shiftKey && colIndexBefore > firstColIndex) {
	goToCell(colIndexBefore - 1, getRowIdFromIndex(rowIndexBefore), 'left');
	} else if (!event.shiftKey && colIndexBefore < lastColIndex) {
	goToCell(colIndexBefore + 1, getRowIdFromIndex(rowIndexBefore), 'right');
	}

This PR creates a regression when a editable: false column is between two editable: true columns.

#6668

HEAD

Above the username column is not editable but it gets focused when pressing Tab.

Checking the next valid column to move the focus is a bit complex and shouldn't be inside useGridKeyboardNavigation because it would be poluted with editing stuff.

Have you considered the proposal I made in #5952 (comment)? The current PR is still prone to non-user-originated clicks, like element.click().

[m4theushw]

cellNavigationKeyDown was removed in v6.

[yaredtsy]

Have you considered the proposal I made in #5952 (comment)

no, I saw the comment in useGridFocus. I thought someone forgot to update useGridKeyboardNavigation to handle the focus.

mui-x/packages/grid/x-data-grid/src/hooks/features/focus/useGridFocus.ts

Lines 208 to 212 in 02df3c6

	// GRID_CELL_NAVIGATION_KEY_DOWN handles the focus on Enter, Tab and navigation keys
	if (event.key === 'Enter' || event.key === 'Tab' || isNavigationKey(event.key)) {
	return;
	}
	apiRef.current.setCellFocus(params.id, params.field);

I thought it was a simple bug, I will look into it.

[yaredtsy]

I have copied the #7098 solution, and it fixed all issues.

Demo: https://codesandbox.io/s/updaterowsapiref-demo-mui-x-forked-kcn7ft?file=/demo.tsx