Skip to content

Implement user registration and login on frontend#24

Open
Vikort wants to merge 3 commits intoostis-dev:mainfrom
Vikort:feature/auth_front
Open

Implement user registration and login on frontend#24
Vikort wants to merge 3 commits intoostis-dev:mainfrom
Vikort:feature/auth_front

Conversation

@Vikort
Copy link
Copy Markdown

@Vikort Vikort commented Oct 29, 2021

Resolves #22

Copy link
Copy Markdown
Contributor

@deniskoronchik deniskoronchik left a comment

Choose a reason for hiding this comment

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

Request some changes

"redux": "^4.1.1",
"redux-devtools-extension": "^2.13.9"
"redux-devtools-extension": "^2.13.9",
"redux-thunk": "^2.4.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need this package?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I thought to use async actions when submitting the form, but when I wrote them, they worked exactly the same as sync actions.
Forgot to remove from package.json

"dependencies": {
"@material-ui/core": "^4.12.3",
"@material-ui/icons": "^4.11.2",
"axios": "^0.24.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need this? Let's do not use additional libraries for base http fucnitons.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Redesigned for fetch

"@material-ui/core": "^4.12.3",
"@material-ui/icons": "^4.11.2",
"axios": "^0.24.0",
"formik": "^2.2.9",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same question: why we need this dependency?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Didn't know how to do form validation, used what was in the template
Redesigned validation, removed formik

"redux-devtools-extension": "^2.13.9",
"redux-thunk": "^2.4.0",
"uuid": "^8.3.2",
"yup": "^0.32.11"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same there. We don't need additional dependency to replace simple regexp values.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Remove

export const ChangePassword = 'USER_CHANGE_PASSWORD';
export const ChangeOnline = 'USER_CHANGE_ONLINE';
export const ChangeAccessToken = 'USER_CHANGE_ACCESS_TOKEN';
export const UpdateAfterSubmit = 'USER_UPDATE_AFTER_SUBMIT';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need so much actions?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Didn't know I could do this

{...state, ...action.payload}

const uuid = uuidv4()

return (
<>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

?

login: Yup.string().max(255).required('Login is required'),
firstName: Yup.string().max(255).required('First name is required'),
lastName: Yup.string().max(255).required('Last name is required'),
password: Yup.string().max(255).required('Password is required')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

})
}
onSubmit={() => {
axiosInstance.post('',{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

</Formik>
</Container>
</Box>
</>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please split to separate functions.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

How to split it? Split the container from the form?


.loader-container {
height: 100%;
height: 94%;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Because if leave 100%, a scroll appears

Copy link
Copy Markdown
Contributor

@deniskoronchik deniskoronchik left a comment

Choose a reason for hiding this comment

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

Request some changes

@Vikort Vikort force-pushed the feature/auth_front branch from 122ea8a to 4fd77c0 Compare December 2, 2021 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement user register on frontend

2 participants