[DataGrid] Fix scrollToIndexes behavior

[oliviertassinari]

Breaking changes

• [DataGrid] Remove public apiRef.current.isColumnVisibleInWindow() as it servers private use cases.

---

Fix the bug reported in #1964 (review) with the solution proposed. I have spent 1 hour on this, nothing crazy.

It also appears to fix #2103 and closes #2104.

So two bug fixes, one extra test case, and 50% less code (isColumnVisibleInWindow seems not to have use cases for developers, so I have removed it).

Preview: https://deploy-preview-2162--material-ui-x.netlify.app/components/data-grid/#commercial-version

---

If you play with the preview, you will notice one last issue we have with the scrollToIndexes logic. See this big jump when I press Arrow Left?:

focus.mp4

The .focus() call for the new active cell wrecks the correct scroll position that the new logic tries to set. You can also notice it in the other direction when using Arrow Up, it drives me nuts.

I didn't look into how to solve this. It might be a timing issue, the browser auto-scroll when focusing an element off-screen, so maybe we need to force a layout computation when we change the scroll, no idea. I will open a new GitHub issue, the current UX is not great.

[m4theushw]

IMO, we shouldn't adopt this new approach for rowIndex. The virtualization doesn't respect the scrollTop which might create new problems. Once #1911 is done we could use it.

[m4theushw]

I noticed a difference of 1px between clientWidth and the old values. Should we calculate the inner width by ourselves?

Suggested change

	clientHeight: windowRef.current!.clientWidth,
	clientHeight: gridState.containerSizes!.windowSizes.width - gridState.scrollBar.scrollBarSize.y,

In Chrome, it misses the right border. In Firefox it works fine. This doesn't happen in https://material-ui.com/components/data-grid/demo/

[oliviertassinari]

I have tried to reproduce on BS with Windows 10 and Chrome 91 but can't:

[oliviertassinari]

Should we calculate the inner width by ourselves?

I think that it would be better to reduce the number of dependencies here. The more we can rely on direct access to the DOM API, the better. It would be great to get to the root cause of what you have found. Is it a sub-pixel alignment issue? Is it a CSS outline rendering glitch? Did you zoom?

Now, if it's a 1px alignment issue, that not everybody can reproduce, we could also decide to ignore it.

[m4theushw]

I have tried to reproduce on BS with Windows 10 and Chrome 91 but can't:

I noticed it only happens if the Windows's scaling is above 100%. Here's 125% because the resolution is 1920x1080. To simulate it, open Chrome in BrowserStack and zoom to 125%. Use https://deploy-preview-2162--material-ui-x.netlify.app/components/data-grid/demo/ to test. The clientWidth should be 830 but it's 829.

[oliviertassinari]

I can reproduce with a zoom of 125% on Windows and macOS. I can also reproduce on AG Grid.

I guess Chrome is rounding numbers when we read the DOM but differently based on what we read.

[m4theushw]

Yes, let's see if some user complains about. With the current logic we use today, this glitch doesn't occur. That was my only concern.

[oliviertassinari]

In any case, 💯 for having found this. Even if I don't that we should try hard to fix it at the state of the component (high opportunity cost).

I think that later on, once we need to find new ways to further differentiate in this mature market, it could matter.

[oliviertassinari]

It's off topic, but seems simple enough, it's meant so the outline fits better.

before:

after:

[oliviertassinari]

IMO, we shouldn't adopt this new approach for rowIndex. The virtualization doesn't respect the scrollTop which might create new problems. Once #1911 is done we could use it.

@m4theushw To be clear, I didn't change the approach used by rowIndex.

This PR is about updating colIndex to use the approach we use in HEAD for rowIndex. It's also about creating an abstraction so both axes use the same logic (scrollIntoView). Now that we give layout information to scrollIntoView, in the future, we can add more options, to it, similar to https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView what accepts.