react-virtualized icon indicating copy to clipboard operation
react-virtualized copied to clipboard

When browser is zoomed or on high-DPI screens, onRowsRendered fires twice with an incorrect startIndex when scrolling via scrollToIndex

Open fwextensions opened this issue 7 years ago • 5 comments

This Code Sandbox project demonstrates the issue.

I've been working on a Chrome extension that uses a <List> component to render a few hundred items. The user can press Pg Up or Pg Down to move the selection a screen at a time, which scrolls the next selected row into view (if needed) by setting the List's scrollToIndex prop.

After developing it on a standard DPI laptop and monitor, I recently tested it on a Windows 10 laptop with a 4K screen, where the UI is scaled to 250% in the display settings. I noticed that when paging up the list after paging down, it scrolls one row too far, which never happened on the standard DPI screen.

In the code example above, the list is exactly the height of 5 rows. Open the console to see the indices returned by onRowsRendered.

If you scroll the list so that an odd-numbered item is partially visible at the bottom of the list, such as Item 7, then onRowsRendered will report 2 as the startIndex and 7 as the stopIndex, since 4 whole rows and 2 partial rows are visible at this point. If you then click the Down button, Item 7 will scroll fully into view and become selected, and Item 3 will be visible at the top of the list.

If you do this on a normal resolution screen, onRowsRendered will correctly report the indices as 3 and 7 after you click Down. But if you do the same thing on a screen scaled at something other than a whole multiple of 100%, then onRowsRendered fires twice, first with 3 and 7 and then again with 2 and 7. The latter event is wrong, since after scrolling Item 7 fully into view, Item 2 is no longer visible. If you then click the Up button, it scrolls too far and shows Item 2, since the component is relying on onRowsRendered to track which rows are currently visible.

I've also seen the same behavior on a 4K desktop monitor with Windows 10 scaled to 175%. Changing that to 200% seems to make the extra, incorrect onRowsRendered event go away. I haven't had a chance to try it on a Mac with a Retina screen.

This issue seems like it may be due to a rounding error somewhere, though I don't know why it would cause onRowsRendered to fire twice. For instance, if you change the RowHeight to 44 instead of 45, it doesn't happen, nor if you change the number of visible rows to 4 instead of 5 (4 * 45 = 180 vs. 5 * 45 = 225).

Test environment:

  • Windows 10
  • Chrome 64
  • React 15.6.2 in the extension, 16.0.0 in the Code Sandbox
  • React Virtualized 9.9.0 and 9.18.5

fwextensions avatar Feb 17 '18 20:02 fwextensions

Bugfix PR welcome.

I doubt I'll have any time to look into this.

bvaughn avatar Feb 18 '18 17:02 bvaughn

I looked at the issue and found that when browser zoom level is not 100%, setting scrollTop to X might not result in scrollTop = X. I put break point at this line and tried to set the scrollTop. This is the result: this._scrollingContainer.scrollTop = 25; console.log(this._scrollingContainer.scrollTop); // 24.44444465637207

This causes the actual scrollTop and scrollLeft to be different from the state. Thus, when the handleScrollEvent function is called after scrollTop or scrollLeft are set, the check here fails to prevent the re-render.

I'm trying to fix the issue by adding new variables for actual scroll values but this approach made "should not re-render grid components if they extend PureComponent" test fail.

prachp avatar Mar 01 '18 06:03 prachp

@prachp , thanks for investigating. I first saw the issue on a high-DPI screen with a scaled UI, where the browser zoom was at 100%. But you're right, the same thing happens if the browser itself is zoomed, so this is probably a more widespread issue.

I just tried it in Chrome on Win7 with normal 100% UI-scaling, but I zoomed the codesandbox posted above to 110%, and onRowsRendered fired twice, the second time with incorrect values.

I haven't looked into the React Virtualized code enough to have a useful opinion, but synching up the state and the actual scroll offsets seems like a reasonable approach.

fwextensions avatar Mar 01 '18 20:03 fwextensions

Hi @fwextensions - did you found any solution for this issue?

I also have a similiar issue when using the react virtualized for a large infinite scrolled list with masonry style.

If I scroll down the list with browser zoom level at 150%, then all the items will start to disappear after 4-5 pages due to wrong offset/position calculations. Similiar to your issue, this only happens when a certain zoom level in the browser is applied, and not with 100%.

pedromagalhaes avatar Jul 07 '20 12:07 pedromagalhaes

Hi, I tested on macOS retina display (zoomed and not zoomed). It's a Chrome only bug. I'll try to create a PR for this bug.

Edit: It will affect Electron based app (such as the Emoji Drawer on Signal-Desktop)

ziiw avatar Oct 18 '21 08:10 ziiw