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

[Autocomplete] Refactor useAutocomplete to use Base UI building blocks #45

Open
6 tasks
michaldudak opened this issue Aug 30, 2023 · 4 comments
Open
6 tasks
Assignees
Labels
component: autocomplete This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature

Comments

@michaldudak
Copy link
Member

michaldudak commented Aug 30, 2023

The useAutocomplete hook was created before Base UI existed and has code patterns different from the rest of the hooks in the library. Additionally, it does not use the lower-level building blocks from Base UI (like useList) but implements its whole functionality by itself. Reusing the same building blocks as other components could help us reduce the overall bundle size and have feature parity with the Select (where applicable).
There are a number of issues reported that could be solved by replacing the implementation.

Areas of improvement:

  • Use useList instead of reimplementing keyboard navigation
  • Convert the code and tests to TypeScript
  • Do not set any classes (it's a responsibility of a component, not a hook)
  • Accept external props in get*Props functions
  • Implement a reducer to handle state and expose its dispatch function to allow programmatic control
  • Revise prop naming (like freeSolo)

Issues that could be solved along the way:

Since this will likely be a breaking change, we should leave the current version intact (but deprecate it and remove it in the future).

@michaldudak michaldudak self-assigned this Aug 30, 2023
@sai6855
Copy link

sai6855 commented Sep 1, 2023

Since useList is going to use in Select, Menu, Autocomplete (In future). does it makes sense to add Infinite scroll feature in useList itself (as it's one of most requested feature)? Once it is implemented in useList, Autocomplete just need to pass necessary props to make it working.

@michaldudak
Copy link
Member Author

I don't think components other than Autocomplete would make use of infinite scroll, so we need to be careful not to include features that won't be used but will increase the component size. I think we can add an event to the List fired when scroll is near the bottom.

@marcpachecog
Copy link
Contributor

@michaldudak Infinite scrolling inside a list is in fact a very common feature rather than not, I have it implemented on my own by using the Intersection Observer. So it make sense to also add it to the List as you mentioned.

@mj12albert
Copy link
Member

I've opened an issue to organize the scope of Autocomplete for material-next: mui/material-ui#39963

I think at a minimum we can convert to TS

@michaldudak michaldudak transferred this issue from mui/material-ui Feb 27, 2024
@michaldudak michaldudak changed the title [Autocomplete][base-ui] Refactor useAutocomplete to use Base UI building blocks [Autocomplete] Refactor useAutocomplete to use Base UI building blocks Feb 27, 2024
@michaldudak michaldudak added component: autocomplete This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature
Projects
Status: Backlog
Development

No branches or pull requests

4 participants