Skip to content
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

The autocomplete component needs to be split into two separate components #637

Open
alexwbbr opened this issue Apr 19, 2024 · 2 comments
Open
Assignees
Milestone

Comments

@alexwbbr
Copy link
Collaborator

alexwbbr commented Apr 19, 2024

The autocomplete component is currently just over 700 lines long and handles multiselect, loading of the options dynamically with the use of onChange and using static options which are then either filtered inside the autocomplete or with an external function using the search parameter. The reason for this refactor would be to reduce the code complexity and hopefully the size of the components so that they are easier to work on in the future.

The two new components would be DynamicAutocomplete and StaticAutoComplete, the main difference between the two components is how they handle filtering and the loading of options.
The dynamic autocomplete would have all filtering external to the component, done by the user, and the user would then update the options property to give the new results. Whereas the static autocomplete options property never changes it's value after it has received the data. The filtering can be done either externally or internally to the component however the options value is never updated by the user.

These two components would still need to handle the current functionality of switching to a form select if the user has turned off javascript and all of the accessibility features that have been added recently. The two components would NOT include the multiselect functionality which would be split off into it's own component in a separate ticket.

Tasks

  • Autocomplete to be marked as deprecated
  • Two new components to be created based off of the existing autocomplete, DynamicAutoComplete and StaticAutocomplete. (StaticAutocomplete can be created first as a base so that DynamicAutocomplete can extend the component)
  • Updated documentation and tests for the components as well as updated examples in storybook.

Please follow these steps to create your branch:

git checkout release/1.1.0
git pull
git checkout -b 'feature/split-autocomplete'
@daniele-zurico daniele-zurico added this to the 1.1.0 milestone May 3, 2024
@alexsteele95 alexsteele95 self-assigned this Jun 21, 2024
@alexsteele95
Copy link
Collaborator

Looking into this

@SidG-CG SidG-CG self-assigned this Jul 15, 2024
@SidG-CG
Copy link
Collaborator

SidG-CG commented Jul 31, 2024

@alexwbbr @Ibabalola
To streamline our discussion in one place, I am adding this information here. Lets continue our discussion in the comments of this ticket so it gets preserved for any future events.
While separating the dynamic and static auto complete functionality, where dynamic used onChange event to provide new options and static allowed an external filter function to be passed in via the 'Search' property, it appears that we can use 'Search' function to provide dynamic filteredList as return value and change the 'Options' at the same time. Furthermore, after the provided 'search' function is called, the code is running exactly the same code as when options are changed while calling 'onChange'. Hence defeating the purpose of having extra code and component for the Dynamic functionality.
Within the PR below, you can find that I have removed the 'onChange' property and I am utilising 'search' property to load options dynamically within the example project.
#672

Hence my suggestion will be that if we only want to change the options using 'onChange' event, we utilise the 'search' property and avoid adding additional component. Once we do see another use case for 'onChange' event we can extend the StaticAutocomplete component and call it something else.

I will also ask if StaticAutocomplete is the correct name for this component now? Would another name be better such as SimpleAutocomplete or something similar to tell it apart from 'Autocomplete' component?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants