-
Notifications
You must be signed in to change notification settings - Fork 71
Fixes TextInput component types #3197
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
Conversation
🦋 Changeset detectedLatest commit: 1ee7daa The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Pull Request Overview
This PR fixes the TypeScript types for the TextInput component to properly support React refs. The primary change updates the component's type definition from ComponentPropsWithoutRef
to ComponentPropsWithRef
, enabling proper ref forwarding to the underlying HTML input element.
Key changes:
- Updated TextInput component types to accept refs properly
- Added tests to verify ref functionality for both TextInput and TextArea components
- Updated package versions and added changeset documentation
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tools/install/src/ALL_PACKAGES.ts |
Added new packages to the installation list |
packages/text-input/src/TextInput/TextInput.types.ts |
Changed from ComponentPropsWithoutRef to ComponentPropsWithRef for proper ref support |
packages/text-input/src/TextInput/TextInput.spec.tsx |
Added test for ref functionality and reorganized existing type tests |
packages/text-area/src/TextArea/TextArea.spec.tsx |
Added test for TextArea ref functionality |
packages/emotion/src/version.ts |
Version bump from 5.0.2 to 5.0.3 |
.changeset/eleven-cars-obey.md |
Added changeset documentation for the TextInput fix |
Size Change: 0 B Total Size: 1.59 MB ℹ️ View Unchanged
|
tools/install/src/ALL_PACKAGES.ts
Outdated
'@leafygreen-ui/avatar', | ||
'@leafygreen-ui/badge', | ||
'@leafygreen-ui/banner', | ||
'@leafygreen-ui/box', |
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.
need to rm the /box
dir from your file system in order to prevent this
/* eslint-disable jest/no-disabled-tests */ | ||
describe.skip('types behave as expected', () => { | ||
test('TextArea throws error when neither aria-labelledby or label is supplied', () => { | ||
describe('types behave as expected', () => { |
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.
Curious why the skip
was moved here and removed entirely from packages/text-input/src/TextInput/TextInput.spec.tsx
. Seems inconsistent and perhaps a mistake?
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.
Technically should still be skipped, since there's no expect
statement
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.
The actual fix seems fine to me 👍
Although some of the changes seems unrelated to the fix 🤔
(although I did see opportunities for drive-by, non-breaking alignment across the packages, which was not picked up from #3167 🙈)
✍️ Proposed changes
🎟 Jira ticket: Name of ticket
✅ Checklist
For new components
For bug fixes, new features & breaking changes
pnpm changeset
and documented my changes🧪 How to test changes