Skip to content

Commit

Permalink
fix(dropdown.tsx): 309 - changed useEffect to use Json.stringify (opt…
Browse files Browse the repository at this point in the history
…ions)/(filteredOptions)

Also added a test to Dropdown.test.tsx with options set to undefined to test infinite loop
  • Loading branch information
jonesdebu committed Dec 21, 2021
1 parent 2904775 commit 05e4484
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 16 deletions.
24 changes: 9 additions & 15 deletions src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,6 @@ export interface DropdownProps {

const defaultCallback = () => {}; // eslint-disable-line @typescript-eslint/no-empty-function

const pokeOptions = [
{ id: 'bulbasaur', optionValue: 'Bulbasaur' },
{ id: 'charmander', optionValue: 'Charmander' },
{ id: 'squirtle', optionValue: 'Squirtle' },
];

const Dropdown = ({
StyledContainer = Container,
StyledValueContainer = ValueContainer,
Expand Down Expand Up @@ -341,14 +335,13 @@ const Dropdown = ({
onFocus,
onClear,
onSelect,
options = pokeOptions,
options = [],
tabIndex = 0,
variant = variants.outline,
optionsVariant = variants.outline,
rememberScrollPosition = true,
valueVariant = variants.text,
values = [],

shouldStayInView = true,
intersectionThreshold = 1.0,
intersectionContainer = null,
Expand Down Expand Up @@ -401,14 +394,13 @@ const Dropdown = ({
const searchInputRef = useRef<HTMLInputElement>(null);
const [searchCharacterCount, setSearchCharacterCount] = useState<number>(0);
const [filteredOptions, setFilteredOptions] = useState<OptionProps[]>([]);

if (typeof options === 'undefined') {
options = pokeOptions;
}
const stringifyOptions = JSON.stringify(options);

useEffect(() => {
setFilteredOptions(options);
}, [options]);
// empty array of options makes this useEffect loop
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [stringifyOptions]);

// Merge the default styled container prop and the placeholderProps object to get user styles
const placeholderMergedProps = {
Expand Down Expand Up @@ -511,6 +503,7 @@ const Dropdown = ({
[intersectOptions, intersectionCallback],
);

const stringifyFilteredOptions = JSON.stringify(filteredOptions);
useEffect(() => {
if (isOpen) {
// setTimeout ensures this code renders after the initial render
Expand Down Expand Up @@ -548,7 +541,7 @@ const Dropdown = ({
intersectObserver.disconnect();
};
}
}, [intersectObserver, isOpen, isVirtual, filteredOptions]);
}, [intersectObserver, isOpen, isVirtual, stringifyFilteredOptions]);

const optionsHash: { [key: string]: OptionProps } = useMemo(() => {
const hash: { [key: string]: OptionProps } = {};
Expand All @@ -568,7 +561,7 @@ const Dropdown = ({
const handleBlur = useCallback(
(e: React.FocusEvent) => {
e.preventDefault();

e.persist();
setFocusTimeoutId(
window.setTimeout(() => {
if (focusWithin) {
Expand All @@ -586,6 +579,7 @@ const Dropdown = ({

const handleFocus = useCallback(
(e: any) => {
e.persist();
window.setTimeout(() => {
if (document.activeElement?.id === `${name}-dropdown-button`) {
searchInputRef?.current?.focus();
Expand Down
2 changes: 1 addition & 1 deletion src/components/Dropdown/__tests__/Dropdown.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ describe('Dropdown', () => {
it('Should pass accessibility test with default props', async () => {
generateIntersectionObserver([]);
const component = (
<Dropdown onSelect={() => {}} placeholder="hello" options={undefined}></Dropdown>
<Dropdown onSelect={() => {}} placeholder="hello" options={undefined}></Dropdown>
);
const { container } = render(component);
const results = await axe(container);
Expand Down

0 comments on commit 05e4484

Please sign in to comment.