From 20fb25df8dc16bcb49331d2b6d5a9b6f4869acf9 Mon Sep 17 00:00:00 2001
From: Miguel Alatzar <migbot@users.noreply.github.com>
Date: Thu, 19 Sep 2019 12:39:47 -0700
Subject: [PATCH] Call scrollToIndex only when ref is set and index is in range
 (#3285)

---
 app/components/post_list/post_list.js      | 31 ++++++++++++----------
 app/components/post_list/post_list.test.js | 26 ++++++++++++++++++
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/app/components/post_list/post_list.js b/app/components/post_list/post_list.js
index d55b77126..588e8ae8c 100644
--- a/app/components/post_list/post_list.js
+++ b/app/components/post_list/post_list.js
@@ -299,30 +299,33 @@ export default class PostList extends PureComponent {
 
     scrollToBottom = () => {
         setTimeout(() => {
-            if (this.flatListRef && this.flatListRef.current) {
+            if (this.flatListRef?.current) {
                 this.flatListRef.current.scrollToOffset({offset: 0, animated: true});
             }
         }, 250);
     };
 
+    flatListScrollToIndex = (index) => {
+        this.flatListRef.current.scrollToIndex({
+            animated: false,
+            index,
+            viewOffset: 0,
+            viewPosition: 1, // 0 is at bottom
+        });
+    }
+
     scrollToIndex = (index) => {
-        if (this.flatListRef?.current) {
-            this.animationFrameInitialIndex = requestAnimationFrame(() => {
-                this.flatListRef.current.scrollToIndex({
-                    animated: false,
-                    index,
-                    viewOffset: 0,
-                    viewPosition: 1, // 0 is at bottom
-                });
-            });
-        }
+        this.animationFrameInitialIndex = requestAnimationFrame(() => {
+            if (this.flatListRef?.current && index > 0 && index <= this.getItemCount()) {
+                this.flatListScrollToIndex(index);
+            }
+        });
     };
 
     scrollToInitialIndexIfNeeded = (index, count = 0) => {
-        if (!this.hasDoneInitialScroll && this.flatListRef?.current) {
-            this.hasDoneInitialScroll = true;
-
+        if (!this.hasDoneInitialScroll) {
             if (index > 0 && index <= this.getItemCount()) {
+                this.hasDoneInitialScroll = true;
                 this.scrollToIndex(index);
             } else if (count < 3) {
                 setTimeout(() => {
diff --git a/app/components/post_list/post_list.test.js b/app/components/post_list/post_list.test.js
index a4219bf89..ffcf4e76b 100644
--- a/app/components/post_list/post_list.test.js
+++ b/app/components/post_list/post_list.test.js
@@ -55,4 +55,30 @@ describe('PostList', () => {
         expect(baseProps.actions.handleSelectChannelByName).toHaveBeenCalled();
         expect(wrapper.getElement()).toMatchSnapshot();
     });
+
+    test('should call flatListScrollToIndex only when ref is set and index is in range', () => {
+        jest.spyOn(global, 'requestAnimationFrame').mockImplementation((cb) => cb());
+
+        const instance = wrapper.instance();
+        const flatListScrollToIndex = jest.spyOn(instance, 'flatListScrollToIndex');
+        const indexInRange = baseProps.postIds.length;
+        const indexOutOfRange = [-1, indexInRange + 1];
+
+        instance.flatListRef = {};
+        instance.scrollToIndex(indexInRange);
+        expect(flatListScrollToIndex).not.toHaveBeenCalled();
+
+        instance.flatListRef = {
+            current: {
+                scrollToIndex: jest.fn(),
+            },
+        };
+        for (const index of indexOutOfRange) {
+            instance.scrollToIndex(index);
+            expect(flatListScrollToIndex).not.toHaveBeenCalled();
+        }
+
+        instance.scrollToIndex(indexInRange);
+        expect(flatListScrollToIndex).toHaveBeenCalled();
+    });
 });