-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix/listbox remove deprecated isdisabled #5865
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
base: canary
Are you sure you want to change the base?
Fix/listbox remove deprecated isdisabled #5865
Conversation
…eroui-inc#5863) Remove deprecated `isDisabled` and `isSelected` props from `AriaOptionProps` inheritance in `ListboxItemBaseProps`. These props were deprecated in react-aria and should not be used on individual items. Users should use the `disabledKeys` prop on the parent component instead. Changes: - Remove `isDisabled` and `isSelected` from AriaOptionProps inheritance - Update test to use `disabledKeys` instead of `isDisabled` prop - Add changeset documenting the breaking change
🦋 Changeset detectedLatest commit: 0dd973a The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
@dharmveer97 is attempting to deploy a commit to the HeroUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request removes the deprecated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/remove-deprecated-isdisabled-prop.md(1 hunks)apps/docs/app/layout.tsx(2 hunks)packages/components/autocomplete/__tests__/autocomplete.test.tsx(1 hunks)packages/components/listbox/src/base/listbox-item-base.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/docs/app/layout.tsx (1)
apps/docs/components/pro-banner.tsx (1)
ProBanner(13-117)
🔇 Additional comments (3)
packages/components/autocomplete/__tests__/autocomplete.test.tsx (1)
982-984: LGTM! Test correctly updated to use parent-leveldisabledKeys.The test has been properly migrated from using per-item
isDisabledprop to the parent component'sdisabledKeyspattern. The test logic remains valid and correctly verifies that focus moves to the first non-disabled item when the first item is disabled..changeset/remove-deprecated-isdisabled-prop.md (1)
1-24: Excellent changeset documentation with clear migration guide.The changeset provides:
- Clear description of the change
- Concrete before/after code examples
- Proper issue reference (#5863)
The migration path is straightforward and well-documented, making it easy for users to update their code.
packages/components/listbox/src/base/listbox-item-base.tsx (1)
95-95: Now let me search for HeroUI v2's specific versioning policy for handling deprecated property removals:Let me check the PR's changeset file to see what version bump type is specified:
Based on my research, I need to provide a definitive assessment. Semantic Versioning specifies that major version must be incremented if any backward incompatible changes are introduced to the public API. However, Semantic Versioning's deprecation guidance recommends that when deprecating functionality, you should issue a new minor release with the deprecation in place, and removal should occur in a new major release.
HeroUI's actual practice differs: HeroUI v2.7.0 shipped with ListboxItem, SelectItem & AutocompleteItem no longer accepting certain props and these were listed as breaking changes in a release that bumped the minor version (2.6.0 → 2.7.0).
Clarify the changeset version bump type.
The review concern about patch vs. major versioning has merit, but the actual answer depends on which versioning component this PR uses. If the changeset marks this as a "patch" bump (incrementing Z in X.Y.Z), that would contradict both strict semver and HeroUI's own practice. If it's marked as "minor" (incrementing Y), it would align with HeroUI's demonstrated convention for breaking prop removals, though it deviates from strict semver. Verify the changeset file's version bump designation (patch, minor, or major) to confirm whether it follows HeroUI's established pattern or requires adjustment.
| import {Navbar} from "@/components/navbar"; | ||
| import {Footer} from "@/components/footer"; | ||
| import {RandomBanner} from "@/components/random-banner"; | ||
| import {ProBanner} from "@/components/pro-banner"; |
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 change appears unrelated to the PR objectives.
The PR objectives state this PR "removes the deprecated isDisabled and isSelected props from ListboxItem and AutocompleteItem" and closes issue #5863 about deprecated props. However, this file changes the docs site banner component from RandomBanner to ProBanner, which is unrelated to the component API changes.
Consider moving this docs site improvement to a separate PR to keep changes focused and make the review clearer.
Also applies to: 86-86
🤖 Prompt for AI Agents
In apps/docs/app/layout.tsx around lines 16 and 86 the import and usage were
changed from the docs' RandomBanner to ProBanner, which is unrelated to removing
deprecated isDisabled/isSelected props; revert the import and any usage back to
the original RandomBanner (restore the prior import line and component reference
at the indicated lines) so the PR only contains the component API changes, and
move the ProBanner/docs banner update into a separate PR for the docs
improvement.
Closes #5863
📝 Description
This PR removes the deprecated
isDisabledandisSelectedprops fromListboxItemandAutocompleteItemcomponents. These props were deprecated in react-aria'sAriaOptionPropsinterface and should no longer be exposed to users.⛳️ Current behavior (updates)
Currently,
ListboxItemandAutocompleteIteminherit all props from react-aria'sAriaOptionProps, including the deprecatedisDisabledandisSelectedprops. This allows users to write code like:However, this pattern is deprecated in react-aria and triggers deprecation warnings in TypeScript.
🚀 New behavior
After this PR, the deprecated props are excluded from the type definition. Users must use the
disabledKeysprop on the parent component instead:This approach:
Changes made:
isDisabledandisSelectedfromAriaOptionPropsinheritance inListboxItemBasePropsdisabledKeyspattern💣 Is this a breaking change (Yes/No):
Yes - Minor breaking change
Impact
Users who are using
isDisabledorisSelectedprops directly onListboxItemorAutocompleteItemwill get TypeScript errors after this update.Migration Path
Replace individual item
isDisabledprops with thedisabledKeysprop on the parent component:Before:
After:
This pattern works for all components using ListboxItem:
Autocomplete,Select,Listbox,Dropdown, etc.📝 Additional Information
disabledKeyspattern has been the recommended approach and is already widely used in the codebasedisabledKeys, so most users should not be affectedisDisabledprop inAriaOptionPropswas marked as deprecated to encourage centralized disabled state managementSummary by CodeRabbit
Release Notes
Breaking Changes
isDisabledandisSelectedprops from listbox and autocomplete item components. Manage disabled state via the parent component'sdisabledKeysprop instead.Documentation