fix: DropdownContainer resize algorithm (#22318)

apache · Dec 4, 2022 · aba3b81 · aba3b81
1 parent d881c5d
commit aba3b81
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 56 deletions.
diff --git a/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx b/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useEffect, useState } from 'react';
+import React, { useCallback, useState } from 'react';
 import { isEqual } from 'lodash';
 import { css } from '@superset-ui/core';
 import Select from '../Select/Select';
@@ -63,10 +63,15 @@ export const Component = (props: DropdownContainerProps) => {
   const [items, setItems] = useState<ItemsType>([]);
   const [overflowingState, setOverflowingState] = useState<OverflowingState>();
   const containerRef = React.useRef<Ref>(null);
-
-  useEffect(() => {
-    setItems(generateItems(overflowingState));
-  }, [overflowingState]);
+  const onOverflowingStateChange = useCallback(
+    value => {
+      if (!isEqual(overflowingState, value)) {
+        setItems(generateItems(value));
+        setOverflowingState(value);
+      }
+    },
+    [overflowingState],
+  );
 
   return (
     <div>
@@ -86,11 +91,7 @@ export const Component = (props: DropdownContainerProps) => {
         <DropdownContainer
           {...props}
           items={items}
-          onOverflowingStateChange={value => {
-            if (!isEqual(overflowingState, value)) {
-              setOverflowingState(value);
-            }
-          }}
+          onOverflowingStateChange={onOverflowingStateChange}
           ref={containerRef}
         />
       </div>

diff --git a/superset-frontend/src/components/DropdownContainer/index.tsx b/superset-frontend/src/components/DropdownContainer/index.tsx
@@ -135,50 +135,6 @@ const DropdownContainer = forwardRef(
 
     const [showOverflow, setShowOverflow] = useState(false);
 
-    useLayoutEffect(() => {
-      const container = current?.children.item(0);
-      if (container) {
-        const { children } = container;
-        const childrenArray = Array.from(children);
-
-        // Stores items width once
-        if (itemsWidth.length === 0) {
-          setItemsWidth(
-            childrenArray.map(child => child.getBoundingClientRect().width),
-          );
-        }
-
-        // Calculates the index of the first overflowed element
-        const index = childrenArray.findIndex(
-          child =>
-            child.getBoundingClientRect().right >
-            container.getBoundingClientRect().right,
-        );
-        setOverflowingIndex(index === -1 ? children.length : index);
-
-        if (width > previousWidth && overflowingIndex !== -1) {
-          // Calculates remaining space in the container
-          const button = current?.children.item(1);
-          const buttonRight = button?.getBoundingClientRect().right || 0;
-          const containerRight = current?.getBoundingClientRect().right || 0;
-          const remainingSpace = containerRight - buttonRight;
-          // Checks if the first element in the dropdown fits in the remaining space
-          const fitsInRemainingSpace = remainingSpace >= itemsWidth[0];
-          if (fitsInRemainingSpace && overflowingIndex < items.length) {
-            // Moves element from dropdown to container
-            setOverflowingIndex(overflowingIndex + 1);
-          }
-        }
-      }
-    }, [
-      current,
-      items.length,
-      itemsWidth,
-      overflowingIndex,
-      previousWidth,
-      width,
-    ]);
-
     const reduceItems = (items: Item[]): [Item[], string[]] =>
       items.reduce(
         ([items, ids], item) => {
@@ -206,11 +162,76 @@ const DropdownContainer = forwardRef(
     const [overflowedItems, overflowedIds] = useMemo(
       () =>
         overflowingIndex !== -1
-          ? reduceItems(items.slice(overflowingIndex, items.length))
+          ? reduceItems(items.slice(overflowingIndex))
           : [[], []],
       [items, overflowingIndex],
     );
 
+    useEffect(() => {
+      if (itemsWidth.length !== items.length) {
+        const container = current?.children.item(0);
+        if (container) {
+          const { children } = container;
+          const childrenArray = Array.from(children);
+          setItemsWidth(
+            childrenArray.map(child => child.getBoundingClientRect().width),
+          );
+        }
+      }
+    }, [current?.children, items.length, itemsWidth.length]);
+
+    useLayoutEffect(() => {
+      const container = current?.children.item(0);
+      if (container) {
+        const { children } = container;
+        const childrenArray = Array.from(children);
+
+        // Calculates the index of the first overflowed element
+        // +1 is to give at least one pixel of difference and avoid flakiness
+        const index = childrenArray.findIndex(
+          child =>
+            child.getBoundingClientRect().right >
+            container.getBoundingClientRect().right + 1,
+        );
+
+        // If elements fit (-1) and there's overflowed items
+        // then preserve the overflow index. We can't use overflowIndex
+        // directly because the items may have been modified
+        let newOverflowingIndex =
+          index === -1 && overflowedItems.length > 0
+            ? items.length - overflowedItems.length
+            : index;
+
+        if (width > previousWidth) {
+          // Calculates remaining space in the container
+          const button = current?.children.item(1);
+          const buttonRight = button?.getBoundingClientRect().right || 0;
+          const containerRight = current?.getBoundingClientRect().right || 0;
+          const remainingSpace = containerRight - buttonRight;
+
+          // Checks if some elements in the dropdown fits in the remaining space
+          let sum = 0;
+          for (let i = childrenArray.length; i < items.length; i += 1) {
+            sum += itemsWidth[i];
+            if (sum <= remainingSpace) {
+              newOverflowingIndex = i + 1;
+            } else {
+              break;
+            }
+          }
+        }
+
+        setOverflowingIndex(newOverflowingIndex);
+      }
+    }, [
+      current,
+      items.length,
+      itemsWidth,
+      overflowedItems.length,
+      previousWidth,
+      width,
+    ]);
+
     useEffect(() => {
       if (onOverflowingStateChange) {
         onOverflowingStateChange({
@@ -296,7 +317,7 @@ const DropdownContainer = forwardRef(
             align-items: center;
             gap: ${theme.gridUnit * 4}px;
             margin-right: ${theme.gridUnit * 3}px;
-            min-width: 100px;
+            min-width: 0px;
           `}
           data-test="container"
           style={style}