-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(multicolumnautocomplete): first implementation #670
Conversation
Hey Masoud 😄 ! Just curious, not sure if you got a chance to explore the possibility of keeping just one Autocomplete component that's capable of handling single and multi columns? Thank you! |
… ComplexFilter components
Hey @tihuan, That's exactly what I've done in this PR, we now have an Autocomplete component that renders a SingleColumn or MultiColumn Autocomplete based on the options prop given to it! if the options prop is a simple array, the result would be a SingleColumn Autocomplete: const OPTIONS = [
{
name: "Status: can't reproduce",
section: "name only",
},
{
name: "Status: confirmed",
section: "name only",
},
{
count: 3,
name: "Status: duplicate",
section: "name with count",
}
]; And if the options is an array with the following structure, the result would be a MultiColumn dropdown: const OPTIONS = [
{
icon: <Icon sdsIcon="chevronRight" sdsSize="xs" sdsType="static" />,
name: "columnOne",
width: 250,
options: [
{
description: "Good for newcomers",
name: "good first issue",
},
{
description: "Extra attention is needed",
name: "help wanted",
},
],
},
{
icon: <Icon sdsIcon="chevronRight" sdsSize="xs" sdsType="static" />,
name: "columnTwo",
width: 200,
options: [
{
description: "needs to be fixed",
name: "bug",
},
{
description: "needs to be implemented",
name: "feature",
},
],
},
]; To check both instances, change the options prop for the Autocomplete component in Storybook. |
Ah cool! That's amazing ⭐ Thanks for doing that! So does that mean Because if that's the case, I wonder if we could just do a breaking change and replace |
Thank you, 😍 Nothing needs to be replaced because the old So, the product teams should not notice any difference if they continue to pass the old options array! Actually, I think we should skip exporting the |
Ooh that's perfect! Sorry I think I missed the important fact that Autocomplete uses AutocompleteMultiColumns under the hood and was thinking that AutocompleteMultiColumns was a replacement of Autocomplete lol Removing AutocompleteMultiColumns and AutocompleteBase from export sounds great to me, since they are implementation details 🙆♂️ Thanks so much for the amazing work, Masoud! |
A followup question, since we're removing exports of I'll do the review after the changes above are done, if that's okay! Thanks again! |
Thank you for your feedback and guidance @tihuan |
Always a pleasure, Masoud! Really appreciate your fantastic work as always 🎆 🥇 !! |
…Column exports and stories and add both under the broader Autocomplete component
…efaultAutocompleteOption type
…ultiColumn into the Autocomplete/components directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good @masoudmanson ! A few changes still to make
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @masoudmanson!
The only change I'd request is adding a Boolean
property for the right chevron between the columns. That implies a relationship between the columns which isn't always wanted.
The options array allows you to change the icons, names, and widths for each column. I've added the option to define an icon for each column so users can remove it by not adding a 'SdsIcon' prop in the column data.
OPTIONS = [
{
name: "Column 1",
width: 260,
options: [
...
],
},
{
name: "Column 2",
width: 260,
options: [
...
],
},
{
name: "Column 3",
width: 170,
options: [
...
],
},
];
OPTIONS = [
{
icon: <Icon sdsIcon="chevronRight" sdsSize="xs" sdsType="static" />,
name: "Column 1",
width: 260,
options: [
...
],
},
{
icon: <Icon sdsIcon="chevronRight2" sdsSize="xs" sdsType="static" />,
name: "Column 2",
width: 260,
options: [
...
],
},
{
name: "Column 3",
width: 170,
options: [
...
],
},
]; |
@clarsen-czi. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few more changes, but almost there. Thanks @masoudmanson !!
…completeMultiColumn DropdownMenu and Dropdown components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Monumental work here, @masoudmanson 😮 🥇 🏆 👏 👏 👏
Really a true testament of your mad skillz and perseverance to get through this!
Thanks so much again for this impressive work 💎 !!
Just some small nits here otherwise LGTM 🚀 !
@@ -136,7 +136,7 @@ const InputSearch = forwardRef<HTMLDivElement, InputSearchProps>( | |||
* (masoudmanson): This prevents the browser's default auto completion | |||
* menu from being displayed for the InputSearch. | |||
*/ | |||
autoComplete="off" | |||
autoComplete="one-time-code" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's weird, but autoComplete="off" seems ineffective in some cases now. Browsers are introducing new potential values for the autocomplete
attribute!
https://stackoverflow.com/questions/12374442/chrome-ignores-autocomplete-off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahaha the saga continues!
I remember reading the same thread and used their 2021 recommended solution of combining type="search"
and autoComplete"off"
here
But I guess that doesn't work anymore 😮💨
Huge thanks for diving into the PR with such attention to detail, @tihuan! 😍 You're awesome! 🥳🙏🏻 |
Styles have been updated per our discussion!
BREAKING CHANGE: Autocomplte options type has changed from DefaultDropdownMenuOption to DefaultAutocompleteOption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My pleasure to review your fantastic PR as always, @masoudmanson 😄 🙏 !!
Just some tiny comments left!
And one last thing is perhaps adding a breaking change commit, so we get the proper version bump and change log 👌
Thanks again for all the amazing work on this!
packages/components/src/core/Autocomplete/components/AutocompleteBase/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/core/Autocomplete/components/AutocompleteGroup/index.tsx
Show resolved
Hide resolved
… into 559-multi-column-dropdown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to the max!! 💯 🔥
Thanks again for everything, Masoud!!
Woot woot! 🎉🔥 |
You did all the important work, my man! Thanks again 🤩 🥇 !! |
Summary
Autocomplete, DropdownMenu, and Dropdown
Github issue: #559
Enhancing the
Autocomplete
,DropdownMenu
, andDropdown
components to support multiple columns. This update enables users to create multi-column dropdowns by introducing a new type of options array to the autocomplete component.Single-Column
The
Autocomplete
component will continue rendering a single-column autocomplete by passing a list of options as before. No alterations have been made to the previous usages of the Autocomplete or Dropdown components.Multi-Column
The provided structure for the new options will enable the rendering of a multi-column
Autocomplete
/Dropdown
component. Each column within this structure includes an options array identical to the single-column Autocomplete's options array.The AutocompleteMultiColumnOption type is defined as below:
See the example below:
Implementation Details
Two new components have been integrated into SDS to introduce the Single-Column/Multi-column functionality using MUI's Autocomplete. These components are the
AutocompleteBase
and theAutocompleteMultiColumn
. The SDS'sAutocomplete
component determines which one to render based on the structure of the options array.AutocompleteBase
AutocompleteBase
is a simple wrapper around MUI's Autocomplete component, enhancing it with specific styles and additional functionalities.AutocompleteMultiColumn
AutocompleteMultiColumn
is responsible for rendering Multi-column Autocompletes. It iterates through the provided options array, creating distinct MUI Autocomplete components for each column. These individual Autocomplete components are then encapsulated within aPopper
component for display.Controllable Autocomplete
To transform the SDS
Autocomplete
into a controlled component, supply thevalue
andonChange
props to the component. A basic implementation for a controlled Autocomplete can be exemplified as follows:Breaking Changes
1. Autocomplete option type
We've replaced DefaultDropdownMenuOption with DefaultAutocompleteOption as the preferred choice. However, we've exported this line for backward compatibility to prevent potential TypeScript type issues for product teams using previous versions.
2. OnChange Function Parameters
The onChange function within the Autocomplete component now allows the use of four parameters, as detailed below:
3. Autocomplete Value Structure
The Autocomplete/Dropdown value structure varies between Single and Multi-column Autocompletes.
Single-column Autocomplete:
For
multiple = false
:For
multiple = true
:Multi-column Autocomplete:
For
multiple = false
:For
multiple = true
:4. Adjusting Dropdown Menu Width
Customizing the width of the Dropdown component's menu is straightforward. You can set the desired width by passing a width prop to the underlying DropdownMenu component.
Checklist
defaultTheme.ts
used wherever possible