Multiple Selectable lists cause scrolling issues due to auto-focus

[musingh1]

Bug Report

Steps

• Navigate to https://stardust-ui.github.io/react/components/list
• Try adding selection to the last list example
  
• Try scrolling the page

Expected Result

The page should work fine if there are multiple selectable lists on it.

Actual Result

The scrolling does not work. It seems the list is trying to auto set the focus on the first item when scrolling is happening.

Version

v0.10

[levithomason]

Confirmed, this is an odd bug.

The core issue seems to be that the first item in a selection list is auto focused on scroll. Having two on the page causes both to be focused and the page tries to scroll them into view... while you're already scrolling.

Smaller Reproduction

Note the behavior of the List's Selection Example when scrolling.

1. Open the Selection Example
2. Click on the page background to unfocus all list items
3. Scroll the page a tiny bit
4. 🐞The first item is now focused

[levithomason]

OK, looks like an obvious List bug:

  componentDidUpdate() {
    this.focusableItem.tryFocus(ReactDOM.findDOMNode(this.itemRef.current) as HTMLElement)
  }

Anytime the list updates it's going to try to focus an item. There are a number of issues with this bit of code that we need to fix:

1. There are no tests for this List behavior.
2. findDOMNode should be avoided at all costs, see the React Docs. It is also deprecated in StrictComponent.
3. The List Item has an instance member this.focusableItem() which is created by passing this to a constructor. Passing instances around is code smell.
4. There is now a prop called focusableItemProps that doesn't make sense to consumers and should probably be removed.
5. The behavior of focusing an item is only required when the list is a selection list, but the this.focusableItem() is applied to every instance of the list.
6. There probably shouldn't be any componentDidUpdate code in the List, especially since it is stateless.

The source PR is #256. It appears we need this for accessibility reasons, however, I propose we reconsider the implementation.

[levithomason]

Fixed