-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
fix(17119): combobox not displaying selected option once search query is cleared #18498
fix(17119): combobox not displaying selected option once search query is cleared #18498
Conversation
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18498 +/- ##
==========================================
- Coverage 84.04% 84.04% -0.01%
==========================================
Files 408 408
Lines 14470 14481 +11
Branches 4660 4717 +57
==========================================
+ Hits 12162 12171 +9
- Misses 2142 2145 +3
+ Partials 166 165 -1 ☔ View full report in Codecov by Sentry. |
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.
Looking good so far! Thanks for opening that! 🚀
A couple things. Looks like the Combobox is not opening when pressing Enter
anymore. Combobox should open when pressing Enter
. The test should remaing the same in this case.
@@ -764,5 +764,17 @@ describe('ComboBox', () => { | |||
|
|||
expect(input).toHaveDisplayValue('Apple'); | |||
}); | |||
|
|||
it('should remove input and enter new conditions', async () => { |
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'm not sure about this test. Is it testing the functionality you created?
Otherwise we could add a test like that, to test step by step the interaction with the Combobox.
You will noticed that if you remove the changes you made in the Combox.tsx
this test will fail (as expected), however it will pass with the current changes (as expected)
it('should open the menu and select null when Enter is pressed with no input and no highlighted item', async () => {
const onInputChange = jest.fn();
render(<ComboBox {...mockProps} onInputChange={onInputChange} />);
await userEvent.type(findInputNode(), 'apple');
expect(findInputNode()).toHaveDisplayValue('apple');
await userEvent.keyboard('[Enter]');
// Delete the selected item
await userEvent.keyboard('[Backspace]');
await userEvent.keyboard('[Backspace]');
await userEvent.keyboard('[Backspace]');
await userEvent.keyboard('[Backspace]');
await userEvent.keyboard('[Backspace]');
// check for an empty value
expect(findInputNode()).toHaveDisplayValue('');
// blur
await userEvent.keyboard('[Tab]');
assertMenuClosed(mockProps);
// open the menu
await userEvent.click(findInputNode());
assertMenuOpen(mockProps);
// check if the `li` item are all false
for (let i = 0; i < mockProps.items.length; i++) {
const item = findMenuItemNode(i);
console.log({ item });
expect(item).toHaveAttribute('aria-selected', 'false');
}
});
```
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.
Yes, this is the test I've created for this addition! I will add this extra evaluation then, thanks!
Thanks for your submission! We ask that you all sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. 2 out of 3 committers have signed the DCO. |
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.
LGTM, thanks!
2018d25
Closes #17119
Changelog
New
Changed
Removed
Testing / Reviewing
Using the Carbon storybook for react components, reviewers can test as shown in the original issue reported.