-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: signup password hint #959
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe core changes in this update enhance the signup form experience by integrating password hints. Components have transitioned from using Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SignUpForm as SignUpForm
participant HintedPasswordInput as HintedPasswordInput
participant Validator as Validator
User->>SignUpForm: Access Signup Form
SignUpForm->>HintedPasswordInput: Focus on Password Input
HintedPasswordInput->>HintedPasswordInput: Display Password Hints
User->>HintedPasswordInput: Enter Password
HintedPasswordInput->>Validator: Validate Password
Validator-->>HintedPasswordInput: Return Validation Results
HintedPasswordInput-->>User: Update Password Hints
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
@anikdhabal is attempting to deploy a commit to the Documenso Team Team on Vercel. A member of the Team first needs to authorize it. |
As per the React docs, it is advised to use |
@anikdhabal The in-built schema validation is giving errors due to the changes to Sign up: Sign in: |
Hey @adithyaakrishna thanks for the review. I've fixed it. |
Some feedback on the design, I haven't looked at the code yet:
Maybe have a look at the |
Thank you for the review. We can move the tooltip to the left side from the right as it is overflows in the new design. Additionally, I will fix the arrow symbol. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/web/src/components/forms/signup.tsx (4 hunks)
- apps/web/src/components/forms/v2/signup.tsx (4 hunks)
- packages/trpc/server/auth-router/schema.ts (1 hunks)
Additional context used
Biome
packages/trpc/server/auth-router/schema.ts
[error] 13-13: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 14-14: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 15-15: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 16-16: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 24-24: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 25-25: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 26-26: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 27-27: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
Additional comments not posted (2)
apps/web/src/components/forms/signup.tsx (1)
14-14
: Changes related to the password input handling align well with the PR objectives.Also applies to: 25-25, 36-36, 185-185
apps/web/src/components/forms/v2/signup.tsx (1)
23-23
: Changes ensure consistency across different versions of the signup form and align with the PR objectives.Also applies to: 34-34, 50-50, 314-314
export const ZHintedPasswordSchema = z | ||
.string() | ||
.regex(new RegExp('.*[A-Z].*')) | ||
.regex(new RegExp('.*[a-z].*')) | ||
.regex(new RegExp('.*\\d.*')) | ||
.regex(new RegExp('.*[`~<>?,./!@#$%^&*()\\-_+="\'|{}\\[\\];:\\\\].*'), { | ||
message: 'One special character is required', | ||
}) | ||
.min(8) | ||
.max(72); |
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.
Replace RegExp
constructor with regex literals for better performance and readability.
- .regex(new RegExp('.*[A-Z].*'))
- .regex(new RegExp('.*[a-z].*'))
- .regex(new RegExp('.*\\d.*'))
- .regex(new RegExp('.*[`~<>?,./!@#$%^&*()\\-_+="\'|{}\\[\\];:\\\\].*'), {
+ .regex(/.*[A-Z].*/)
+ .regex(/.*[a-z].*/)
+ .regex(/.*\\d.*/)
+ .regex(/.*[`~<>?,./!@#$%^&*()\\-_+="\'|{}\\[\\];:\\\\].*/, {
message: 'One special character is required',
})
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const ZHintedPasswordSchema = z | |
.string() | |
.regex(new RegExp('.*[A-Z].*')) | |
.regex(new RegExp('.*[a-z].*')) | |
.regex(new RegExp('.*\\d.*')) | |
.regex(new RegExp('.*[`~<>?,./!@#$%^&*()\\-_+="\'|{}\\[\\];:\\\\].*'), { | |
message: 'One special character is required', | |
}) | |
.min(8) | |
.max(72); | |
export const ZHintedPasswordSchema = z | |
.string() | |
.regex(/.*[A-Z].*/) | |
.regex(/.*[a-z].*/) | |
.regex(/.*\\d.*/) | |
.regex(/.*[`~<>?,./!@#$%^&*()\\-_+="\'|{}\\[\\];:\\\\].*/, { | |
message: 'One special character is required', | |
}) | |
.min(8) | |
.max(72); |
Tools
Biome
[error] 13-13: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 14-14: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 15-15: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
[error] 16-16: Use a regular expression literal instead of the RegExp constructor. (lint/complexity/useRegexLiterals)
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/web/src/components/forms/v2/signup.tsx (4 hunks)
Additional comments not posted (4)
apps/web/src/components/forms/v2/signup.tsx (4)
23-23
: Import ofZHintedPasswordSchema
looks good.The import statement for
ZHintedPasswordSchema
is correctly added. Ensure that the schema is used properly in the form validation.
34-34
: Import ofHintedPasswordInput
looks good.The import statement for
HintedPasswordInput
is correctly added. Ensure that the component is used properly in the form rendering.
50-50
: Usage ofZHintedPasswordSchema
looks good.The
ZHintedPasswordSchema
is correctly used in the schema definition. Ensure that the validation logic is appropriate and covers all necessary password requirements.
314-314
: Usage ofHintedPasswordInput
looks good.The
HintedPasswordInput
is correctly used in the form rendering. Ensure that the rendering logic is appropriate and the hint functionality works as expected.
fixes #954
14b819e39cfe404188af025df75cd6d9.mp4
Summary by CodeRabbit
New Features
Enhancements
Refactor