-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore: better select prop types to match antd exports #486
Conversation
Review Forge |
|
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.
This is great, but the name of this PR hides the fact that you've defaulted the suffixIcon
for the Select
to something specific. It's a good thing for consistency, but probably should add a note about that.
…Particle/aquarium into chore/better-select-prop-types
## [1.34.1-chore-better-select-prop-types.2](v1.34.1-chore-better-select-prop-types.1...v1.34.1-chore-better-select-prop-types.2) (2024-11-05)
Oh definitely, thanks for pointing that out. I've been working on this since last week and totally forgot I had done that. |
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-expect-error Introduced when we properly typed the Select value and option generics from Aquarium. Need to double check to fix this. |
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.
@jared-dickman @ZoeAstra would any of you guys be able to take a quick look at this? I didn't want to change anything without the proper context. Ideally the <Select>
used here in Qualifier would be typed and hopefully this problem would be avoided 🙌🏼
🎉 This PR is included in version 1.35.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
This PR makes Aquarium Select component match the way Antd exports its props so we are able to use generics while developing with the Select component.
It also fixes the
suffixIcon
to be thedropdownOpen
icon from Aquarium.Testing Plan
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)