-
Notifications
You must be signed in to change notification settings - Fork 936
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: UseComboboxGetComboboxPropsOptions typing #1193
fix: UseComboboxGetComboboxPropsOptions typing #1193
Conversation
Rats! You're right, thanks you very much! I see the build system is not feeling well. I probably need to update the dependencies and fix this error before merging anything. Thank you again for the contribution! |
@clementgarbay can you rebase this PR? It should pass now. Thank you very much! One question: this fix can't be considered a breaking change right? I don't think it's very probable for someone to use label related field without them working. |
1f326e2
to
2f37832
Compare
Codecov Report
@@ Coverage Diff @@
## master #1193 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1705 1705
Branches 493 493
=========================================
Hits 1705 1705 Continue to review full report at Codecov.
|
@silviuaavram I just rebased and the build seems now fine! I don't think this fix should be considered as a breaking change |
@all-contributors please add @clementgarbay for code |
I've put up a pull request to add @clementgarbay! 🎉 |
🎉 This PR is included in version 6.0.13 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What:
I fixed the wrong typing of the
UseComboboxGetComboboxPropsOptions
interface. The generic type of theHTMLProps
extend wasHTMLLabelElement
instead ofHTMLDivElement
.Why:
From the related documentation (https://www.downshift-js.com/use-combobox), the DOM element where the
getComboboxProps
should be spread isdiv
.Checklist: