base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[Autocomplete] Refactor useAutocomplete to use Base UI building blocks

Open michaldudak opened this issue 1 year ago • 6 comments

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:

  • mui/material-ui#23708
  • mui/material-ui#23916
  • mui/material-ui#25365
  • mui/material-ui#31192
  • mui/material-ui#31383
  • mui/material-ui#32280
  • mui/material-ui#35533
  • mui/material-ui#35653
  • mui/material-ui#36552
  • mui/material-ui#37738

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 avatar Aug 30 '23 14:08 michaldudak

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.

sai6855 avatar Sep 01 '23 07:09 sai6855

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.

michaldudak avatar Sep 01 '23 07:09 michaldudak

@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.

marcpachecog avatar Oct 23 '23 12:10 marcpachecog

I've opened an issue to organize the scope of Autocomplete for material-next: https://github.com/mui/material-ui/issues/39963

I think at a minimum we can convert to TS

mj12albert avatar Nov 22 '23 13:11 mj12albert

@michaldudak Hey, I would like to work on the useAutocomplete hook and convert it to typescript, if that would be ok?

lhilgert9 avatar Jul 11 '24 15:07 lhilgert9

There's no point in doing this now. We plan to create the new Autocomplete/Combobox component in Base UI and use it in Material UI.

michaldudak avatar Jul 15 '24 09:07 michaldudak