-
Notifications
You must be signed in to change notification settings - Fork 39
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
Typecasting forwardRef
inside Select
#1774
Conversation
forwardRef
inside Select
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.
No additional comments. Just found this StackOverflow answer that might be related.
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.
can we also make ComboBox
behave similarly?
Is it okay to have a new PR for it or should I have it included in this one? |
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.
can we also make
ComboBox
behave similarly?Is it okay to have a new PR for it or should I have it included in this one?
upto you, i'm fine with either
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, just missing changeset
packages/itwinui-react/src/core/LabeledSelect/LabeledSelect.types-test.tsx
Show resolved
Hide resolved
Is the changeset okay? (9c481a7). |
packages/itwinui-react/src/core/LabeledSelect/LabeledSelect.types-test.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Mayank <[email protected]>
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.
Action items after this PR merge:
can we also make
ComboBox
behave similarly?
migrate all unit tests and types tests to
vitest
Changes
Fixes: #1767
Added type for forwardRef inside
Select
andLabeledSelect
component (Refer to option 1 inside this link).Added Type checking test case for both components.
Added unit test that forwards the ref Inside the components.
For reviewing, turn off whitespace diff.
Testing
Tested locally in the Vite Playground. No longer getting the error mentioned in #1767.
Docs
Added changeset.