From 845188255ae953fc5117dd62c50749804f5953bd Mon Sep 17 00:00:00 2001
From: aarce-uncharted <60662654+aarce-uncharted@users.noreply.github.com>
Date: Sat, 8 Aug 2020 21:31:07 -0400
Subject: [PATCH] fix: Prevent jump on node selection (no auto scroll to
 center) (#408)

Co-authored-by: Hrusikesh Panda <mrchief@users.noreply.github.com>
---
 src/index.js                  |  5 ++++-
 src/index.keyboardNav.test.js | 15 +++++++++------
 src/index.test.js             | 14 ++++++++++++++
 src/tree/index.js             |  9 +--------
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/src/index.js b/src/index.js
index 687202bc..4081b403 100644
--- a/src/index.js
+++ b/src/index.js
@@ -254,7 +254,10 @@ class DropdownTreeSelect extends Component {
         this.onNodeToggle
       )
       if (newFocus !== currentFocus) {
-        this.setState({ currentFocus: newFocus })
+        this.setState({ currentFocus: newFocus }, () => {
+          const ele = document && document.getElementById(`${newFocus}_li`)
+          ele && ele.scrollIntoView()
+        })
       }
     } else if (showDropdown && ['Escape', 'Tab'].indexOf(e.key) > -1) {
       if (mode === 'simpleSelect' && tree.has(currentFocus)) {
diff --git a/src/index.keyboardNav.test.js b/src/index.keyboardNav.test.js
index f484b908..6334abb7 100644
--- a/src/index.keyboardNav.test.js
+++ b/src/index.keyboardNav.test.js
@@ -1,6 +1,6 @@
 import test from 'ava'
 import React from 'react'
-import { spy, stub } from 'sinon'
+import { spy, stub, assert } from 'sinon'
 import { mount } from 'enzyme'
 import DropdownTreeSelect from './index'
 
@@ -164,6 +164,7 @@ test('should set current focus as selected on tab out for simpleSelect', t => {
 
 test('should scroll on keyboard navigation', t => {
   const largeTree = [...Array(150).keys()].map(i => node(`id${i}`, `label${i}`))
+  const scrollIntoView = (Element.prototype.scrollIntoView = spy())
   const wrapper = mount(<DropdownTreeSelect data={largeTree} showDropdown="initial" />)
   const getElementById = stub(document, 'getElementById')
   const contentNode = wrapper.find('.dropdown-content').getDOMNode()
@@ -172,25 +173,26 @@ test('should scroll on keyboard navigation', t => {
 
   triggerOnKeyboardKeyDown(wrapper, ['ArrowUp'])
   largeTree.forEach((n, index) => {
-    getElementById.withArgs(`${n.id}_li`).returns({ offsetTop: index, clientHeight: 1 })
+    getElementById.withArgs(`${n.id}_li`).returns({ offsetTop: index, clientHeight: 1, scrollIntoView })
   })
 
   triggerOnKeyboardKeyDown(wrapper, ['ArrowUp'])
   t.deepEqual(wrapper.find('li.focused').text(), 'label148')
-  t.notDeepEqual(contentNode.scrollTop, 0)
+  assert.calledOnce(scrollIntoView)
 
   getElementById.restore()
 })
 
 test('should only scroll on keyboard navigation', t => {
   const largeTree = [...Array(150).keys()].map(i => node(`id${i}`, `label${i}`))
-  const wrapper = mount(<DropdownTreeSelect data={largeTree} showDropdown="initial" />)
   const getElementById = stub(document, 'getElementById')
+  const scrollIntoView = (Element.prototype.scrollIntoView = spy())
+  const wrapper = mount(<DropdownTreeSelect data={largeTree} showDropdown="initial" />)
   const contentNode = wrapper.find('.dropdown-content').getDOMNode()
 
   triggerOnKeyboardKeyDown(wrapper, ['ArrowUp'])
   largeTree.forEach((n, index) => {
-    getElementById.withArgs(`${n.id}_li`).returns({ offsetTop: index, clientHeight: 1 })
+    getElementById.withArgs(`${n.id}_li`).returns({ offsetTop: index, clientHeight: 1, scrollIntoView })
   })
 
   triggerOnKeyboardKeyDown(wrapper, ['ArrowUp'])
@@ -207,7 +209,8 @@ test('should only scroll on keyboard navigation', t => {
 
   // Verify scroll is restored to previous position after keyboard nav
   triggerOnKeyboardKeyDown(wrapper, ['ArrowUp', 'ArrowDown'])
-  t.deepEqual(contentNode.scrollTop, scrollTop)
+  // Called once for each input, 3 in this case.
+  assert.calledThrice(scrollIntoView)
 
   getElementById.restore()
 })
diff --git a/src/index.test.js b/src/index.test.js
index 4ce73387..68b43c6e 100644
--- a/src/index.test.js
+++ b/src/index.test.js
@@ -366,3 +366,17 @@ test('select correct focused node when using external state data container', t =
   })
   t.deepEqual(wrapper.state().currentFocus, nodeAllData._id)
 })
+
+test('should not scroll on select', t => {
+  const node = (id, label) => ({ id, label, value: label })
+  const largeTree = [...Array(150).keys()].map(i => node(`id${i}`, `label${i}`))
+  const wrapper = mount(<DropdownTreeSelect data={largeTree} showDropdown="initial" />)
+  const { scrollTop } = wrapper.find('.dropdown-content').getDOMNode()
+
+  t.deepEqual(scrollTop, 0)
+
+  const checkboxes = wrapper.find('.checkbox-item')
+  checkboxes.at(140).simulate('click')
+
+  t.deepEqual(scrollTop, 0)
+})
diff --git a/src/tree/index.js b/src/tree/index.js
index a385c35c..95690fca 100644
--- a/src/tree/index.js
+++ b/src/tree/index.js
@@ -51,14 +51,7 @@ class Tree extends Component {
     const { activeDescendant } = nextProps
     const hasSameActiveDescendant = activeDescendant === this.props.activeDescendant
     this.computeInstanceProps(nextProps, !hasSameActiveDescendant)
-    this.setState({ items: this.allVisibleNodes.slice(0, this.currentPage * this.props.pageSize) }, () => {
-      if (hasSameActiveDescendant) return
-      const { scrollableTarget } = this.state
-      const activeLi = activeDescendant && document && document.getElementById(activeDescendant)
-      if (activeLi && scrollableTarget) {
-        scrollableTarget.scrollTop = activeLi.offsetTop - (scrollableTarget.clientHeight - activeLi.clientHeight) / 2
-      }
-    })
+    this.setState({ items: this.allVisibleNodes.slice(0, this.currentPage * this.props.pageSize) })
   }
 
   componentDidMount = () => {