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

[docs][Base] useAutocomplete demos & docs #37029

Merged
merged 12 commits into from
Jun 5, 2023

Conversation

mj12albert
Copy link
Member

@mj12albert mj12albert commented Apr 26, 2023

Resolves #36884
Resolves #37252

Preview: https://deploy-preview-37029--material-ui.netlify.app/base/react-autocomplete/

Adding some demos to show how to use useAutocomplete. There is no Autocomplete component yet, just the hook.

Demos

In this PR I'm aiming to add the minimal amount of topics to cover the most basic use-cases of useAutocomplete, and will exclude big topics such as freeSolo or multi.

  • Introduction
  • Minimal example (modify Material UI one)
  • How to render and customize options
  • States (value vs inputValue)
  • With/without Popper/Portal

@mui-bot
Copy link

mui-bot commented Apr 26, 2023

Netlify deploy preview

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against ecada5c

@mj12albert mj12albert added docs Improvements or additions to the documentation package: base-ui Specific to @mui/base labels Apr 26, 2023
@mj12albert mj12albert force-pushed the base/autocomplete-docs branch 9 times, most recently from 225c441 to 28c1349 Compare April 27, 2023 16:41
@mj12albert mj12albert changed the title [docs][Base] Autocomplete demos & docs [docs][Base] useAutocomplete demos & docs Apr 27, 2023
@mj12albert mj12albert force-pushed the base/autocomplete-docs branch 3 times, most recently from c3d04ea to eda6e76 Compare May 10, 2023 17:05
@mj12albert mj12albert force-pushed the base/autocomplete-docs branch 2 times, most recently from 9a24231 to d318db4 Compare May 11, 2023 08:58
@mj12albert mj12albert marked this pull request as ready for review May 11, 2023 08:58
@zannager zannager requested a review from michaldudak May 11, 2023 09:18

{{"demo": "UseAutocompletePopper.js", "hideToolbar": true}}

{{"demo": ""}}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a weird workaround for this:

.slice(i, rendered.length - 1)`

which I suppose is to hide the old API docs links section, but here it's just cutting off my content as there's no component: in the markdown front matter 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the listbox is always opening on top.

Screenshot (16)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actualy the reason why it is cut-off is because the components header is empty, so the API section is not added. Let's instead change the logic in MarkdownDocsV2.js to do the slice only if the headers.components.length > 0. Please validate if I am correct in this assumption :)

@mj12albert mj12albert requested a review from a team May 11, 2023 09:30
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up. Few thoughts:

  • I believe we should make all demos customized, similar to how the first one is done

Not related to this PR:

  • the groupedOptions should probably be just called options. when there is no need for groups the name is misleading

const options = ['The Godfather', 'Pulp Fiction'];
```

If you need to use a different struction for options, you must provide a function to the `getOptionLabel` prop that resolves each option to a unique value.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified this bit compared to the other docs (Material, Joy) because it kind of understates the importance of getOptionLabel.

The API docs just say “Used to determine the string value for a given option”, but it seems the hook heavily relies on getOptionLabel to tell different options from each other

I noticed issues kind of related to this:

Comment on lines 23 to 31
:::warning
Material UI and Joy UI have Autocomplete components that are built using the useAutocomplete hook, with many features that are not demo-ed here yet.

To learn more about implementing a custom Autocomplete, you can explore the useAutocomplete API [docs](/base/react-autocomplete/hooks-api/), or reference the Material UI and Joy UI implementations:

- [Material UI Autocomplete](/material-ui/react-autocomplete/)
- [Joy UI Autocomplete](/joy-ui/react-autocomplete/)

:::
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samuelsycamore I think the message in this callout is a little clumsy, any suggestions? 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me! Here's how I'd phrase that first part.

Suggested change
:::warning
Material UI and Joy UI have Autocomplete components that are built using the useAutocomplete hook, with many features that are not demo-ed here yet.
To learn more about implementing a custom Autocomplete, you can explore the useAutocomplete API [docs](/base/react-autocomplete/hooks-api/), or reference the Material UI and Joy UI implementations:
- [Material UI Autocomplete](/material-ui/react-autocomplete/)
- [Joy UI Autocomplete](/joy-ui/react-autocomplete/)
:::
:::warning
Material UI and Joy UI have Autocomplete components that are built using the `useAutocomplete` hook, and they include many features not yet described here.
To learn more about implementing a custom Autocomplete, you can explore the [`useAutocomplete` API docs](/base/react-autocomplete/hooks-api/), or reference the Material UI and Joy UI implementations:
- [Material UI Autocomplete](/material-ui/react-autocomplete/)
- [Joy UI Autocomplete](/joy-ui/react-autocomplete/)
:::

@mj12albert mj12albert requested review from mnajdova and a team May 12, 2023 10:21
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one comment left, the rest looks great


{{"demo": "UseAutocompletePopper.js", "hideToolbar": true}}

{{"demo": ""}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actualy the reason why it is cut-off is because the components header is empty, so the API section is not added. Let's instead change the logic in MarkdownDocsV2.js to do the slice only if the headers.components.length > 0. Please validate if I am correct in this assumption :)

@mj12albert mj12albert force-pushed the base/autocomplete-docs branch 2 times, most recently from 33c3e30 to 0228d28 Compare June 5, 2023 07:39
@mj12albert
Copy link
Member Author

instead change the logic in MarkdownDocsV2.js to do the slice only if the headers.components.length > 0

This worked and I added a comment! @mnajdova

@mj12albert mj12albert requested a review from mnajdova June 5, 2023 07:59
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just left two comments. Looks good. Let's ship it!

@mj12albert mj12albert force-pushed the base/autocomplete-docs branch 3 times, most recently from d6f90f1 to 55576c1 Compare June 5, 2023 09:40
@mj12albert mj12albert merged commit 8bf37de into mui:master Jun 5, 2023
21 checks passed
@mj12albert mj12albert mentioned this pull request Jun 5, 2023
DiegoAndai added a commit to DiegoAndai/material-ui that referenced this pull request Jun 5, 2023
DiegoAndai added a commit to DiegoAndai/material-ui that referenced this pull request Jun 5, 2023
@mj12albert mj12albert deleted the base/autocomplete-docs branch July 1, 2023 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: base-ui Specific to @mui/base
Projects
None yet
5 participants