Downshift: Scrolling is janky when controlling the `highlightedIndex` prop

[jlongster]

I originally filed #280 even though it wasn't the exact issue I was hitting, but was related. I was able to reduce my issue down to a test case.

• downshift version: 1.22.3
• node version: 7.9.0
• npm (or yarn) version: 4.2.0

Reproduction repository: https://codesandbox.io/s/x3y2xzppyw

Problem description:

When you control the highlightedIndex prop, in componentDidUpdate it checks if the index has changed and if so scrolls that item into view:

    if (
      this.isControlledProp('highlightedIndex') &&
      this.props.highlightedIndex !== prevProps.highlightedIndex
    ) {
      this.scrollHighlightedItemIntoView()
    }

The problem is that the index can change just simply by scrolling the list. After you scroll the list, Chrome apparently fires a onMouseEnter event if the cursor is sitting over an item, even if the browser has hidden the cursor because and you're moving it with the keyboard or something. This is related to #240, but not exactly the same. In that issue, that event firing causes issues when navigating with the keyboard.

This also causes problems when scrolling with the trackpad though, and is probably hit much more frequently. You have to hover the cursor over the list to scroll it, so the cursor will always be hovering over an item.

In that case, as you scroll the list with the trackpad, if you've scrolled far enough, Downshift will try to scroll the item into view and mess up your scrolling. There seems to be an issue with the order of events: after you've scrolled, if the previous highlighted item has been scrolled out of view, Downshift thinks it needs to scroll the list, even though it shouldn't because the new highlighted item that the cursor sits over it obviously in view.

Here's a video: http://jlongster.com/s/downshift-scroll.mov

Downshift should never touch scrolling if you're scrolling with the trackpad.

Even if we fix this bug and downshift properly sees that the new highlighted item is in view and doesn't touch scrolling, since downshift scrolls partially hidden items to be fully shown, there's still a chance that my cursor will sit over the item at the bottom and cause similar problems. The fix should be to detect if the user is scrolling and never call scrollIntoView.

Suggested solution:

Detect if the user is scrolling and never call scrollIntoView if they are. The difficulty is how to detect this, since we don't know anything about the scrolled element. scrollIntoView used getClosestScrollParent to find it, but we need to set a scroll event handler on it at mount time and don't have a child node to walk up from.

[jlongster]

Thinking about this more, that fix is probably not right. It's very hard to guess when the user stops scrolling, and the onMouseEnter event that is causing problems will probably fire after we think the user has stopped scrolling anyway.

Let's focus on why it's not measuring the right node - why does it even think it needs to scroll something into view, when the new highlighted item should be fully in view? I'd be fine if we fixed that and it still scrolled the list if the item was partially shown at the top/bottom.

[kentcdodds]

Thank you for filing this issue so thoroughly! Definitely want to see this fixed. I don't have time at the moment. If anyone wants to dive in to answer the questions @jlongster asked to identify the core issue that'd be super :+1:

[jlongster]

I think I know a fix, if it works I'll file a PR.

[sebasalvarado]

Hey @jlongster thanks for thoroughly explaining this issue and filing a PR! I'm having a similar issue but I want that Downshift actually scrolls the list if I'm navigating with the arrow keys, from an a11y point of view, that's important for my use case.

Any ideas on how we can support that? I am currently navigating with the arrow keys and after the highlightedIndex gets past what's visible in the view I can no longer see what I'm highlighting.