From d6fc82b250fcb4bdf5cb5244d00fe168465bac49 Mon Sep 17 00:00:00 2001 From: Masoud Amjadi Date: Wed, 22 Nov 2023 13:11:06 -0500 Subject: [PATCH] refactor(autocomplete): update comments and code cleanup --- .../components/AutocompleteBase/index.tsx | 23 +++++++++++-------- .../components/src/core/Dropdown/index.tsx | 4 +++- .../src/core/DropdownMenu/index.tsx | 14 +++++------ 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/packages/components/src/core/Autocomplete/components/AutocompleteBase/index.tsx b/packages/components/src/core/Autocomplete/components/AutocompleteBase/index.tsx index 279c1a6de..04dfb5c98 100644 --- a/packages/components/src/core/Autocomplete/components/AutocompleteBase/index.tsx +++ b/packages/components/src/core/Autocomplete/components/AutocompleteBase/index.tsx @@ -142,18 +142,21 @@ const AutocompleteBase = < renderInput={defaultRenderInput} multiple={multiple} {...props} - // (masoudmanson): If multiple options are allowed (multiple === true), - // the blurOnSelect behavior should set to false + // (masoudmanson): For multiple options (multiple = true), setting blurOnSelect to false + // keeps the dropdown open after selecting an option. This feature enhances user experience + // by preventing the dropdown from closing, enabling users to select multiple options + // without having to reopen the menu repeatedly. blurOnSelect={multiple ? false : blurOnSelect} - // (masoudmanson): Given the necessity of maintaining consistent onBlur - // functionality, it's crucial to include the onBlur handler after spreading - // other props. Furthermore, within the defaultOnBlur function, the actual - // onBlur function is invoked for completeness. + // (masoudmanson): Given the necessity of maintaining consistent onBlur functionality, + // and clearing the input value on blur, it's crucial to include the onBlur handler + // after spreading other props. Furthermore, within the defaultOnBlur function, the + // actual onBlur function is invoked for completeness. onBlur={defaultOnBlur} - // (masoudmanson): Considering the necessity of executing our function upon input - // changes universally, it's essential to place the onInputChange handler after - // spreading the remaining props. Additionally, within the defaultOnInputChange - // function, the actual onInputChange function is invoked at its conclusion. + // (masoudmanson): To ensure component-level actions (changing the input field value) + // trigger upon input changes, it's crucial to position the onInputChange handler after + // spreading the other props. Furthermore, within the defaultOnInputChange function, + // the provided onInputChange from props is called at the end to emit the input changes + // to the user. onInputChange={defaultOnInputChange} /> ); diff --git a/packages/components/src/core/Dropdown/index.tsx b/packages/components/src/core/Dropdown/index.tsx index d30849601..9c7387c62 100644 --- a/packages/components/src/core/Dropdown/index.tsx +++ b/packages/components/src/core/Dropdown/index.tsx @@ -6,6 +6,7 @@ import { AutocompleteValue, } from "@mui/material/useAutocomplete"; import React, { ReactNode, useEffect, useState } from "react"; +import { EMPTY_OBJECT } from "src/common/utils"; import { AutocompleteMultiColumnOption, AutocompleteSingleColumnOption, @@ -110,7 +111,8 @@ const Dropdown = < ); } - const isMultiColumn = options && !!options[0] && "options" in options[0]; + const isMultiColumn = "options" in (options?.[0] || EMPTY_OBJECT); + const isControlled = propValue !== undefined; const [anchorEl, setAnchorEl] = useState(null); const [open, setOpen] = useState(false); diff --git a/packages/components/src/core/DropdownMenu/index.tsx b/packages/components/src/core/DropdownMenu/index.tsx index a183247a1..ad609c651 100644 --- a/packages/components/src/core/DropdownMenu/index.tsx +++ b/packages/components/src/core/DropdownMenu/index.tsx @@ -8,7 +8,7 @@ import { } from "@mui/material"; import { useTheme } from "@mui/material/styles"; import React, { SyntheticEvent, useMemo } from "react"; -import { noop } from "src/common/utils"; +import { EMPTY_OBJECT, noop } from "src/common/utils"; import Autocomplete, { AutocompleteProps } from "../Autocomplete"; import { DefaultAutocompleteOption } from "../Autocomplete/components/AutocompleteBase"; import { InputSearchProps } from "../InputSearch"; @@ -73,6 +73,10 @@ export type DropdownMenuProps< > = CustomAutocompleteProps & ExtraDropdownMenuProps; +const DEFAULT_POPPER_BASE_PROPS: Partial = { + disablePortal: true, +}; + const DropdownMenu = < T extends DefaultAutocompleteOption, Multiple extends boolean | undefined, @@ -103,7 +107,7 @@ const DropdownMenu = < ...rest } = props; - const isMultiColumn = options && !!options[0] && "options" in options[0]; + const isMultiColumn = "options" in (options?.[0] || EMPTY_OBJECT); // (masoudmanson): The DropdownMenu's Popper component should have a minimum // width of MINIMUM_DROPDOWN_MENU_POPPER_WIDTH pixels if the DropdownMenu is @@ -119,10 +123,6 @@ const DropdownMenu = < }; }, [PopperBaseProps?.sx, isMultiColumn, width]); - const DefaultPopperBaseProps = { - disablePortal: true, - }; - const DefaultInputBaseProps = useMemo(() => { return { ...InputBaseProps, @@ -166,7 +166,7 @@ const DropdownMenu = < title={title} open={open} options={options} - PopperBaseProps={DefaultPopperBaseProps} + PopperBaseProps={DEFAULT_POPPER_BASE_PROPS} disablePortal onClickAway={noop} {...rest}