-
Notifications
You must be signed in to change notification settings - Fork 3
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
#216 add modal window email varify #262
Conversation
mode: 'all', | ||
}); | ||
|
||
const { setIsValid } = props; |
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.
You can destructure it directly in function params
export function ResendActivationFormContentComponent(({ setIsValid })) {
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.
Fixed
data: dataToSend | ||
}).then(res => console.log(res.data)).catch(error => console.log(error)); | ||
console.log(process.env.REACT_APP_BASE_API_URL); | ||
navigate('/login'); |
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.
I think we should navigate users only when activation is successful and show error when smth went wrong
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.
Fixed.
What about error, It will probably be better to do this in a new task because I need to come up with a design for the error?
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.
I do not think that we have a design for every second error state. I would recommend using existing toast alerts
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.
Fixed
}).then(res => console.log(res.data)).catch(error => console.log(error)); | ||
console.log(process.env.REACT_APP_BASE_API_URL); |
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.
Do we need all these logs in the production code?
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.
Fixed
@@ -0,0 +1,112 @@ | |||
.modal { | |||
position: absolute; /* or fixed */ |
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.
What does this comment mean?
https://stackoverflow.blog/2021/12/23/best-practices-for-writing-code-comments/
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.
Fixed
color: var(--character-title-85, rgba(0, 0, 0, 0.85)); | ||
font-feature-settings: "calt" off; | ||
|
||
/* Text/Body/16-Semi bold */ |
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.
Very obvious comment. Check best practices above
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.
Fixed. I copied styles for buttons, etc., and forgot to remove these comments.
import axios from 'axios'; | ||
import styles from './ResendActivationFormContent.module.css'; | ||
|
||
export function ResendActivationFormContentComponent({ setIsValid }) { |
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.
Could you add prop types here
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.
Fixed
Page about confirming your email
Resend email activation
Successful confirmation via email link
Failed confirmation via email link