[docs] Fix scrolling demo to work with React 18

[cherniavskii]

Fixes #6002
Fixes #8834

Preview: https://deploy-preview-6489--material-ui-x.netlify.app/x/react-data-grid/scrolling/#scrolling-to-specific-cells

[mui-bot]

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-6489--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	645.4	1,001.4	645.6	783.74	154.438
Sort 100k rows	ms	632.2	1,161.7	1,161.7	980.22	187.449
Select 100k rows	ms	273.9	355.4	311.9	308.42	29.387
Deselect 100k rows	ms	150.3	285	193.2	204.1	46.073

Generated by 🚫 dangerJS against 6bfc92b

[m4theushw]

I need to click twice the Home button to scroll and focus the very first cell. Have you checked this?

Since you're working on this demo, please add a white background to the grid, it's odd everything in gray.

[cherniavskii]

I need to click twice the Home button to scroll and focus the very first cell. Have you checked this?

Indeed, I can reproduce it by pressing Home icon after scrolling to the very bottom-right and focusing last cell.
It works just fine when using legacy ReactDOM.render API: https://codesandbox.io/s/withered-hooks-7p76ho?file=/index.tsx
I'll look into it.

please add a white background to the grid, it's odd everything in gray.

Done

[oliviertassinari]

I got curious about understanding what changes React 18 introduced.

With this change, state.focus.cell is empty after the navigation button is clicked. You can reproduce by opening the PR preview, doing the reproduction noticing how the document.activeElement is on the right cell, scrolling enough for virtualization to remove the focused cell, scroll back, and seeing that the focus isn't restored to the cell.

Screen.Recording.2022-10-16.at.23.37.39.mov

For what I understand, using useLayoutEffeect over useEffect allows React to render twice, once with the right focus state, then call .focus() and a second time with an empty state, and since we don't blur when the focus state is empty, the activeElement looks correct, but the state behind is wrong.

Things that can be explored:

1. Replace this logic with a blur event listener, it would likely be more resilient. We might be able to simplify things.

diff --git a/packages/grid/x-data-grid/src/hooks/features/focus/useGridFocus.ts b/packages/grid/x-data-grid/src/hooks/features/focus/useGridFocus.ts
index 5debc738f..d098cfc3f 100644
--- a/packages/grid/x-data-grid/src/hooks/features/focus/useGridFocus.ts
+++ b/packages/grid/x-data-grid/src/hooks/features/focus/useGridFocus.ts
@@ -230,19 +230,9 @@ export const useGridFocus = (

       if (cellParams) {
         apiRef.current.setCellFocus(cellParams.id, cellParams.field);
-      } else {
-        apiRef.current.setState((state) => ({
-          ...state,
-          focus: { cell: null, columnHeader: null },
-        }));
-        apiRef.current.forceUpdate();
-
-        // There's a focused cell but another element (not a cell) was clicked
-        // Publishes an event to notify that the focus was lost
-        publishCellFocusOut(focusedCell, event);
       }

2. If 1. is too much work compared to the benefit, we could wrap the content of React.useEffect(() => { in the demo with a setTimeout or anything that allows for the document click event handle to handle the event before the effect runs.
3. In the demo, we keep the focused cell in sync with one of the data grid, onCellClick is not enough, the focus can move with a lot of other ways. It doesn't seem to be a onFocusChange, so maybe with cellFocusIn and cellFocusOut

[cherniavskii]

Alright, I finally got back to this issue.
I believe this is a similar issue to facebook/react#24657, and the change in React 18 that broke the original behavior for us was facebook/react#21150

I ended up listening to the mouseup event on the document instead of the click event.
I had to fix a few unit tests to ensure they fire mousedown and mouseup events before the click event.

Before that, I tried listening to the click event in the capture phase, but it introduced a weird regression where the checkbox did not react to the first click.

[cherniavskii]

@oliviertassinari

Things that can be explored:

1. Replace this logic with a blur event listener, it would likely be more resilient. We might be able to simplify things.

This would be a pretty complex change, especially because of virtualization where the focused cell might be unmounted and mounted again.

[MBilalShafi]

I ended up listening to the mouseup event on the document instead of the click event.

Do you think these points may cause some unexpected behavior on the end user's side especially:

it appears that mouseup and mousedown events can also be caused by mouse buttons other than the left click button. Which is very different from what a generic user would expect.

[cherniavskii]

@MBilalShafi good points!

With a mouseup event, you can click somewhere else on the screen, hold down the click button, and move the pointer to your mouseup element, and then release the mouse pointer.
A click event requires the mousedown and mouseup event to happen on that element.

This one should not matter in our use case, since we are listening for events on document, so both mouseup and click events will be propagated to it anyway.

appears that mouseup and mousedown events can also be caused by mouse buttons other than the left click button. Which is very different from what a generic user would expect.

I tested the behavior with the right button (on a trackpad) and didn't notice any differences in how the focus state is handled.
Let me know if I'm missing something!

[MBilalShafi]

I tested the behavior with the right button (on a trackpad) and didn't notice any differences in how the focus state is handled.

Yes, correct, shouldn't be an issue, I was only curious what would be the behavior with middle button click (some computer mice have a middle button under the scroll wheel), if that also triggers the focus, that is probably not the user's expectation 🤔

But since I don't have such a mouse available, I can't comment much on this. It shouldn't be much of an issue in any case 👍

[cherniavskii]

Let's wait for @m4theushw's review then

[cherniavskii]

There is an editing issue that this PR fixes #8834

Before: https://codesandbox.io/s/priceless-breeze-xxwmep
After: https://codesandbox.io/s/elegant-microservice-1kebog