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

OV-1: Sign In flow #15

Merged
merged 22 commits into from
Aug 21, 2024
Merged

OV-1: Sign In flow #15

merged 22 commits into from
Aug 21, 2024

Conversation

o-nedashkivska
Copy link
Collaborator

This PR adds Sign In flow:

  • /sign-in route
  • sign-in form
  • sign-in action flow
  • sign-in form validation
Screenshot 2024-08-19 at 15 17 17 Screenshot 2024-08-19 at 15 17 36 Screenshot 2024-08-19 at 15 18 07 Screenshot 2024-08-19 at 15 18 33

@o-nedashkivska o-nedashkivska linked an issue Aug 19, 2024 that may be closed by this pull request
15 tasks
@o-nedashkivska o-nedashkivska self-assigned this Aug 19, 2024
@o-nedashkivska o-nedashkivska added the FE Fronted feature label Aug 19, 2024
@o-nedashkivska o-nedashkivska added this to the Release 1.0 milestone Aug 19, 2024
@o-nedashkivska o-nedashkivska marked this pull request as ready for review August 19, 2024 12:34
};

const PasswordInput: React.FC<Properties> = ({ hasError }) => {
const [showPassword, setShowPassword] = useState<boolean>(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment on lines 79 to 80
errorMessage ||
UserValidationMessage.INVALID_DATA
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
errorMessage ||
UserValidationMessage.INVALID_DATA
errorMessage ??
UserValidationMessage.INVALID_DATA

Comment on lines 17 to 18
user: undefined,
errorMessage: undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

replace undefined with null

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I use undefined for errorMessage because on rejected case I assign state.errorMessage = action.error.message which has string | undefined type. And for user I use undefined for consistency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Error messages from the backend we will display as Toast notification so there is no need to have this field in store

You will get the error messages that we need from zod

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What about this type of error (when user with provided email does not exist or password is invalid)?
Screenshot 2024-08-19 at 16 57 38

Copy link
Collaborator

Choose a reason for hiding this comment

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

Skip it for now, we'll also handle it in our middleware

@anton-otroshchenko
Copy link
Collaborator

Please replace rem with px

we will use px throughout the code

@anton-otroshchenko anton-otroshchenko added the MERGE CONFLICTS Merge conflicts while merging the PR label Aug 20, 2024
@o-nedashkivska o-nedashkivska force-pushed the task/OV-1-add-sign-in-flow branch from 64714e3 to 92546b1 Compare August 20, 2024 09:54
@o-nedashkivska o-nedashkivska removed the MERGE CONFLICTS Merge conflicts while merging the PR label Aug 20, 2024
@nikita-remeslov nikita-remeslov added the MERGE CONFLICTS Merge conflicts while merging the PR label Aug 20, 2024
@o-nedashkivska o-nedashkivska force-pushed the task/OV-1-add-sign-in-flow branch from 92546b1 to f554d56 Compare August 20, 2024 13:02
@o-nedashkivska o-nedashkivska removed the MERGE CONFLICTS Merge conflicts while merging the PR label Aug 20, 2024
@nikita-remeslov nikita-remeslov merged commit b1dee31 into next Aug 21, 2024
2 checks passed
@nikita-remeslov nikita-remeslov deleted the task/OV-1-add-sign-in-flow branch August 21, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE Fronted feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEAT: Sign In flow
3 participants