From 1b8f98f7a61c74a27d1ff469d19ddcbbce43e7d7 Mon Sep 17 00:00:00 2001
From: "Michael S. Molina" <michael.s.molina@gmail.com>
Date: Mon, 28 Nov 2022 18:10:43 -0500
Subject: [PATCH 1/2] feat: Adds overflow to the DropdownContainer popover

---
 .github/CODEOWNERS                            |  1 +
 .../components/DropdownContainer/index.tsx    | 67 +++++++++++++------
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
index 9ca3516f8fb9b..931ede6b484db 100644
--- a/.github/CODEOWNERS
+++ b/.github/CODEOWNERS
@@ -14,6 +14,7 @@
 
 /superset-frontend/src/components/Select/ @michael-s-molina @geido @ktmud
 /superset-frontend/src/components/MetadataBar/ @michael-s-molina
+/superset-frontend/src/components/DropdownContainer/ @michael-s-molina
 
 # Notify Helm Chart maintainers about changes in it
 
diff --git a/superset-frontend/src/components/DropdownContainer/index.tsx b/superset-frontend/src/components/DropdownContainer/index.tsx
index d364d3542ee73..73e57e7ab3469 100644
--- a/superset-frontend/src/components/DropdownContainer/index.tsx
+++ b/superset-frontend/src/components/DropdownContainer/index.tsx
@@ -26,6 +26,7 @@ import React, {
   useLayoutEffect,
   useMemo,
   useState,
+  useRef,
 } from 'react';
 import { css, t, useTheme } from '@superset-ui/core';
 import { useResizeDetector } from 'react-resize-detector';
@@ -35,6 +36,8 @@ import Icons from '../Icons';
 import Button from '../Button';
 import Popover from '../Popover';
 
+const MAX_HEIGHT = 500;
+
 /**
  * Container item.
  */
