From e6aad39bc00ba61690e17f2421cfa4852d054700 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Mon, 9 Aug 2021 17:22:10 -0500 Subject: [PATCH 01/20] Prevent generating duplicate cities --- src/components/Dropdown/Dropdown.stories.tsx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 3ef793893..7172c5a81 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -11,19 +11,20 @@ import Label from '../Label'; import { colors } from '../../index'; const generateCityList = (amount: number): OptionProps[] => { - const finalData = []; + const tempData: string[] = []; for (let i = 0; i < amount; i += 1) { - const item = address.city(); - finalData.push({ - id: item.toLowerCase(), - optionValue: item, - }); + let item = address.city(); + while (tempData.includes(item)) item = address.city(); + tempData.push(address.city()); } - return finalData; + return tempData.map(item => ({ + id: item.toLowerCase(), + optionValue: item, + })); }; -const cities = generateCityList(50); +const cities = generateCityList(1000); export const Basic: Story = args => { const [values, setValues] = useState<(string | number)[] | undefined>(); From 80ce1735c0ca22f838282f68ac6be733555293ac Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Mon, 9 Aug 2021 17:23:13 -0500 Subject: [PATCH 02/20] Show value in `onSelect` action --- src/components/Dropdown/Dropdown.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 7172c5a81..f57b1666c 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -36,7 +36,7 @@ export const Basic: Story = args => { onClear={args.clearable ? args.onClear : undefined} onSelect={(selected?: Array) => { setValues(selected); - return args.onSelect(); + return args.onSelect(selected); }} options={cities} values={values} @@ -106,7 +106,7 @@ export const Icons: Story = args => { name="tea-rank" onSelect={(selected?: Array) => { setValues(selected); - return args.onSelect(); + return args.onSelect(selected); }} options={teaOptions} values={values} From 0004a9b84ba6b31685b438ad652cc1cbc112adcd Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Mon, 9 Aug 2021 17:30:58 -0500 Subject: [PATCH 03/20] Fix placeholder for `Icons` story --- src/components/Dropdown/Dropdown.stories.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index f57b1666c..89f05e6d3 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -116,6 +116,7 @@ export const Icons: Story = args => { }; Icons.args = { ...Basic.args, + placeholder: 'Choose a rating...', color: '#0A7700', elevation: 1, }; From 805ed048e9f360f793c60fba79bbf196b0d1d6f6 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Mon, 9 Aug 2021 17:34:43 -0500 Subject: [PATCH 04/20] feat(dropdown): virtualize options list for better performance of long lists Use `react-virtuoso` to virtualize the option items in the dropdown. This is done to reduce the amount of time to render the initial view and process updates. fix #184 --- package.json | 1 + src/components/Dropdown/Dropdown.tsx | 51 ++++++++++++++++++++-------- yarn.lock | 23 +++++++++++++ 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index fa2ec8bc5..ff650cef3 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "nanoid": "^3.1.20", "polished": "^4.1.1", "react-portal": "^4.2.1", + "react-virtuoso": "^1.10.5", "src": "^1.1.2" }, "peerDependencies": { diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 78e86187c..434d63816 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -4,6 +4,7 @@ import Icon from '@mdi/react'; import { mdiCheck, mdiClose, mdiMenuDown, mdiMenuUp } from '@mdi/js'; import { shade, tint, getLuminance, darken, readableColor } from 'polished'; +import { Components, Virtuoso } from 'react-virtuoso'; import { useTheme } from '../../context'; import Button from '../Button/Button'; import variants from '../../enums/variants'; @@ -86,7 +87,7 @@ const OptionsContainer = styled(Div)` position: absolute; top: 100%; left: 0px; - max-height: 10rem; + height: 10rem; overflow-y: auto; width: 15rem; ${ @@ -481,6 +482,34 @@ const Dropdown = ({ ); + const VirtuosoComponents = useMemo( + () => ({ + Scroller: React.forwardRef(({ children }, listRef) => ( + , + ])} + {...optionsContainerProps} + > + {children} + + )), + }), + [ + defaultedColor, + optionsContainerProps, + optionsContainerRef, + optionsScrollListenerCallbackRef, + optionsVariant, + ], + ); + return ( {isOpen && ( - - {options.map(option => ( + ( {option.optionValue} - ))} - + )} + /> )} ); diff --git a/yarn.lock b/yarn.lock index dad58bbae..389c99951 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4083,6 +4083,18 @@ dependencies: eslint-visitor-keys "^1.1.0" +"@virtuoso.dev/react-urx@^0.2.5": + version "0.2.6" + resolved "https://registry.yarnpkg.com/@virtuoso.dev/react-urx/-/react-urx-0.2.6.tgz#e1d8bc717723b2fc23d80ea4e07703dbc276448b" + integrity sha512-+PLQ2iWmSH/rW7WGPEf+Kkql+xygHFL43Jij5aREde/O9mE0OFFGqeetA2a6lry3LDVWzupPntvvWhdaYw0TyA== + dependencies: + "@virtuoso.dev/urx" "^0.2.6" + +"@virtuoso.dev/urx@^0.2.5", "@virtuoso.dev/urx@^0.2.6": + version "0.2.6" + resolved "https://registry.yarnpkg.com/@virtuoso.dev/urx/-/urx-0.2.6.tgz#0028c49e52037e673993900d32abea83262fbd53" + integrity sha512-EKJ0WvJgWaXIz6zKbh9Q63Bcq//p8OHXHbdz4Fy+ruhjJCyI8ADE8E5gwSqBoUchaiYlgwKrT+sX4L2h/H+hMg== + "@webassemblyjs/ast@1.11.1": version "1.11.1" resolved "https://registry.yarnpkg.com/@webassemblyjs/ast/-/ast-1.11.1.tgz#2bfd767eae1a6996f432ff7e8d7fc75679c0b6a7" @@ -15903,6 +15915,15 @@ react-use-measure@^2.0.3: dependencies: debounce "^1.2.0" +react-virtuoso@^1.10.5: + version "1.10.5" + resolved "https://registry.yarnpkg.com/react-virtuoso/-/react-virtuoso-1.10.5.tgz#8c8da1c3de1e6214437cb2bcaf07c8df19013316" + integrity sha512-UE1KmgjDbJ6pR/ymEgCeOI1mX9ni2I6MwQpQJ6ycRBovaobb0MVBbn7NM3KDE8HiaRXaRm+y0rq9xNhCX1GbsA== + dependencies: + "@virtuoso.dev/react-urx" "^0.2.5" + "@virtuoso.dev/urx" "^0.2.5" + resize-observer-polyfill "^1.5.1" + react@^16.13.1: version "16.14.0" resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" @@ -18825,8 +18846,10 @@ watchpack@^1.6.0, watchpack@^1.7.4: resolved "https://registry.yarnpkg.com/watchpack/-/watchpack-1.7.5.tgz#1267e6c55e0b9b5be44c2023aed5437a2c26c453" integrity sha512-9P3MWk6SrKjHsGkLT2KHXdQ/9SNkyoJbabxnKOoJepsvJjJG8uYTR3yTPxPQvNDI3w4Nz1xnE0TLHK4RIVe/MQ== dependencies: + chokidar "^3.4.1" graceful-fs "^4.1.2" neo-async "^2.5.0" + watchpack-chokidar2 "^2.0.1" optionalDependencies: chokidar "^3.4.1" watchpack-chokidar2 "^2.0.1" From 37802f2ae76e22d4c096b4e5a583bbf857f3585c Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Tue, 10 Aug 2021 11:23:45 -0500 Subject: [PATCH 05/20] #184 Fix keyboard navigation --- src/components/Dropdown/Dropdown.tsx | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 8449285ba..4e33da32d 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -412,9 +412,13 @@ const Dropdown = ({ break; case 'ArrowUp': if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { - const sibling = focusedElement.previousElementSibling as HTMLElement | null; - if (sibling) { - sibling.focus(); + const row = focusedElement.parentNode as HTMLElement | undefined; + const rowPrevSibling = row ? row.previousElementSibling : null; + if (rowPrevSibling) { + const toFocus = rowPrevSibling.children[0] as HTMLElement | undefined; + if (toFocus) { + toFocus.focus(); + } } } break; @@ -424,15 +428,20 @@ const Dropdown = ({ // get parent before nextElementSibling because buttons are nested inside of skeletons const optionsContainer = button ? button.nextElementSibling : null; if (optionsContainer) { - const toFocus = optionsContainer.children[0] as HTMLElement | undefined; + const toFocus = optionsContainer.children[0]?.children[0]?.children[0] + ?.children[0] as HTMLElement | undefined; if (toFocus) { toFocus.focus(); } } } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { - const sibling = focusedElement.nextElementSibling as HTMLElement | null; - if (sibling) { - sibling.focus(); + const row = focusedElement.parentNode as HTMLElement | undefined; + const rowNextSibling = row ? row.nextElementSibling : null; + if (rowNextSibling) { + const toFocus = rowNextSibling.children[0] as HTMLElement | undefined; + if (toFocus) { + toFocus.focus(); + } } } break; @@ -486,7 +495,7 @@ const Dropdown = ({ const VirtuosoComponents = useMemo( () => ({ - Scroller: React.forwardRef(({ children }, listRef) => ( + Scroller: React.forwardRef(({ children }: { children: React.ReactNode }, listRef) => ( Date: Tue, 10 Aug 2021 11:36:29 -0500 Subject: [PATCH 06/20] #184 Set `initialItemCount` to pass tests --- src/components/Dropdown/Dropdown.tsx | 1 + .../__snapshots__/Dropdown.test.tsx.snap | 316 +++++++++++------- 2 files changed, 198 insertions(+), 119 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 4e33da32d..21cb6edab 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -586,6 +586,7 @@ const Dropdown = ({ {isOpen && ( (
- - Bulbasaur - -
-
- - Charmander - -
-
- - Squirtle - +
+
+ + Bulbasaur + +
+
+
+
+ + Charmander + +
+
+
+
+ + Squirtle + +
+
+
@@ -819,7 +845,7 @@ exports[`Dropdown deselects option when clicking on them twice when dropdown is position: absolute; top: 100%; left: 0px; - max-height: 10rem; + height: 10rem; overflow-y: auto; width: 15rem; border: 1px solid #252D34; @@ -941,55 +967,81 @@ exports[`Dropdown deselects option when clicking on them twice when dropdown is role="listbox" >
-
- - Bulbasaur - -
-
-
- - Charmander - -
-
- - Squirtle - +
+
+
+ + Bulbasaur + +
+
+
+
+
+ + Charmander + +
+
+
+
+
+ + Squirtle + +
+
+
@@ -1136,7 +1188,7 @@ exports[`Dropdown displays all options when focused 1`] = ` position: absolute; top: 100%; left: 0px; - max-height: 10rem; + height: 10rem; overflow-y: auto; width: 15rem; border: 1px solid #252D34; @@ -1240,43 +1292,69 @@ exports[`Dropdown displays all options when focused 1`] = ` role="listbox" >
- - Bulbasaur - -
-
- - Charmander - -
-
- - Squirtle - +
+
+ + Bulbasaur + +
+
+
+
+ + Charmander + +
+
+
+
+ + Squirtle + +
+
+
From 059d4fe263c3272e486841eed7417d16a9182c03 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Tue, 10 Aug 2021 13:29:08 -0500 Subject: [PATCH 07/20] Fix duplicate options prevention --- src/components/Dropdown/Dropdown.stories.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 5536d5784..9d53353de 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -15,8 +15,10 @@ const generateCityList = (amount: number): OptionProps[] => { for (let i = 0; i < amount; i += 1) { let item = address.city(); - while (tempData.includes(item)) item = address.city(); - tempData.push(address.city()); + while (tempData.includes(item)) { + item = address.city(); + } + tempData.push(item); } return tempData.map(item => ({ From 872765829fb23335e486d23aa34d9ddf8bfce812 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Wed, 11 Aug 2021 12:04:33 -0500 Subject: [PATCH 08/20] #184 Only virtualize options when DOM is available --- src/components/Dropdown/Dropdown.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 21cb6edab..e5f40d367 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -586,7 +586,8 @@ const Dropdown = ({ {isOpen && ( ( Date: Wed, 11 Aug 2021 12:17:12 -0500 Subject: [PATCH 09/20] Fix dropdown stories style and performance --- src/components/Dropdown/Dropdown.stories.tsx | 24 +++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 9d53353de..981e1a6e7 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { Story, Meta } from '@storybook/react'; import { address } from 'faker'; @@ -26,16 +26,27 @@ const generateCityList = (amount: number): OptionProps[] => { optionValue: item, })); }; -const cities = generateCityList(1000); -type BasicProps = DropdownProps & { clearable: boolean }; +type BasicProps = DropdownProps & { clearable: boolean; numCities: number }; -export const Basic: Story = ({ clearable, onClear, onSelect }: BasicProps) => { +export const Basic: Story = ({ + clearable, + numCities, + onClear, + onSelect, + ...args +}: BasicProps) => { + const [cities, setCities] = useState([]); const [values, setValues] = useState<(string | number)[] | undefined>(); + useEffect(() => { + setCities(generateCityList(numCities)); + }, [numCities]); + return ( )), }), - [ - defaultedColor, - optionsContainerProps, - optionsContainerRef, - optionsScrollListenerCallbackRef, - optionsVariant, - ], + [defaultedColor, optionsContainerProps, optionsContainerRef, optionsVariant], ); return ( @@ -590,13 +564,15 @@ const Dropdown = ({ {isOpen && ( setScrollIndex(range.startIndex)} + initialTopMostItemIndex={rememberScrollPosition ? scrollIndex : 0} initialItemCount={ typeof window !== 'undefined' && window.document && window.document.createElement ? initialOptionCount : options.length } components={VirtuosoComponents as Components} - itemContent={(index, option) => ( + itemContent={(_index, option) => ( Date: Wed, 11 Aug 2021 22:53:22 -0500 Subject: [PATCH 12/20] #184 Remove virtualization if less than max height --- src/components/Dropdown/Dropdown.stories.tsx | 3 +- src/components/Dropdown/Dropdown.tsx | 287 ++++++++++++------ .../Dropdown/__tests__/Dropdown.test.tsx | 18 +- .../__snapshots__/Dropdown.test.tsx.snap | 120 +++----- 4 files changed, 254 insertions(+), 174 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 981e1a6e7..5dfbbf842 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -69,7 +69,8 @@ Basic.args = { variant: variants.fill, optionsVariant: variants.outline, valueVariant: variants.text, - numCities: 20000, + numCities: 200, + virtualizeOptions: true, }; const teaOptions = [ diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 14ea14233..0cde281d6 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -82,12 +82,12 @@ const ValueItem = styled(Div)` `; const OptionsContainer = styled(Div)` - ${({ color, variant }: UsefulDropdownState) => ` + ${({ color, variant, isVirtual }: UsefulDropdownState & { isVirtual: boolean }) => ` background: white; position: absolute; top: 100%; left: 0px; - height: 10rem; + ${isVirtual ? 'height: 10rem;' : 'max-height: 10rem;'} overflow-y: auto; width: 15rem; ${ @@ -239,7 +239,7 @@ export interface DropdownProps { optionsVariant?: variants; valueVariant?: variants; - initialOptionCount?: number; + virtualizeOptions?: boolean; } const Dropdown = ({ @@ -292,7 +292,7 @@ const Dropdown = ({ valueVariant = variants.text, values = [], - initialOptionCount, + virtualizeOptions = true, }: DropdownProps): JSX.Element | null => { const { colors } = useTheme(); const defaultedColor = color || colors.grayDark; @@ -305,6 +305,12 @@ const Dropdown = ({ const [scrollIndex, setScrollIndex] = useState(0); + const [isVirtual, setIsVirtual] = useState(virtualizeOptions); // TODO: Update if the scroller div is smaller than the max-height + + useEffect(() => { + setIsVirtual(virtualizeOptions); + }, [virtualizeOptions]); + // Merge the default styled container prop and the placeholderProps object to get user styles const placeholderMergedProps = { StyledContainer: PlaceholderContainer, @@ -349,10 +355,34 @@ const Dropdown = ({ } setIsOpen(true); + + window.setTimeout(() => { + const focusedElement = document.activeElement; + + if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { + const button = focusedElement.parentNode as HTMLElement | undefined; + const optionsContainer = button ? button.nextElementSibling : null; + + if (optionsContainer) { + if (isVirtual) { + const lowestContainer = optionsContainer.children[0]?.children[0]?.children[0]; + if (lowestContainer && lowestContainer.clientHeight < optionsContainer.clientHeight) { + setIsVirtual(false); + } + } else if ( + virtualizeOptions && + optionsContainer.scrollHeight > optionsContainer.clientHeight + ) { + setIsVirtual(true); + } + } + } + }, 0); + if (onFocus) { onFocus(); } - }, [onFocus, focusWithin, focusTimeoutId]); + }, [focusTimeoutId, focusWithin, onFocus, name, isVirtual, virtualizeOptions]); const handleSelect = useCallback( (clickedId: string | number) => { @@ -401,54 +431,94 @@ const Dropdown = ({ // to activeElement to after it is updated in the DOM window.setTimeout(() => { const focusedElement = document.activeElement; - switch (key) { - case 'Enter': - const match = focusedElement && focusedElement.id.match(`${name}-option-(.*)`); - if (match) { - handleSelect(match[1]); - } - break; - case 'ArrowUp': - if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { - const row = focusedElement.parentNode as HTMLElement | undefined; - const rowPrevSibling = row ? row.previousElementSibling : null; - if (rowPrevSibling) { - const toFocus = rowPrevSibling.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + + if (isVirtual) { + switch (key) { + case 'Enter': + const match = focusedElement && focusedElement.id.match(`${name}-option-(.*)`); + if (match) { + handleSelect(match[1]); + } + break; + case 'ArrowUp': + if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + const row = focusedElement.parentNode as HTMLElement | undefined; + const rowPrevSibling = row ? row.previousElementSibling : null; + if (rowPrevSibling) { + const toFocus = rowPrevSibling.children[0] as HTMLElement | undefined; + if (toFocus) { + toFocus.focus(); + } } } - } - break; - case 'ArrowDown': - if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { - const button = focusedElement.parentNode as HTMLElement | undefined; - // get parent before nextElementSibling because buttons are nested inside of skeletons - const optionsContainer = button ? button.nextElementSibling : null; - if (optionsContainer) { - const toFocus = optionsContainer.children[0]?.children[0]?.children[0] - ?.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + break; + case 'ArrowDown': + if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { + const button = focusedElement.parentNode as HTMLElement | undefined; + // get parent before nextElementSibling because buttons are nested inside of skeletons + const optionsContainer = button ? button.nextElementSibling : null; + if (optionsContainer) { + const toFocus = optionsContainer.children[0]?.children[0]?.children[0] + ?.children[0] as HTMLElement | undefined; + if (toFocus) { + toFocus.focus(); + } + } + } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + const row = focusedElement.parentNode as HTMLElement | undefined; + const rowNextSibling = row ? row.nextElementSibling : null; + if (rowNextSibling) { + const toFocus = rowNextSibling.children[0] as HTMLElement | undefined; + if (toFocus) { + toFocus.focus(); + } } } - } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { - const row = focusedElement.parentNode as HTMLElement | undefined; - const rowNextSibling = row ? row.nextElementSibling : null; - if (rowNextSibling) { - const toFocus = rowNextSibling.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + break; + default: + break; + } + } else { + switch (key) { + case 'Enter': + const match = focusedElement && focusedElement.id.match(`${name}-option-(.*)`); + if (match) { + handleSelect(match[1]); + } + break; + case 'ArrowUp': + if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + const sibling = focusedElement.previousElementSibling as HTMLElement | null; + if (sibling) { + sibling.focus(); } } - } - break; - default: - break; + break; + case 'ArrowDown': + if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { + const button = focusedElement.parentNode as HTMLElement | undefined; + // get parent before nextElementSibling because buttons are nested inside of skeletons + const optionsContainer = button ? button.nextElementSibling : null; + if (optionsContainer) { + const toFocus = optionsContainer.children[0] as HTMLElement | undefined; + if (toFocus) { + toFocus.focus(); + } + } + } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + const sibling = focusedElement.nextElementSibling as HTMLElement | null; + if (sibling) { + sibling.focus(); + } + } + break; + default: + break; + } } }, 0); }, - [handleSelect, name], + [handleSelect, isVirtual, name], ); useEffect(() => { @@ -484,6 +554,7 @@ const Dropdown = ({ )), }), - [defaultedColor, optionsContainerProps, optionsContainerRef, optionsVariant], + [defaultedColor, optionsContainerProps, optionsContainerRef, optionsVariant, isVirtual], ); return ( @@ -561,48 +632,90 @@ const Dropdown = ({ {closeIcons} - {isOpen && ( - setScrollIndex(range.startIndex)} - initialTopMostItemIndex={rememberScrollPosition ? scrollIndex : 0} - initialItemCount={ - typeof window !== 'undefined' && window.document && window.document.createElement - ? initialOptionCount - : options.length - } - components={VirtuosoComponents as Components} - itemContent={(_index, option) => ( - handleSelect(option.id)} - tabIndex={-1} - color={defaultedColor} - variant={optionsVariant} - multi={multi} - selected={optionsHash[option.id].isSelected} - ref={optionItemRef} - role="option" - {...optionItemProps} - > - {multi && ( - - {optionsHash[option.id].isSelected && } - - )} - {option.optionValue} - - )} - /> - )} + {isOpen && + (isVirtual ? ( + setScrollIndex(range.startIndex)} + initialTopMostItemIndex={ + rememberScrollPosition && scrollIndex < options.length ? scrollIndex : 0 + } + initialItemCount={ + typeof window !== 'undefined' && window.document && window.document.createElement + ? undefined + : options.length + } + components={VirtuosoComponents as Components} + itemContent={(_index, option) => ( + handleSelect(option.id)} + tabIndex={-1} + color={defaultedColor} + variant={optionsVariant} + multi={multi} + selected={optionsHash[option.id].isSelected} + ref={optionItemRef} + role="option" + {...optionItemProps} + > + {multi && ( + + {optionsHash[option.id].isSelected && } + + )} + {option.optionValue} + + )} + /> + ) : ( + + {options.map(option => ( + handleSelect(option.id)} + tabIndex={-1} + color={defaultedColor} + variant={optionsVariant} + multi={multi} + selected={optionsHash[option.id].isSelected} + ref={optionItemRef} + role="option" + {...optionItemProps} + > + {multi && ( + + {optionsHash[option.id].isSelected && } + + )} + {option.optionValue} + + ))} + + ))} ); }; diff --git a/src/components/Dropdown/__tests__/Dropdown.test.tsx b/src/components/Dropdown/__tests__/Dropdown.test.tsx index 47d640a02..44a8dccf9 100644 --- a/src/components/Dropdown/__tests__/Dropdown.test.tsx +++ b/src/components/Dropdown/__tests__/Dropdown.test.tsx @@ -99,11 +99,7 @@ describe('Dropdown', () => { it('can focus dropdown and select option', async () => { const { container, getByText } = render( - , + , ); // TODO - Don't use id, see if we can use a more semantically meaningful element @@ -122,7 +118,7 @@ describe('Dropdown', () => { onSelect={mockedSelectHandler} multi options={pokeOptions} - initialOptionCount={pokeOptions.length} + virtualizeOptions={false} />, ); @@ -143,7 +139,7 @@ describe('Dropdown', () => { onSelect={mockedSelectHandler} multi options={pokeOptions} - initialOptionCount={pokeOptions.length} + virtualizeOptions={false} />, ); @@ -182,11 +178,7 @@ describe('Dropdown', () => { it('can use arrow keys and enter to navigate options', async () => { const { queryByText } = render( - , + , ); act(() => { screen.getByRole('button').focus(); @@ -263,7 +255,7 @@ describe('Dropdown', () => { options={pokeOptions} onSelect={() => {}} optionItemRef={ref} - initialOptionCount={pokeOptions.length} + virtualizeOptions={false} />, ); act(() => { diff --git a/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap b/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap index 1ccb0d855..4764bec7c 100644 --- a/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap +++ b/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap @@ -758,7 +758,7 @@ exports[`Dropdown deselects option when clicking on them twice when dropdown is position: absolute; top: 100%; left: 0px; - height: 10rem; + max-height: 10rem; overflow-y: auto; width: 15rem; border: 1px solid #252D34; @@ -880,81 +880,55 @@ exports[`Dropdown deselects option when clicking on them twice when dropdown is role="listbox" >
+ -
-
-
- - Bulbasaur - -
-
-
-
-
- - Charmander - -
-
-
-
-
- - Squirtle - -
-
-
+ Bulbasaur + +
+
+
+ + Charmander + +
+
+
+ + Squirtle +
From f79835e205a846b1ebf93c8c3bcc1eaa04ab5b00 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Wed, 11 Aug 2021 23:06:00 -0500 Subject: [PATCH 13/20] #184 Adjust naming for nested elements on keypress --- src/components/Dropdown/Dropdown.tsx | 36 ++++++++++++++++------------ 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 0cde281d6..8d70c2366 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -365,8 +365,10 @@ const Dropdown = ({ if (optionsContainer) { if (isVirtual) { - const lowestContainer = optionsContainer.children[0]?.children[0]?.children[0]; - if (lowestContainer && lowestContainer.clientHeight < optionsContainer.clientHeight) { + const virtuosoOuterContainer = optionsContainer.firstElementChild; + const virtuosoInnerContainer = virtuosoOuterContainer?.firstElementChild; + const virtuosoScroller = virtuosoInnerContainer?.firstElementChild; + if (virtuosoScroller && virtuosoScroller.clientHeight < optionsContainer.clientHeight) { setIsVirtual(false); } } else if ( @@ -445,9 +447,9 @@ const Dropdown = ({ const row = focusedElement.parentNode as HTMLElement | undefined; const rowPrevSibling = row ? row.previousElementSibling : null; if (rowPrevSibling) { - const toFocus = rowPrevSibling.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + const prevOption = rowPrevSibling.firstElementChild as HTMLElement | undefined; + if (prevOption) { + prevOption.focus(); } } } @@ -458,19 +460,23 @@ const Dropdown = ({ // get parent before nextElementSibling because buttons are nested inside of skeletons const optionsContainer = button ? button.nextElementSibling : null; if (optionsContainer) { - const toFocus = optionsContainer.children[0]?.children[0]?.children[0] - ?.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + const virtuosoOuterContainer = optionsContainer.firstElementChild; + const virtuosoInnerContainer = virtuosoOuterContainer?.firstElementChild; + const virtuosoScroller = virtuosoInnerContainer?.firstElementChild; + const firstOption = virtuosoScroller?.firstElementChild as + | HTMLElement + | undefined; + if (firstOption) { + firstOption.focus(); } } } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { const row = focusedElement.parentNode as HTMLElement | undefined; const rowNextSibling = row ? row.nextElementSibling : null; if (rowNextSibling) { - const toFocus = rowNextSibling.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + const nextOption = rowNextSibling.firstElementChild as HTMLElement | undefined; + if (nextOption) { + nextOption.focus(); } } } @@ -500,9 +506,9 @@ const Dropdown = ({ // get parent before nextElementSibling because buttons are nested inside of skeletons const optionsContainer = button ? button.nextElementSibling : null; if (optionsContainer) { - const toFocus = optionsContainer.children[0] as HTMLElement | undefined; - if (toFocus) { - toFocus.focus(); + const firstOption = optionsContainer.firstElementChild as HTMLElement | undefined; + if (firstOption) { + firstOption.focus(); } } } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { From 823cb18bbf56e1e9d3f374b38752449e96d54093 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Wed, 11 Aug 2021 23:10:39 -0500 Subject: [PATCH 14/20] Improve city option item variable naming --- src/components/Dropdown/Dropdown.stories.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 5dfbbf842..10ead4853 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -14,11 +14,11 @@ const generateCityList = (amount: number): OptionProps[] => { const tempData: string[] = []; for (let i = 0; i < amount; i += 1) { - let item = address.city(); - while (tempData.includes(item)) { - item = address.city(); + let city = address.city(); + while (tempData.includes(city)) { + city = address.city(); } - tempData.push(item); + tempData.push(city); } return tempData.map(item => ({ From 2aa71d299f43185459bb063c200f83ff2d68aa53 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Thu, 12 Aug 2021 10:54:46 -0500 Subject: [PATCH 15/20] Improve performance of city option generation --- src/components/Dropdown/Dropdown.stories.tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/Dropdown/Dropdown.stories.tsx b/src/components/Dropdown/Dropdown.stories.tsx index 10ead4853..ad430d830 100644 --- a/src/components/Dropdown/Dropdown.stories.tsx +++ b/src/components/Dropdown/Dropdown.stories.tsx @@ -11,20 +11,22 @@ import Label from '../Label'; import { colors } from '../../index'; const generateCityList = (amount: number): OptionProps[] => { - const tempData: string[] = []; + const citySet = new Set(); + const cityOptions: OptionProps[] = []; for (let i = 0; i < amount; i += 1) { let city = address.city(); - while (tempData.includes(city)) { + while (citySet.has(city)) { city = address.city(); } - tempData.push(city); + citySet.add(city); + cityOptions.push({ + id: city.toLowerCase(), + optionValue: city, + }); } - return tempData.map(item => ({ - id: item.toLowerCase(), - optionValue: item, - })); + return cityOptions; }; type BasicProps = DropdownProps & { clearable: boolean; numCities: number }; From 004ed15b590fd33ae269fe12ac33d4c089bb3f21 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Thu, 12 Aug 2021 10:56:10 -0500 Subject: [PATCH 16/20] #184 Add state for whether to virtualize options --- src/components/Dropdown/Dropdown.tsx | 32 ++++++++++++---------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 8d70c2366..5b7d95b52 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -305,11 +305,12 @@ const Dropdown = ({ const [scrollIndex, setScrollIndex] = useState(0); - const [isVirtual, setIsVirtual] = useState(virtualizeOptions); // TODO: Update if the scroller div is smaller than the max-height + const [isOverflowing, setIsOverflowing] = useState(true); + const [isVirtual, setIsVirtual] = useState(virtualizeOptions); useEffect(() => { - setIsVirtual(virtualizeOptions); - }, [virtualizeOptions]); + setIsVirtual(virtualizeOptions && isOverflowing); + }, [virtualizeOptions, isOverflowing]); // Merge the default styled container prop and the placeholderProps object to get user styles const placeholderMergedProps = { @@ -365,17 +366,13 @@ const Dropdown = ({ if (optionsContainer) { if (isVirtual) { - const virtuosoOuterContainer = optionsContainer.firstElementChild; - const virtuosoInnerContainer = virtuosoOuterContainer?.firstElementChild; - const virtuosoScroller = virtuosoInnerContainer?.firstElementChild; + const virtuosoContainer = optionsContainer.firstElementChild; + const virtuosoScroller = virtuosoContainer?.firstElementChild; if (virtuosoScroller && virtuosoScroller.clientHeight < optionsContainer.clientHeight) { - setIsVirtual(false); + setIsOverflowing(false); } - } else if ( - virtualizeOptions && - optionsContainer.scrollHeight > optionsContainer.clientHeight - ) { - setIsVirtual(true); + } else if (optionsContainer.scrollHeight > optionsContainer.clientHeight) { + setIsOverflowing(true); } } } @@ -384,7 +381,7 @@ const Dropdown = ({ if (onFocus) { onFocus(); } - }, [focusTimeoutId, focusWithin, onFocus, name, isVirtual, virtualizeOptions]); + }, [focusTimeoutId, focusWithin, onFocus, name, isVirtual]); const handleSelect = useCallback( (clickedId: string | number) => { @@ -460,10 +457,9 @@ const Dropdown = ({ // get parent before nextElementSibling because buttons are nested inside of skeletons const optionsContainer = button ? button.nextElementSibling : null; if (optionsContainer) { - const virtuosoOuterContainer = optionsContainer.firstElementChild; - const virtuosoInnerContainer = virtuosoOuterContainer?.firstElementChild; - const virtuosoScroller = virtuosoInnerContainer?.firstElementChild; - const firstOption = virtuosoScroller?.firstElementChild as + const virtuosoContainer = optionsContainer.firstElementChild; + const virtuosoScroller = virtuosoContainer?.firstElementChild; + const firstOption = virtuosoScroller?.firstElementChild?.firstElementChild as | HTMLElement | undefined; if (firstOption) { @@ -573,7 +569,7 @@ const Dropdown = ({ )), }), - [defaultedColor, optionsContainerProps, optionsContainerRef, optionsVariant, isVirtual], + [defaultedColor, optionsVariant, isVirtual, optionsContainerRef, optionsContainerProps], ); return ( From ffe6cb33512e54da1a796f436abda128893925ad Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Thu, 12 Aug 2021 11:38:02 -0500 Subject: [PATCH 17/20] #184 Refactor keypress handler and nested scroller --- src/components/Dropdown/Dropdown.tsx | 148 +++++++----------- .../__snapshots__/Dropdown.test.tsx.snap | 4 +- 2 files changed, 59 insertions(+), 93 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 5b7d95b52..e9710d31e 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -431,92 +431,69 @@ const Dropdown = ({ window.setTimeout(() => { const focusedElement = document.activeElement; - if (isVirtual) { - switch (key) { - case 'Enter': - const match = focusedElement && focusedElement.id.match(`${name}-option-(.*)`); - if (match) { - handleSelect(match[1]); - } - break; - case 'ArrowUp': - if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + switch (key) { + case 'Enter': + const match = focusedElement && focusedElement.id.match(`${name}-option-(.*)`); + if (match) { + handleSelect(match[1]); + } + break; + case 'ArrowUp': + if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + let prevOption; + if (isVirtual) { const row = focusedElement.parentNode as HTMLElement | undefined; const rowPrevSibling = row ? row.previousElementSibling : null; if (rowPrevSibling) { - const prevOption = rowPrevSibling.firstElementChild as HTMLElement | undefined; - if (prevOption) { - prevOption.focus(); - } + prevOption = rowPrevSibling.firstElementChild as HTMLElement | undefined; } + } else { + prevOption = focusedElement.previousElementSibling as HTMLElement | null; + } + + if (prevOption) { + prevOption.focus(); } - break; - case 'ArrowDown': - if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { - const button = focusedElement.parentNode as HTMLElement | undefined; - // get parent before nextElementSibling because buttons are nested inside of skeletons - const optionsContainer = button ? button.nextElementSibling : null; - if (optionsContainer) { + } + break; + case 'ArrowDown': + if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { + const button = focusedElement.parentNode as HTMLElement | undefined; + // get parent before nextElementSibling because buttons are nested inside of skeletons + const optionsContainer = button ? button.nextElementSibling : null; + if (optionsContainer) { + let firstOption; + if (isVirtual) { const virtuosoContainer = optionsContainer.firstElementChild; const virtuosoScroller = virtuosoContainer?.firstElementChild; - const firstOption = virtuosoScroller?.firstElementChild?.firstElementChild as + firstOption = virtuosoScroller?.firstElementChild?.firstElementChild as | HTMLElement | undefined; - if (firstOption) { - firstOption.focus(); - } + } else { + firstOption = optionsContainer.firstElementChild as HTMLElement | undefined; + } + if (firstOption) { + firstOption.focus(); } - } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + } + } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { + let nextOption; + if (isVirtual) { const row = focusedElement.parentNode as HTMLElement | undefined; const rowNextSibling = row ? row.nextElementSibling : null; if (rowNextSibling) { - const nextOption = rowNextSibling.firstElementChild as HTMLElement | undefined; - if (nextOption) { - nextOption.focus(); - } + nextOption = rowNextSibling.firstElementChild as HTMLElement | undefined; } + } else { + nextOption = focusedElement.nextElementSibling as HTMLElement | null; } - break; - default: - break; - } - } else { - switch (key) { - case 'Enter': - const match = focusedElement && focusedElement.id.match(`${name}-option-(.*)`); - if (match) { - handleSelect(match[1]); + if (nextOption) { + nextOption.focus(); } - break; - case 'ArrowUp': - if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { - const sibling = focusedElement.previousElementSibling as HTMLElement | null; - if (sibling) { - sibling.focus(); - } - } - break; - case 'ArrowDown': - if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { - const button = focusedElement.parentNode as HTMLElement | undefined; - // get parent before nextElementSibling because buttons are nested inside of skeletons - const optionsContainer = button ? button.nextElementSibling : null; - if (optionsContainer) { - const firstOption = optionsContainer.firstElementChild as HTMLElement | undefined; - if (firstOption) { - firstOption.focus(); - } - } - } else if (focusedElement && focusedElement.id.match(`${name}-option-.*`)) { - const sibling = focusedElement.nextElementSibling as HTMLElement | null; - if (sibling) { - sibling.focus(); - } - } - break; - default: - break; - } + } + break; + default: + break; } }, 0); }, @@ -550,9 +527,9 @@ const Dropdown = ({ ); - const VirtuosoComponents = useMemo( - () => ({ - Scroller: React.forwardRef(({ children }: { children: React.ReactNode }, listRef) => ( + const InternalOptionsContainer = useMemo( + () => + React.forwardRef(({ children }: { children: React.ReactNode }, listRef) => ( )), - }), - [defaultedColor, optionsVariant, isVirtual, optionsContainerRef, optionsContainerProps], + [defaultedColor, isVirtual, optionsContainerProps, optionsContainerRef, optionsVariant], ); return ( @@ -642,16 +618,14 @@ const Dropdown = ({ initialTopMostItemIndex={ rememberScrollPosition && scrollIndex < options.length ? scrollIndex : 0 } - initialItemCount={ - typeof window !== 'undefined' && window.document && window.document.createElement - ? undefined - : options.length + components={ + { + Scroller: InternalOptionsContainer, + } as Components } - components={VirtuosoComponents as Components} itemContent={(_index, option) => ( handleSelect(option.id)} tabIndex={-1} color={defaultedColor} @@ -679,18 +653,10 @@ const Dropdown = ({ )} /> ) : ( - + {options.map(option => ( handleSelect(option.id)} tabIndex={-1} color={defaultedColor} @@ -716,7 +682,7 @@ const Dropdown = ({ {option.optionValue} ))} - + ))} ); diff --git a/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap b/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap index 4764bec7c..2a7f121fb 100644 --- a/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap +++ b/src/components/Dropdown/__tests__/__snapshots__/Dropdown.test.tsx.snap @@ -414,7 +414,7 @@ exports[`Dropdown closes options when clicking outside 1`] = ` style="width: 100%; height: 100%; position: absolute; top: 0px;" >
@@ -1153,7 +1153,7 @@ exports[`Dropdown displays all options when focused 1`] = ` style="width: 100%; height: 100%; position: absolute; top: 0px;" >
From 31fcd452ef365782e1182e51ac07bba6e47d214a Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Thu, 12 Aug 2021 11:44:27 -0500 Subject: [PATCH 18/20] #184 Remember scroll position for non-virtual list --- src/components/Dropdown/Dropdown.tsx | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index e9710d31e..93979e909 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -303,6 +303,8 @@ const Dropdown = ({ const [focusWithin, setFocusWithin] = useState(false); const [focusTimeoutId, setFocusTimeoutId] = useState(); + const scrollPos = useRef(0); + const [scrollIndex, setScrollIndex] = useState(0); const [isOverflowing, setIsOverflowing] = useState(true); @@ -329,6 +331,12 @@ const Dropdown = ({ return hash; }, [options, values]); + const scrollListener = () => { + scrollPos.current = optionsContainerInternalRef.current + ? optionsContainerInternalRef.current.scrollTop + : 0; + }; + const handleBlur = useCallback( (e: React.FocusEvent) => { e.preventDefault(); @@ -508,6 +516,19 @@ const Dropdown = ({ }; }, [keyDownHandler]); + const optionsScrollListenerCallbackRef = useCallback( + node => { + if (node && rememberScrollPosition) { + node.addEventListener('scroll', scrollListener, true); + + if (scrollPos.current) { + node.scrollTop = scrollPos.current; + } + } + }, + [rememberScrollPosition], + ); + const closeIcons = ( <> {onClear && values.length > 0 && ( @@ -626,6 +647,7 @@ const Dropdown = ({ itemContent={(_index, option) => ( handleSelect(option.id)} tabIndex={-1} color={defaultedColor} @@ -653,10 +675,11 @@ const Dropdown = ({ )} /> ) : ( - + {options.map(option => ( handleSelect(option.id)} tabIndex={-1} color={defaultedColor} From 11e7cd4fafbbf6974d57d0dc96621e5b1d4d24d8 Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Thu, 12 Aug 2021 14:03:02 -0500 Subject: [PATCH 19/20] #184 Remove extra timeout and document navigation --- src/components/Dropdown/Dropdown.tsx | 30 ++++++++++------------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index 93979e909..e6c00946c 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -365,31 +365,23 @@ const Dropdown = ({ setIsOpen(true); - window.setTimeout(() => { - const focusedElement = document.activeElement; - - if (focusedElement && focusedElement.id === `${name}-dropdown-button`) { - const button = focusedElement.parentNode as HTMLElement | undefined; - const optionsContainer = button ? button.nextElementSibling : null; - - if (optionsContainer) { - if (isVirtual) { - const virtuosoContainer = optionsContainer.firstElementChild; - const virtuosoScroller = virtuosoContainer?.firstElementChild; - if (virtuosoScroller && virtuosoScroller.clientHeight < optionsContainer.clientHeight) { - setIsOverflowing(false); - } - } else if (optionsContainer.scrollHeight > optionsContainer.clientHeight) { - setIsOverflowing(true); - } + const optionsContainer = optionsContainerInternalRef.current; + if (optionsContainer) { + if (isVirtual) { + const virtuosoContainer = optionsContainer.firstElementChild; + const virtuosoScroller = virtuosoContainer?.firstElementChild; + if (virtuosoScroller && virtuosoScroller.clientHeight < optionsContainer.clientHeight) { + setIsOverflowing(false); } + } else if (optionsContainer.scrollHeight > optionsContainer.clientHeight) { + setIsOverflowing(true); } - }, 0); + } if (onFocus) { onFocus(); } - }, [focusTimeoutId, focusWithin, onFocus, name, isVirtual]); + }, [focusTimeoutId, focusWithin, onFocus, isVirtual]); const handleSelect = useCallback( (clickedId: string | number) => { From 2c9c7a0ab5406eb4d1b30225e08d2d93767b893b Mon Sep 17 00:00:00 2001 From: Dawson Booth Date: Thu, 12 Aug 2021 14:35:04 -0500 Subject: [PATCH 20/20] #184 Add open/close timeout to improve performance --- src/components/Dropdown/Dropdown.tsx | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx index e6c00946c..cb1e59670 100644 --- a/src/components/Dropdown/Dropdown.tsx +++ b/src/components/Dropdown/Dropdown.tsx @@ -365,18 +365,20 @@ const Dropdown = ({ setIsOpen(true); - const optionsContainer = optionsContainerInternalRef.current; - if (optionsContainer) { - if (isVirtual) { - const virtuosoContainer = optionsContainer.firstElementChild; - const virtuosoScroller = virtuosoContainer?.firstElementChild; - if (virtuosoScroller && virtuosoScroller.clientHeight < optionsContainer.clientHeight) { - setIsOverflowing(false); + window.setTimeout(() => { + const optionsContainer = optionsContainerInternalRef.current; + if (optionsContainer) { + if (isVirtual) { + const virtuosoContainer = optionsContainer.firstElementChild; + const virtuosoScroller = virtuosoContainer?.firstElementChild; + if (virtuosoScroller && virtuosoScroller.clientHeight < optionsContainer.clientHeight) { + setIsOverflowing(false); + } + } else if (optionsContainer.scrollHeight > optionsContainer.clientHeight) { + setIsOverflowing(true); } - } else if (optionsContainer.scrollHeight > optionsContainer.clientHeight) { - setIsOverflowing(true); } - } + }, 0); if (onFocus) { onFocus();