-
Notifications
You must be signed in to change notification settings - Fork 71
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
Autocomplete: Handle dynamic options #2723
Autocomplete: Handle dynamic options #2723
Conversation
…-in-autocomplete' into 2710-autocomplete-handle-dynamic-options
✅ Deploy Preview for moduswebcomponents ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
My main feedback here is whether we can handle without adding a new prop, but instead allow |
Thanks @cjwinsor I'm thinking if we can introduce two new props loading and filterOptions that accepts an async function. onValueChange if filterOptions prop is defined, then we will skip the filtering and call the method to get the filtered options from the integrator and display it. This will be useful for both dynamic options and custom filtering in static options if required. The integrator can use the loading prop to display the spinner while they fetch the data from server. The integrator will be responsible to reject promises in case of multiple calls to the filterOptions and ensure they resolve the most recent one. |
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.
Waiting for Sam to try the discussed approach
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.
@ElishaSamPeterPrabhu some suggestions
- The results in the list should be hidden when the loading prop is set to true
- Rename isLoading to loading, and getFilteredOptions to filterOptions
- The same behavior for customOptions is not handled.
- When changing the loading prop to true after resolving the promise, the no results found message is displayed before showing the filtered list.
- The PR also seems to be contain code that needs to be cleaned up
- Update documentation for filterOptions and loading prop
BTW I managed to test the updates in react test application using yalc and it works fine. |
All comments are addressed except custom options implementation and ''m not able to reproduce this For custom options implementation are are we going to get the values and format separately or like in li format like
` |
Description
filterOptions
to accept values dynamicallyReferences
Fixes #2710
Type of change
How Has This Been Tested?
Checklist