@@ -104,12 +107,12 @@ const DropdownContainer = forwardRef(
     {
       items,
       onOverflowingStateChange,
-      dropdownContent: getPopoverContent,
-      dropdownRef: popoverRef,
-      dropdownStyle: popoverStyle = {},
-      dropdownTriggerCount: popoverTriggerCount,
-      dropdownTriggerIcon: popoverTriggerIcon,
-      dropdownTriggerText: popoverTriggerText = t('More'),
+      dropdownContent,
+      dropdownRef,
+      dropdownStyle = {},
+      dropdownTriggerCount,
+      dropdownTriggerIcon,
+      dropdownTriggerText = t('More'),
       style,
     }: DropdownContainerProps,
     outerRef: RefObject<Ref>,
@@ -124,6 +127,13 @@ const DropdownContainer = forwardRef(
     // We use React.useState to be able to mock the state in Jest
     const [overflowingIndex, setOverflowingIndex] = React.useState<number>(-1);
 
+    let targetRef = useRef<HTMLDivElement>(null);
+    if (dropdownRef) {
+      targetRef = dropdownRef;
+    }
+
+    const [showOverflow, setShowOverflow] = useState(false);
+
     useLayoutEffect(() => {
       const container = current?.children.item(0);
       if (container) {
@@ -214,7 +224,7 @@ const DropdownContainer = forwardRef(
 
     const popoverContent = useMemo(
       () =>
-        getPopoverContent || overflowingCount ? (
+        dropdownContent || overflowingCount ? (
           <div
             css={css`
               display: flex;
@@ -222,24 +232,37 @@ const DropdownContainer = forwardRef(
               gap: ${theme.gridUnit * 4}px;
             `}
             data-test="dropdown-content"
-            style={popoverStyle}
-            ref={popoverRef}
+            style={dropdownStyle}
+            ref={targetRef}
           >
-            {getPopoverContent
-              ? getPopoverContent(overflowedItems)
+            {dropdownContent
+              ? dropdownContent(overflowedItems)
               : overflowedItems.map(item => item.element)}
           </div>
         ) : null,
       [
-        getPopoverContent,
-        overflowedItems,
+        dropdownContent,
         overflowingCount,
-        popoverRef,
-        popoverStyle,
         theme.gridUnit,
+        dropdownStyle,
+        targetRef,
+        overflowedItems,
       ],
     );
 
+    useLayoutEffect(() => {
+      if (popoverVisible) {
+        // Measures scroll height after rendering the elements
+        setTimeout(() => {
+          if (targetRef.current) {
+            // We only set overflow when there's enough space to display
+            // Select's popovers because they are restrained by the overflow property.
+            setShowOverflow(targetRef.current.scrollHeight > MAX_HEIGHT);
+          }
+        }, 100);
+      }
+    }, [popoverVisible]);
+
     useImperativeHandle(
       outerRef,
       () => ({
@@ -277,15 +300,19 @@ const DropdownContainer = forwardRef(
             visible={popoverVisible}
             onVisibleChange={visible => setPopoverVisible(visible)}
             placement="bottom"
+            overlayInnerStyle={{
+              maxHeight: MAX_HEIGHT,
+              overflow: showOverflow ? 'auto' : 'visible',
+            }}
           >
             <Button buttonStyle="secondary">
-              {popoverTriggerIcon}
-              {popoverTriggerText}
+              {dropdownTriggerIcon}
+              {dropdownTriggerText}
               <Badge
-                count={popoverTriggerCount ?? overflowingCount}
+                count={dropdownTriggerCount ?? overflowingCount}
                 css={css`
-                  margin-left: ${popoverTriggerCount ?? overflowingCount
-                    ? '8px'
+                  margin-left: ${dropdownTriggerCount ?? overflowingCount
+                    ? `${theme.gridUnit * 2}px`
                     : '0'};
                 `}
               />

From 244d5e8da82c4f6242f3d1a35eb661cc51515a5d Mon Sep 17 00:00:00 2001
From: "Michael S. Molina" <michael.s.molina@gmail.com>
Date: Wed, 30 Nov 2022 15:19:58 -0500
Subject: [PATCH 2/2] Makes scroll always visible when there's one

---
 .../DropdownContainer.stories.tsx             |  2 +-
 .../DropdownContainer/Overview.stories.mdx    |  4 +-
 .../components/DropdownContainer/index.tsx    | 90 ++++++++++++-------
 3 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx b/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx
index d72b1bdd39736..4e6f5fedfd6f1 100644
--- a/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx
+++ b/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx
@@ -24,7 +24,7 @@ import Button from '../Button';
 import DropdownContainer, { DropdownContainerProps, Ref } from '.';
 
 export default {
-  title: 'DropdownContainer',
+  title: 'Design System/Components/DropdownContainer',
   component: DropdownContainer,
 };
 
diff --git a/superset-frontend/src/components/DropdownContainer/Overview.stories.mdx b/superset-frontend/src/components/DropdownContainer/Overview.stories.mdx
index 5d3792cc5ff9d..691dfeac7318f 100644
--- a/superset-frontend/src/components/DropdownContainer/Overview.stories.mdx
+++ b/superset-frontend/src/components/DropdownContainer/Overview.stories.mdx
@@ -1,6 +1,6 @@
 import { Meta, Source } from '@storybook/addon-docs';
 
-<Meta title="DropdownContainer/Overview" />
+<Meta title="Design System/Components/DropdownContainer/Overview" />
 
 # Usage
 
@@ -14,4 +14,4 @@ the available width, they are displayed vertically in a dropdown. Some of the co
 
 The component accepts any React element which ensures a high level of variability in Superset.
 
-To check the component in detail and its interactions, check the [DropdownContainer](/story/dropdowncontainer--component) page.
+To check the component in detail and its interactions, check the [DropdownContainer](/story/design-system-components-dropdowncontainer--component) page.
diff --git a/superset-frontend/src/components/DropdownContainer/index.tsx b/superset-frontend/src/components/DropdownContainer/index.tsx
index 73e57e7ab3469..2d0e3af1d6fcf 100644
--- a/superset-frontend/src/components/DropdownContainer/index.tsx
+++ b/superset-frontend/src/components/DropdownContainer/index.tsx
@@ -28,6 +28,7 @@ import React, {
   useState,
   useRef,
 } from 'react';
+import { Global } from '@emotion/react';
 import { css, t, useTheme } from '@superset-ui/core';
 import { useResizeDetector } from 'react-resize-detector';
 import { usePrevious } from 'src/hooks/usePrevious';
@@ -245,7 +246,6 @@ const DropdownContainer = forwardRef(
         overflowingCount,
         theme.gridUnit,
         dropdownStyle,
-        targetRef,
         overflowedItems,
       ],
     );
@@ -294,39 +294,63 @@ const DropdownContainer = forwardRef(
           {notOverflowedItems.map(item => item.element)}
         </div>
         {popoverContent && (
-          <Popover
-            content={popoverContent}
-            trigger="click"
-            visible={popoverVisible}
-            onVisibleChange={visible => setPopoverVisible(visible)}
-            placement="bottom"
-            overlayInnerStyle={{
-              maxHeight: MAX_HEIGHT,
-              overflow: showOverflow ? 'auto' : 'visible',
-            }}
-          >
-            <Button buttonStyle="secondary">
-              {dropdownTriggerIcon}
-              {dropdownTriggerText}
-              <Badge
-                count={dropdownTriggerCount ?? overflowingCount}
-                css={css`
-                  margin-left: ${dropdownTriggerCount ?? overflowingCount
-                    ? `${theme.gridUnit * 2}px`
-                    : '0'};
-                `}
-              />
-              <Icons.DownOutlined
-                iconSize="m"
-                iconColor={theme.colors.grayscale.light1}
-                css={css`
-                  .anticon {
-                    display: flex;
+          <>
+            <Global
+              styles={css`
+                .ant-popover-inner-content {
+                  max-height: ${MAX_HEIGHT}px;
+                  overflow: ${showOverflow ? 'auto' : 'visible'};
+                  padding: ${theme.gridUnit * 3}px ${theme.gridUnit * 4}px;
+
+                  // Some OS versions only show the scroll when hovering.
+                  // These settings will make the scroll always visible.
+                  ::-webkit-scrollbar {
+                    -webkit-appearance: none;
+                    width: 14px;
+                  }
+                  ::-webkit-scrollbar-thumb {
+                    border-radius: 9px;
+                    background-color: ${theme.colors.grayscale.light1};
+                    border: 3px solid transparent;
+                    background-clip: content-box;
+                  }
+                  ::-webkit-scrollbar-track {
+                    background-color: ${theme.colors.grayscale.light4};
+                    border-left: 1px solid ${theme.colors.grayscale.light2};
                   }
-                `}
-              />
-            </Button>
-          </Popover>
+                }
+              `}
+            />
+            <Popover
+              content={popoverContent}
+              trigger="click"
+              visible={popoverVisible}
+              onVisibleChange={visible => setPopoverVisible(visible)}
+              placement="bottom"
+            >
+              <Button buttonStyle="secondary">
+                {dropdownTriggerIcon}
+                {dropdownTriggerText}
+                <Badge
+                  count={dropdownTriggerCount ?? overflowingCount}
+                  css={css`
+                    margin-left: ${dropdownTriggerCount ?? overflowingCount
+                      ? `${theme.gridUnit * 2}px`
+                      : '0'};
+                  `}
+                />
+                <Icons.DownOutlined
+                  iconSize="m"
+                  iconColor={theme.colors.grayscale.light1}
+                  css={css`
+                    .anticon {
+                      display: flex;
+                    }
+                  `}
+                />
+              </Button>
+            </Popover>
+          </>
         )}
       </div>
     );