fix(input-otp): autofocus #4296
Conversation
🦋 Changeset detectedLatest commit: 780d88a 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a patch to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/components/input-otp/src/use-input-otp.ts (1)
Line range hint
233-246: Add autoFocus to the dependencies array of useCallback.The autoFocus prop is used in getInputOtpProps but isn't included in its dependencies array, which could lead to stale closures.
Update the dependencies array to include autoFocus:
[ isRequired, isDisabled, isReadOnly, allowedKeys, inputRef, name, value, length, setValue, props.onBlur, onComplete, + originalProps.autoFocus, ],
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.changeset/smooth-trainers-walk.md(1 hunks)packages/components/input-otp/__tests__/input-otp.test.tsx(1 hunks)packages/components/input-otp/src/use-input-otp.ts(1 hunks)
🔇 Additional comments (3)
.changeset/smooth-trainers-walk.md (1)
1-5: LGTM! Changeset correctly describes the patch.
The changeset properly identifies this as a patch update and references the correct issue (#4250).
packages/components/input-otp/__tests__/input-otp.test.tsx (1)
172-185: LGTM! Test coverage is comprehensive.
The new test cases effectively verify both:
- Autofocus behavior when the prop is passed
- Default behavior when the prop is not passed
The assertions are correct and follow the established patterns in the test suite.
packages/components/input-otp/src/use-input-otp.ts (1)
233-233: LGTM! Implementation correctly handles autoFocus.
The autoFocus prop is properly forwarded to the underlying input component and integrates well with the existing focus management system.
wingkwong
changed the title
There was a problem hiding this comment.
please also add autoFocus in https://nextui.org/docs/components/input-otp#inputotp-props
macci001
force-pushed
the
macci001/fix-4250
branch
from
fffa4bb to
780d88a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/docs/content/docs/components/input-otp.mdx (2)
306-311: Enhance the autoFocus prop documentationWhile the basic documentation is correct, consider expanding the description to provide more context and usage guidance:
{ attribute: "autoFocus", type: "boolean", - description: "Whether the element should receive focus on render.", + description: "When true, the first input segment automatically receives focus when the component mounts. Useful in multi-step forms or after navigation.", default: "false" },
Line range hint
1-500: Consider adding autoFocus examples and accessibility notesTo provide comprehensive documentation for the new autoFocus feature, consider:
- Adding an example in the Usage section demonstrating the autoFocus prop
- Including a note in the Accessibility section about the autofocus behavior and its implications for screen readers
Would you like me to help draft these additional documentation sections?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/docs/content/docs/components/input-otp.mdx(1 hunks)packages/components/input-otp/__tests__/input-otp.test.tsx(1 hunks)packages/components/input-otp/src/use-input-otp.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/input-otp/tests/input-otp.test.tsx
- packages/components/input-otp/src/use-input-otp.ts
Closes #4250
📝 Description
Fixes the autofocus functionality in input-otp component## ⛳️ Current behavior (updates)
Current behavior
Screen.Recording.2024-12-09.at.7.32.24.PM.mov
🚀 New behavior
Screen.Recording.2024-12-09.at.7.34.02.PM.mov
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
autoFocusproperty for the OTP input component.