-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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] Fix unexpected clearing #19511
Conversation
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.
Thanks for the fix!
Could you add a test to Autocomplete.test.js
that would fail without this fix?
Details of bundle changes.Comparing: 62e439b...0091444
|
@eps1lon sure. |
1093154
to
e530032
Compare
@oliviertassinari @eps1lon It is hard to figure out the logic (predict all usecases). I've spent several hours and I feel like: Could you please also explain me what does mean next error:
|
@SerhiiBilyk You can assume the current behavior on master is correct.
Don't give up :). Being a hard task is a good signal! The harder the problem, the more you will learn and the more our users will value our solutions.
This sounds like an installation issue on your environment. |
a041157
to
5c84559
Compare
@oliviertassinari @eps1lon Could you please check ? |
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.
- I have updated the test case to be failing without the patch. Otherwise, it won't prevent future regressions. This is especially important as a case I have missed during the initial implementation.
- They are two different problems solved at once here. I have added a new test case.
@oliviertassinari Good point! |
@SerhiiBilyk Thank you! |
Closes #18646