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] Add new AutocompleteInputChangeReason #37301

Merged
merged 7 commits into from
Jul 11, 2024

Conversation

binh1298
Copy link
Contributor

@binh1298 binh1298 commented May 17, 2023

fixes: #36085
duplicate of #37013

@mui-bot
Copy link

mui-bot commented May 17, 2023

@zannager zannager added the component: autocomplete This is the name of the generic UI component, not the React module! label May 17, 2023
@zannager zannager requested a review from michaldudak May 17, 2023 13:09
@mj12albert mj12albert changed the title autocomplete add new AutocompleteInputChangeReason [Autocomplete] add new AutocompleteInputChangeReason May 19, 2023
@michaldudak michaldudak added the on hold There is a blocker, we need to wait label Jun 14, 2023
@michaldudak michaldudak added this to the v6 milestone Jun 14, 2023
@michaldudak michaldudak changed the title [Autocomplete] add new AutocompleteInputChangeReason [Autocomplete] Add new AutocompleteInputChangeReason Jun 14, 2023
@mnajdova mnajdova requested a review from mj12albert June 21, 2023 13:35
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 20, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 27, 2023
@DiegoAndai
Copy link
Member

Hi! isn't this PR missing things from the original one (#37013)? for example this change.

@DiegoAndai DiegoAndai assigned DiegoAndai and unassigned DiegoAndai Feb 19, 2024
@aarongarciah aarongarciah changed the base branch from master to next June 17, 2024 11:37
@aarongarciah aarongarciah force-pushed the autocomplete-resetInputValue-reason-2 branch from 9bbfb41 to 82e36d9 Compare June 17, 2024 11:44
@aarongarciah
Copy link
Member

I added the missing changes that were present in #37013

@aarongarciah aarongarciah added package: material-ui Specific to @mui/material package: base-ui Specific to @mui/base package: joy-ui Specific to @mui/joy port-to-base-ui PR to be included in the Base UI repository once the API changes are done and removed on hold There is a blocker, we need to wait labels Jun 24, 2024
@aarongarciah
Copy link
Member

@michaldudak @DiegoAndai this is ready for review.

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

@aarongarciah we should add the following

  • A section in the v6 migration guide explaining the change and how to adapt
  • Tests for the introduced reasons

Do you agree? Are we missing anything else?

@aarongarciah
Copy link
Member

@DiegoAndai that should cover everything. I'll add it.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 4, 2024
@aarongarciah aarongarciah force-pushed the autocomplete-resetInputValue-reason-2 branch from b5b2ba8 to 41351fa Compare July 4, 2024 16:50
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 4, 2024
@aarongarciah aarongarciah force-pushed the autocomplete-resetInputValue-reason-2 branch from 41351fa to c33f43f Compare July 4, 2024 16:52
Comment on lines 128 to 130
- `"blur"`: like `"reset"` but triggered when the focus is moved off the input. `clearOnBlur` must be `true`. Before, it was treated as `"reset"`.
- `"selectOption"`: triggered when the input value changes after an option has been selected. Before, it was treated as `"reset"`.
- `"removeOption"`: triggered in multiple selection when a chip gets removed due to the corresponding option being selected. Before, it was treated as `"reset"`.
Copy link
Member

Choose a reason for hiding this comment

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

@michaldudak let me know if these descriptions are accurate.

@aarongarciah
Copy link
Member

@DiegoAndai just pushed the tests covering the missing onInputChange reasons and an entry in the v6 migration guide.

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Nice @aarongarciah! One final question. Do you think we should take this opportunity to use user event instead of fireEvent on these tests? If you prefer doing it on a separate PR, that's ok. Let me know.

@aarongarciah aarongarciah force-pushed the autocomplete-resetInputValue-reason-2 branch from 9a5b265 to 5adc07f Compare July 11, 2024 10:55
@aarongarciah
Copy link
Member

@DiegoAndai all tests updated to use the new user object returned from render. I rebased to get it from next.

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Great addition ⭐
Tests getting refactored ✨
Amazing job 💯

@aarongarciah aarongarciah merged commit f899014 into mui:next Jul 11, 2024
22 checks passed
joserodolfofreitas pushed a commit to joserodolfofreitas/material-ui that referenced this pull request Jul 29, 2024
Co-authored-by: Aarón García Hervás <aaron@mui.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: autocomplete This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base package: joy-ui Specific to @mui/joy package: material-ui Specific to @mui/material port-to-base-ui PR to be included in the Base UI repository once the API changes are done v6.x
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Autocomplete] Add an option to not resetInputValue on selectNewValue
7 participants