From 2da0c347dbbbfb846e1b1a883ed801f9e3dbefbe Mon Sep 17 00:00:00 2001
From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com>
Date: Fri, 14 May 2021 15:50:20 -0300
Subject: [PATCH] fix: Fixes top level tabs and automatic scroll (#14624)

(cherry picked from commit 9cb4a4602f1ab7c4d545c54dd98b540928ec2142)
---
 .../DashboardBuilder/DashboardBuilder.tsx     |  2 +-
 .../dashboard/components/dnd/handleHover.js   | 11 +++--
 .../dnd/handleScroll/handleScroll.test.ts     |  1 +
 .../components/dnd/handleScroll/index.ts      | 44 ++++++++++++-------
 .../src/dashboard/util/getDropPosition.js     |  7 +--
 5 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
index 6322e8842b1..484432d6895 100644
--- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
@@ -181,7 +181,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
             depth={DASHBOARD_ROOT_DEPTH}
             index={0}
             orientation="column"
-            onDrop={() => dispatch(handleComponentDrop)}
+            onDrop={dropResult => dispatch(handleComponentDrop(dropResult))}
             editMode={editMode}
             // you cannot drop on/displace tabs if they already exist
             disableDragdrop={!!topLevelTabs}
diff --git a/superset-frontend/src/dashboard/components/dnd/handleHover.js b/superset-frontend/src/dashboard/components/dnd/handleHover.js
index 789a5cc14b2..71862a66c5b 100644
--- a/superset-frontend/src/dashboard/components/dnd/handleHover.js
+++ b/superset-frontend/src/dashboard/components/dnd/handleHover.js
@@ -17,7 +17,8 @@
  * under the License.
  */
 import { throttle } from 'lodash';
-import getDropPosition from '../../util/getDropPosition';
+import { DASHBOARD_ROOT_TYPE } from 'src/dashboard/util/componentTypes';
+import getDropPosition from 'src/dashboard/util/getDropPosition';
 import handleScroll from './handleScroll';
 
 const HOVER_THROTTLE_MS = 100;
@@ -28,9 +29,13 @@ function handleHover(props, monitor, Component) {
 
   const dropPosition = getDropPosition(monitor, Component);
 
-  handleScroll(dropPosition);
+  const isDashboardRoot =
+    Component?.props?.component?.type === DASHBOARD_ROOT_TYPE;
+  const scroll = isDashboardRoot ? 'SCROLL_TOP' : null;
 
-  if (!dropPosition || dropPosition === 'SCROLL_TOP') {
+  handleScroll(scroll);
+
+  if (!dropPosition) {
     Component.setState(() => ({ dropIndicator: null }));
     return;
   }
diff --git a/superset-frontend/src/dashboard/components/dnd/handleScroll/handleScroll.test.ts b/superset-frontend/src/dashboard/components/dnd/handleScroll/handleScroll.test.ts
index 8ce057d3215..17908c49095 100644
--- a/superset-frontend/src/dashboard/components/dnd/handleScroll/handleScroll.test.ts
+++ b/superset-frontend/src/dashboard/components/dnd/handleScroll/handleScroll.test.ts
@@ -28,6 +28,7 @@ afterAll(() => {
 
 test('calling: "NOT_SCROLL_TOP" ,"SCROLL_TOP", "NOT_SCROLL_TOP"', () => {
   window.scroll = jest.fn();
+  document.documentElement.scrollTop = 500;
 
   handleScroll('NOT_SCROLL_TOP');
 
diff --git a/superset-frontend/src/dashboard/components/dnd/handleScroll/index.ts b/superset-frontend/src/dashboard/components/dnd/handleScroll/index.ts
index 66d801f47f1..9609edb011b 100644
--- a/superset-frontend/src/dashboard/components/dnd/handleScroll/index.ts
+++ b/superset-frontend/src/dashboard/components/dnd/handleScroll/index.ts
@@ -20,22 +20,34 @@ let scrollTopDashboardInterval: any;
 const SCROLL_STEP = 120;
 const INTERVAL_DELAY = 50;
 
-export default function handleScroll(dropPosition: string) {
-  if (dropPosition === 'SCROLL_TOP') {
-    if (!scrollTopDashboardInterval) {
-      scrollTopDashboardInterval = setInterval(() => {
-        let scrollTop = document.documentElement.scrollTop - SCROLL_STEP;
-        if (scrollTop < 0) {
-          scrollTop = 0;
-        }
-        window.scroll({
-          top: scrollTop,
-          behavior: 'smooth',
-        });
-      }, INTERVAL_DELAY);
-    }
-  }
-  if (dropPosition !== 'SCROLL_TOP' && scrollTopDashboardInterval) {
+export default function handleScroll(scroll: string) {
+  const setupScroll =
+    scroll === 'SCROLL_TOP' &&
+    !scrollTopDashboardInterval &&
+    document.documentElement.scrollTop !== 0;
+
+  const clearScroll =
+    scrollTopDashboardInterval &&
+    (scroll !== 'SCROLL_TOP' || document.documentElement.scrollTop === 0);
+
+  if (setupScroll) {
+    scrollTopDashboardInterval = setInterval(() => {
+      if (document.documentElement.scrollTop === 0) {
+        clearInterval(scrollTopDashboardInterval);
+        scrollTopDashboardInterval = null;
+        return;
+      }
+
+      let scrollTop = document.documentElement.scrollTop - SCROLL_STEP;
+      if (scrollTop < 0) {
+        scrollTop = 0;
+      }
+      window.scroll({
+        top: scrollTop,
+        behavior: 'smooth',
+      });
+    }, INTERVAL_DELAY);
+  } else if (clearScroll) {
     clearInterval(scrollTopDashboardInterval);
     scrollTopDashboardInterval = null;
   }
diff --git a/superset-frontend/src/dashboard/util/getDropPosition.js b/superset-frontend/src/dashboard/util/getDropPosition.js
index b2b2ec5093a..49c70dc9e59 100644
--- a/superset-frontend/src/dashboard/util/getDropPosition.js
+++ b/superset-frontend/src/dashboard/util/getDropPosition.js
@@ -17,13 +17,12 @@
  * under the License.
  */
 import isValidChild from './isValidChild';
-import { DASHBOARD_ROOT_TYPE, TAB_TYPE, TABS_TYPE } from './componentTypes';
+import { TAB_TYPE, TABS_TYPE } from './componentTypes';
 
 export const DROP_TOP = 'DROP_TOP';
 export const DROP_RIGHT = 'DROP_RIGHT';
 export const DROP_BOTTOM = 'DROP_BOTTOM';
 export const DROP_LEFT = 'DROP_LEFT';
-export const SCROLL_TOP = 'SCROLL_TOP';
 
 // this defines how close the mouse must be to the edge of a component to display
 // a sibling type drop indicator
@@ -55,10 +54,6 @@ export default function getDropPosition(monitor, Component) {
     return null;
   }
 
-  if (component.type === DASHBOARD_ROOT_TYPE) {
-    return SCROLL_TOP;
-  }
-
   // TODO need a better solution to prevent nested tabs
   if (
     draggingItem.type === TABS_TYPE &&