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(PinInput): implement component #2570

Merged
merged 44 commits into from
Nov 12, 2024
Merged

feat(PinInput): implement component #2570

merged 44 commits into from
Nov 12, 2024

Conversation

maxsteinwand
Copy link
Contributor

@maxsteinwand maxsteinwand commented Nov 8, 2024

πŸ”— Linked issue

Resolves #1123

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Add the PinInput component from radix-vue. https://www.radix-vue.com/components/pin-input
Tests and Documentation are also added.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@maxsteinwand
Copy link
Contributor Author

I hope this implementation is okay even tough the radix-vue component is tagged as alpha in their docs. It will not work in a Form (validation etc.) i struggled to get that working. Hope the tests are good/fine as well as i have no expierence with that so far πŸ˜„
I oriented myself on the normal input and some other radix-vue implementations.

Copy link
Member

@benjamincanac benjamincanac left a comment

Choose a reason for hiding this comment

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

Wow 🀩 Thank you for doing this, you just saved me a lot of time and the PR looks perfect!

Did you find the CLI to start the component?

docs/content/3.components/pin-input.md Outdated Show resolved Hide resolved
---
::

### Mask
Copy link
Member

Choose a reason for hiding this comment

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

Could it be worth having an example with otp prop? Not sure what it does exactly though.

Copy link
Contributor Author

@maxsteinwand maxsteinwand Nov 8, 2024

Choose a reason for hiding this comment

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

I was thinking about that too but there is nothing to showcase. It does set the autocomplete type to "one-time-code". This is defined here: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete#one-time-code
which then allows mobile users to autocomplete with the clipboard or from recieved messages.
https://github.com/unovue/radix-vue/blob/main/packages/radix-vue/src/PinInput/PinInputInput.vue#L159

Copy link
Member

@benjamincanac benjamincanac Nov 9, 2024

Choose a reason for hiding this comment

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

Shouldn't this be set by default then? πŸ€” Nevermind, I added it to the docs!

src/runtime/components/PinInput.vue Outdated Show resolved Hide resolved
src/runtime/components/PinInput.vue Outdated Show resolved Hide resolved
src/theme/pin-input.ts Outdated Show resolved Hide resolved
@benjamincanac
Copy link
Member

You also have to run pnpm test:vue to create the snapshots 😊 I haven't found a way to execute both at the same time yet.

@benjamincanac benjamincanac added the v3 #1289 label Nov 8, 2024
@benjamincanac
Copy link
Member

@romhml Could you have a look into this to handle it inside a form?

@maxsteinwand
Copy link
Contributor Author

@benjamincanac Thanks for the quick feedback! I adjusted the docs as you said. Yeah i found the cli, but actually after i already created the files and then i renamed everything againπŸ˜„

Copy link

pkg-pr-new bot commented Nov 8, 2024

pnpm add https://pkg.pr.new/@nuxt/ui@2570

commit: ca66e80

@benjamincanac benjamincanac requested a review from romhml November 9, 2024 16:42
@romhml
Copy link
Collaborator

romhml commented Nov 11, 2024

I've made some changes to integrate the PinInput with the Form/FormField components. One thing I could not figure out is how to emit a @blur event, @maxsteinwand do you have an idea?

@maxsteinwand
Copy link
Contributor Author

@romhml i added the blur event. The event.relatedTarget will be null if the user leaves the input. It would be set to the next input element if the user enters a value. But i am still thinking about the change event. If you now test on the playground and you enter all 5 values it will validate and mark the field as invalid (because max. 4 allowed) but if you then remove the last entry it will not validate again as the change will only get called once all fields are filled.

@romhml
Copy link
Collaborator

romhml commented Nov 11, 2024

I made the playground example to test that the validation is triggered, I'll update it to have something more realistic.

@romhml
Copy link
Collaborator

romhml commented Nov 11, 2024

I added the form validation on blur and updated the example. The playground example does not work properly currently because the input uses an array rule, I'll do another PR to fix this.

@romhml
Copy link
Collaborator

romhml commented Nov 11, 2024

I added the error-pattern prop here which should be used in the PinInput example to correctly associate the errors with the form field.

@benjamincanac benjamincanac merged commit 95aa6f6 into nuxt:v3 Nov 12, 2024
2 checks passed
@benjamincanac
Copy link
Member

Thanks @maxsteinwand! 😊

@maxsteinwand maxsteinwand deleted the PinInput branch November 12, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 #1289
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants