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

fix(signin.tsx): set type of built-in email sign-in input to "email" and add "required" attribute for browser validation #4352

Merged
merged 8 commits into from Apr 15, 2022

Conversation

raulmarindev
Copy link
Contributor

@raulmarindev raulmarindev commented Apr 7, 2022

Reasoning 💡

Currently, the built-in sign-in page has the wrong type (text) for the input for emails when using
the magic link provider. Because of this, a user can enter whatever value they want in that input and click on the "Sign in with Email". By switching the type to be "email" we can make use of the browser-provided validation and users will be
forced to use properly formed emails addresses in order to sign in.

image

image

Additionally, the built-in sign-in page doesn't have the required attribute for the input for emails when using the magic link provider. Because of this, a user is able to submit the form without entering a value. By adding the "required" attribute we can make use of the browser-provided validation and users will be forced to provide a value in order to sign in.

Checklist 🧢

  • Documentation
  • Tests
  • Ready to be merged

Affected issues 🎟

…to email for browse validation

Currently, the built-in sign-in page has the wrong type (text) for the input for emails when using
the magic link provider. Because of this a user can enter whatever they want in that input. By
switching the type to be email we can make use of the browser provided validation and users will be
forced to use properly formed emails addresses in order to sign-in.
@vercel
Copy link

vercel bot commented Apr 7, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/EA2pfZwuVZ9dFnrcVKFJn76Sst8j
✅ Preview: https://next-auth-git-fork-raulmarindev-main-nextauthjs.vercel.app

@github-actions github-actions bot added core Refers to `@auth/core` pages labels Apr 7, 2022
@raulmarindev raulmarindev changed the title fix(signin.tsx): set type of built-in email sign-in input to email for browse validation fix(signin.tsx): set type of built-in email sign-in input to email for browser validation Apr 7, 2022
Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Good catch, yeah, I think we can also make it required. 👍

Copy link
Collaborator

@ubbe-xyz ubbe-xyz left a comment

Choose a reason for hiding this comment

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

Good catch 💚

@ubbe-xyz
Copy link
Collaborator

ubbe-xyz commented Apr 9, 2022

@raulmarindev lets make it required also as per @balazsorban44 hint 👍🏽

@vercel vercel bot temporarily deployed to Preview April 9, 2022 10:49 Inactive
@ubbe-xyz ubbe-xyz self-assigned this Apr 9, 2022
…n email input

Currently, the built-in sign-in page doesn't have the required attribute for the input for emails
when using the magic link provider. Because of this a user is able to submit the form without
entering a value. By adding the "required" attribute we can make use of the browser provided
validation and users will be forced to provide a value in order to sign-in.
@raulmarindev
Copy link
Contributor Author

@lluia Thank you both, I've added the required attribute

@raulmarindev raulmarindev changed the title fix(signin.tsx): set type of built-in email sign-in input to email for browser validation fix(signin.tsx): set type of built-in email sign-in input to "email" and add "required" attribute for browser validation Apr 9, 2022
Copy link
Collaborator

@ubbe-xyz ubbe-xyz left a comment

Choose a reason for hiding this comment

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

Nice 💯

@vercel vercel bot temporarily deployed to Preview April 10, 2022 09:47 Inactive
@vercel vercel bot temporarily deployed to Preview April 13, 2022 10:22 Inactive
@vercel vercel bot temporarily deployed to Preview April 15, 2022 16:59 Inactive
@ubbe-xyz ubbe-xyz merged commit fd755bc into nextauthjs:main Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core` pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants