Skip to content
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

feat: new accessible otp input component #2428

Closed
wants to merge 13 commits into from

Conversation

damianricobelli
Copy link
Contributor

@damianricobelli damianricobelli commented Jan 15, 2024

This PR is a preview that adds an accessible component called OTPInput based on the react-otp-input that allows to render as many inputs as you want (based on the Input component of shadcn) through a numInputs prop so that a user can write OTP verification codes in an accessible way.

The component allows:

  • Copy and paste from clipboard
  • Auto focus
  • Move through inputs with arrows and keyboard tabs
  • Delete code and move the focus to the previous input
  • Type a character and move the focus to the next input
  • Setter and getter props to control component values
  • Input of type hidden to allow to have an input with an id that we pass as prop ready for RSC or forms that require to capture data from formData

Pending tasks:

  • Document the component
  • Fix a focus use case where if the user moves the focus to a different input than the first one and starts typing, the characters start to be inserted from the first input instead of the focused input.

Previews
image
image

I would appreciate feedback and anything you expect from a component of this type

Copy link

vercel bot commented Jan 15, 2024

@damianricobelli is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

@lokeshvasnik
Copy link

A OTP verification input fields #1431

Copy link

vercel bot commented Jan 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 5, 2024 6:05am

@coleellis
Copy link

Love the feature here. Threw it into my application and it works great. Hope to see this get released officially!

Reference:
image

@harrysolovay
Copy link

This is looking spectacular! Might also be worthwhile to have an example of usage with zod and react-hook-form (as do other inputs).

@damianricobelli
Copy link
Contributor Author

@shadcn The component is ready to be sent to production. I have added 8 complete examples of how to use it

Copy link

@simon-v-swyftx simon-v-swyftx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For compatibility with password managers and native phone keyboards, autoComplete should be one-time-code. Unsure how that will affect the rest of your implementation

https://github.com/shadcn-ui/ui/pull/2428/files#diff-baa521c77caec6c807432e289ca2a4da22ea3ee3f88062c69daf80d695b24f8eR194

@shadcn
Copy link
Collaborator

shadcn commented Feb 6, 2024

This is amazing! Thanks @damianricobelli

@shadcn shadcn added the area: roadmap This looks great. We'll add it to the roadmap, review and merge. label Feb 6, 2024
@damianricobelli
Copy link
Contributor Author

For compatibility with password managers and native phone keyboards, autoComplete should be one-time-code. Unsure how that will affect the rest of your implementation

https://github.com/shadcn-ui/ui/pull/2428/files#diff-baa521c77caec6c807432e289ca2a4da22ea3ee3f88062c69daf80d695b24f8eR194

Done, thanks

@hm-arora
Copy link

This is amazing 🔥 @damianricobelli

@hm-arora
Copy link

shadcn_otp_input.mp4

When attempting to use arrow keys to shift focus between input boxes, the input unintentionally gets entered into the first box rather than the currently focused one

@shadcn
Copy link
Collaborator

shadcn commented Mar 5, 2024

@damianricobelli What are your thoughts on https://input-otp.rodz.dev? It's built as a headless component.

(Tagging @guilhermerodz if he's available to help us here. Thanks)

@damianricobelli
Copy link
Contributor Author

damianricobelli commented Mar 5, 2024

@shadcn hey, I have reviewed the source code and experience of the component you mention. In my opinion it is excellent, but I think the API built here in this PR (and the code) is simpler to understand and refactor for those people who want to use it and modify the component in case they need a different functionality and make it their own.

What I would like to take from the operation of the component you mentioned is:

  • If focus is done on an empty element, that focus is done on the first element of them. I mean: if there are 6 elements and all of them are empty, and you click on any of them the element with focus will be the first one. If there are 3 with values, and you click on any of the remaining 3, the focus will be on 4 which is the first empty element.
  • That you can move to the right until +1 empty element to avoid the problem mentioned by @hm-arora

I think that with those two details the component of this PR is extremely flexible and customizable.

What do you think?

@damianricobelli
Copy link
Contributor Author

@shadcn I have made a last important commit that improves all the usability issues that are still pending in the PR and issues of the react-otp-input library from which this component is inspired.

In addition, an example of how to give custom styles to the component is added demonstrating how to achieve in a very simple way the main example of the library that you mentioned in your comment. Although it does not have animations, the idea is to demonstrate how flexible it is to customize the container and each input of the component.

Among the improvements, the problem mentioned by @hm-arora is solved

@guilhermerodz
Copy link

guilhermerodz commented Mar 5, 2024

hi @shadcn @damianricobelli of course I would love to help building the OTP input for shadcn-ui!

I reviewed this PR, however I think this is not the best approach... why:

  • I'm not a fan of exposing logic as the primitive input in the component/code provided by shadcn-ui. You can see shadcn-ui already includes components that consume Radix primitives, @emilkowalski 's Vaul and Sonner.
  • @damianricobelli your implementation is beautiful, but it isn't fully accessible as you are rendering 6 (multiple = N) inputs instead of a single one. That way screen readers cannot behave like they do with single inputs...

It would be nice for shadcn-ui to only consume/configure primitives, say import + customize otp-input or any other OTP input library. And then providing this as source code at shadcn-ui docs. This is also nice because it supports updates/versioning of the underlying primitive library itself.

also, will be pushing a 1.0 (not beta) version of input-otp today! and then @shadcn everyone let me know your thoughts.

@damianricobelli I love the examples you've built!!

@damianricobelli
Copy link
Contributor Author

@guilhermerodz thank you very much for your feedback. You are right about the accessibility aspect

@shadcn do you prefer to continue with an approach where the otp-input component is used? Or to use the current implementation but adding that important accessibility detail that Guilherme mentions?

@shadcn
Copy link
Collaborator

shadcn commented Mar 6, 2024

It would be nice for shadcn-ui to only consume/configure primitives, say import + customize otp-input or any other OTP input library.

@damianricobelli let's go with @guilhermerodz's otp-input.

I think this follows the idea/principle behind the components. (Though we're starting to see a few cases where the underlying components are getting too complex - I'm working on a solution for this)

@shadcn
Copy link
Collaborator

shadcn commented Mar 19, 2024

@damianricobelli Just a quick thank you before I close this PR. Appreciate your help on kicking this off. Thank you.

@shadcn shadcn closed this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: roadmap This looks great. We'll add it to the roadmap, review and merge. new component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants