From 302205b7b0af8211ed7345dbafe9482e24085d71 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Fri, 2 Dec 2022 10:59:42 -0500 Subject: [PATCH 1/2] fix: DropdownContainer resize algorithm --- .../DropdownContainer.stories.tsx | 21 +++++++------ .../components/DropdownContainer/index.tsx | 31 ++++++++++++------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx b/superset-frontend/src/components/DropdownContainer/DropdownContainer.stories.tsx index 4e6f5fedfd6f..4aeaa03a16f3 100644 --- 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([]); const [overflowingState, setOverflowingState] = useState(); const containerRef = React.useRef(null); - - useEffect(() => { - setItems(generateItems(overflowingState)); - }, [overflowingState]); + const onOverflowingStateChange = useCallback( + value => { + if (!isEqual(overflowingState, value)) { + setItems(generateItems(value)); + setOverflowingState(value); + } + }, + [overflowingState], + ); return (
@@ -86,11 +91,7 @@ export const Component = (props: DropdownContainerProps) => { { - if (!isEqual(overflowingState, value)) { - setOverflowingState(value); - } - }} + onOverflowingStateChange={onOverflowingStateChange} ref={containerRef} />
diff --git a/superset-frontend/src/components/DropdownContainer/index.tsx b/superset-frontend/src/components/DropdownContainer/index.tsx index 92a2ff82e74a..5db223b08a64 100644 --- a/superset-frontend/src/components/DropdownContainer/index.tsx +++ b/superset-frontend/src/components/DropdownContainer/index.tsx @@ -142,33 +142,42 @@ const DropdownContainer = forwardRef( const childrenArray = Array.from(children); // Stores items width once - if (itemsWidth.length === 0) { + if (itemsWidth.length === 0 && childrenArray.length > 0) { setItemsWidth( childrenArray.map(child => child.getBoundingClientRect().width), ); } // 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, + container.getBoundingClientRect().right + 1, ); - setOverflowingIndex(index === -1 ? children.length : index); - if (width > previousWidth && overflowingIndex !== -1) { + let newOverflowingIndex = index === -1 ? children.length : index; + + if (width > previousWidth && newOverflowingIndex !== -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); + + // Checks if some elements in the dropdown fits in the remaining space + let sum = 0; + for (let i = newOverflowingIndex; i < items.length; i += 1) { + sum += itemsWidth[i]; + if (sum <= remainingSpace) { + newOverflowingIndex = i + 1; + } else { + break; + } } } + + setOverflowingIndex(newOverflowingIndex); } }, [ current, @@ -206,7 +215,7 @@ const DropdownContainer = forwardRef( const [overflowedItems, overflowedIds] = useMemo( () => overflowingIndex !== -1 - ? reduceItems(items.slice(overflowingIndex, items.length)) + ? reduceItems(items.slice(overflowingIndex)) : [[], []], [items, overflowingIndex], ); @@ -296,7 +305,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} From 0c9d162c6315dc34420b50febac6513d32ec7c02 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Fri, 2 Dec 2022 18:11:51 -0500 Subject: [PATCH 2/2] Accounts for items change --- .../components/DropdownContainer/index.tsx | 98 +++++++++++-------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/superset-frontend/src/components/DropdownContainer/index.tsx b/superset-frontend/src/components/DropdownContainer/index.tsx index 5db223b08a64..29380d4a9350 100644 --- a/superset-frontend/src/components/DropdownContainer/index.tsx +++ b/superset-frontend/src/components/DropdownContainer/index.tsx @@ -135,18 +135,56 @@ 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); + const reduceItems = (items: Item[]): [Item[], string[]] => + items.reduce( + ([items, ids], item) => { + items.push({ + id: item.id, + element: React.cloneElement(item.element, { key: item.id }), + }); + ids.push(item.id); + return [items, ids]; + }, + [[], []] as [Item[], string[]], + ); + + const [notOverflowedItems, notOverflowedIds] = useMemo( + () => + reduceItems( + items.slice( + 0, + overflowingIndex !== -1 ? overflowingIndex : items.length, + ), + ), + [items, overflowingIndex], + ); - // Stores items width once - if (itemsWidth.length === 0 && childrenArray.length > 0) { + const [overflowedItems, overflowedIds] = useMemo( + () => + overflowingIndex !== -1 + ? 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 @@ -156,9 +194,15 @@ const DropdownContainer = forwardRef( container.getBoundingClientRect().right + 1, ); - let newOverflowingIndex = index === -1 ? children.length : index; + // 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 && newOverflowingIndex !== -1) { + if (width > previousWidth) { // Calculates remaining space in the container const button = current?.children.item(1); const buttonRight = button?.getBoundingClientRect().right || 0; @@ -167,7 +211,7 @@ const DropdownContainer = forwardRef( // Checks if some elements in the dropdown fits in the remaining space let sum = 0; - for (let i = newOverflowingIndex; i < items.length; i += 1) { + for (let i = childrenArray.length; i < items.length; i += 1) { sum += itemsWidth[i]; if (sum <= remainingSpace) { newOverflowingIndex = i + 1; @@ -183,43 +227,11 @@ const DropdownContainer = forwardRef( current, items.length, itemsWidth, - overflowingIndex, + overflowedItems.length, previousWidth, width, ]); - const reduceItems = (items: Item[]): [Item[], string[]] => - items.reduce( - ([items, ids], item) => { - items.push({ - id: item.id, - element: React.cloneElement(item.element, { key: item.id }), - }); - ids.push(item.id); - return [items, ids]; - }, - [[], []] as [Item[], string[]], - ); - - const [notOverflowedItems, notOverflowedIds] = useMemo( - () => - reduceItems( - items.slice( - 0, - overflowingIndex !== -1 ? overflowingIndex : items.length, - ), - ), - [items, overflowingIndex], - ); - - const [overflowedItems, overflowedIds] = useMemo( - () => - overflowingIndex !== -1 - ? reduceItems(items.slice(overflowingIndex)) - : [[], []], - [items, overflowingIndex], - ); - useEffect(() => { if (onOverflowingStateChange) { onOverflowingStateChange({