-
Notifications
You must be signed in to change notification settings - Fork 71
LG-5656: Remove support for refEl property #3270
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: c0c7637 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 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 removes support for the refEl property from the Select component to enforce proper reference handling by always using the internal MenuButton instance. This is a breaking change that simplifies the API by removing a potentially confusing prop.
Key changes:
- Excludes
refElfrom inherited PopoverProps in Select component types - Updates import statements to use named imports instead of default imports
- Documents the breaking change in a changeset
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/select/src/Select/Select.types.ts | Adds 'refEl' to the omitted PopoverProps to prevent external ref override |
| packages/select/src/MenuButton/MenuButton.tsx | Updates Button import from default to named import |
| packages/select/src/ListMenu/ListMenu.tsx | Updates Popover import from default to named imports |
| .changeset/clean-news-lay.md | Documents the breaking change for release notes |
|
Size Change: -4 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@leafygreen-ui/select': major | |||
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.
dang I wish this wasn't a major but I guess it has to be 🥲
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.
updated the language RFAL?
.changeset/clean-news-lay.md
Outdated
| '@leafygreen-ui/select': major | ||
| --- | ||
|
|
||
| Removes support for `refEl` property, which should always be referencing the `MenuButton` instance defined in the `Select` component |
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 update this language that it's addressing a bug and that refEl was never doing anything anyways?
|
Coverage after merging brooke/select-mod into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
✍️ Proposed changes
🎟 Jira ticket: LG-5656
✅ Checklist
For new components
For bug fixes, new features & breaking changes
pnpm changesetand documented my changes🧪 How to test changes