WindowScroller setting incorrect scrollTop after resizing window height

[juliepagano]

I updated the version of WindowScroller for one of my projects today from 8.8.1 to the current version (8.11.0) and noticed this bug. More details about the use cases and the specifics of the bug below.

To reproduce

To reproduce the bug:

• Set up WindowScroller without specifying the new scrollElement prop (i.e. you want it to use window). My use case uses a Grid inside WindowScroller, but I think a similar issue would occur with other components.
• Scroll the page. scrollTop is set as expected at this point and everything works well.
• Scroll the page down far enough that the top of the container (i.e. element for your Grid or other component inside the WindowScroller) is off the top of the viewport.
• Resize the height of your browser window.
• Scroll the page. scrollTop is incorrectly set and the Grid behaves weirdly because it has the wrong scroll value.

I took a quick look at the project's WindowScroller demo and see similar "weird" behavior when I follow these steps, so hopefully that will be an easy example to work from.

Suspected cause

After looking at the changelog and some of the related source code, I suspect this bug came in with the support for a custom target element in v8.10.0. I'm guessing the problem lies in getPositionFromTop and how it is handling the calculations for the window use case.

Additional details:

I rolled back to my previous version (8.8.1) and verified this bug does not exist there.

Browser: Chrome 55 on OSX 10.11

Use case in my code: Grid inside a WindowScroller that only uses WindowScroller for vertical scrolling per the documentation that says it should not be used for horizontal scrolling.

[bvaughn]

Paging @andrewbranch; any ideas? Seems related to the recent scrollElement change maybe?

[andrewbranch]

Yep, looks like my addition of getVerticalScroll(container) should only be part of the calculation for custom elements. Guess that answers this question.

Sorry folks, I don’t know how I missed that. 😦

[bvaughn]

We both missed it 😅 No worries. I'm swamped today. I could push out a release if you think you'd have time to fix? But I won't have time to fix. Meetings meetings meetings

[andrewbranch]

Yeah, I'm already on it. Will have a PR shortly.

[bvaughn]

Fix released in version 8.11.1. Thanks for the detailed bug report @juliepagano! And thanks for the quick fix @andrewbranch! ❤️