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

#284 fe restore password #285

Merged
merged 9 commits into from
Nov 15, 2023
Merged

#284 fe restore password #285

merged 9 commits into from
Nov 15, 2023

Conversation

whooaami
Copy link
Collaborator

@whooaami whooaami commented Oct 28, 2023

Info about reset password
Знімок екрана 2023-10-30 о 21 00 03

Page about sending email
Знімок екрана 2023-10-30 о 21 00 37

Form for resetting password
Знімок екрана 2023-10-30 о 21 41 56

Password successfully restored
Знімок екрана 2023-10-30 о 22 10 19

Password restored failed
Знімок екрана 2023-10-30 о 22 40 26

@whooaami whooaami added the task label Oct 28, 2023
@whooaami whooaami added this to the Sprint 6 milestone Oct 28, 2023
@whooaami whooaami self-assigned this Oct 28, 2023
@whooaami whooaami requested a review from popovycholeg October 30, 2023 20:42
return <div className={css['dot-row']}>{dots}</div>;
};

function DotDecorComponent(props) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add prop-types

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines 1 to 4
import styles from './RestorePasswordForm.module.css';
import { RestorePasswordFormContentComponent } from './restorepassword-form/RestorePasswordFormContent';
import { useState } from 'react';
import { Link } from 'react-router-dom';
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: libs import usually is above than custom component/styles. It would be nice to add eslint rule for it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

email: 'Email не відповідає вимогам',
};

const emailPattern = /^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}$/i;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we use emailPattern only here? It would be good to create a constant for it and reuse it in all places (Login, Register...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines +30 to +31
const formIsValid = isValid;
setIsValid(formIsValid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we use isValid directly w/o reasigning?

color: var(--red-red-100, #f34444);

/* Body/regular */
font-family: Roboto, sans-serif;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we use this font for most of the text boxes it makes sense to define it as a global style and do to duplicate it every time

confirmPassword: 'Паролі не збігаються',
};

const passwordPattern = /^(?=.*[0-9])(?=.*[a-zA-Z])[a-zA-Z0-9]{8,20}$/;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to use the same patter for Register and Login. It makes sense to export it from one place

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed


axios({
method: 'post',
url: `${process.env.REACT_APP_BASE_API_URL}/api/auth/users/reset_password/`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need to use this env var? I thought that we decided to use the relative path since API leaves on the same domain

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 do not have any information about it

Comment on lines 68 to 70
<div>

<div className={styles['reset-password-form']}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bad formatting

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed


export function RestorePasswordModalPage() {
return (
<div className={styles['modal']}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we create a shared modal component and reuse it for the rest modals?

Copy link
Collaborator

@popovycholeg popovycholeg left a comment

Choose a reason for hiding this comment

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

Non-blocking comments. But I would appreciate these improvements

@whooaami whooaami merged commit 3e7bc69 into develop Nov 15, 2023
4 checks passed
@whooaami whooaami deleted the #284-FERestorePassword branch November 15, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants