Merge b83961a into db0e59c

dowjones · Aug 5, 2020 · 2e9c39c · 2e9c39c
2 parents db0e59c + b83961a
commit 2e9c39c
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 15 deletions.
diff --git a/src/index.js b/src/index.js
@@ -255,7 +255,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
@@ -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
@@ -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
@@ -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 = () => {