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

525 implement ability to change password #527

Merged
merged 19 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
75db25c
Added ability to change password
YanZhylavy Apr 15, 2024
66db9ca
Fixed minor mistake in ChangePassword props
YanZhylavy Apr 15, 2024
3cadd2a
Added props validation for PasswordField
YanZhylavy Apr 15, 2024
0a9e3c6
Corrected typo in requiredField prop
YanZhylavy Apr 16, 2024
394baa6
Corrected more typos in requiredField prop
YanZhylavy Apr 16, 2024
8ea4c06
Added aria-label attribute for password visibility span and props des…
YanZhylavy Apr 16, 2024
8e60a30
Merge branch 'develop' into 525-implement-ability-to-change-password
YanZhylavy Apr 16, 2024
6176201
Merge branch 'develop' into 525-implement-ability-to-change-password
YanZhylavy Apr 17, 2024
2a2f783
Removed unnecessary div for error when there is no error in PasswordF…
YanZhylavy Apr 17, 2024
c59ddbd
Refactored handleInputChange for ChangePassword component
YanZhylavy Apr 17, 2024
7b44ebe
Added useForm for ChangePassword form handling
YanZhylavy Apr 18, 2024
101b586
Moved error messages into separate const
YanZhylavy Apr 18, 2024
327dc01
Added reset() instead of manually setting form values to default afte…
YanZhylavy Apr 18, 2024
1cf6d66
Destructured props for PasswordField
YanZhylavy Apr 18, 2024
588a1cb
Removed unused prop requiredField
YanZhylavy Apr 18, 2024
cbcb783
Merge branch 'develop' into 525-implement-ability-to-change-password
YanZhylavy Apr 18, 2024
50a6a7b
Merge branch 'develop' into 525-implement-ability-to-change-password
YanZhylavy Apr 18, 2024
7750f44
Improved PasswordField reusability, fixed label-input connection
YanZhylavy Apr 19, 2024
029340d
Made password input required
YanZhylavy Apr 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions FrontEnd/src/components/ProfilePage/FormComponents/ChangePassword.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import axios from 'axios';
import { PropTypes } from 'prop-types';
import { toast } from 'react-toastify';
import { useState, useEffect } from 'react';
import { useContext } from 'react';
import { DirtyFormContext } from '../../../context/DirtyFormContext';
import PasswordField from './FormFields/PasswordField';
import Loader from '../../loader/Loader';
import css from './ChangePassword.module.css';
import { PASSWORD_PATTERN } from '../../../constants/constants';


export default function ChangePassword(props) {
const [formData, setFormData] = useState({ currentPassword: '', newPassword: '', reNewPassword: '' });
const [passwordsDontMatchError, setPasswordsDontMatchError] = useState(null);
const [invalidPasswordError, setInvalidPasswordError] = useState(null);
const { setFormIsDirty } = useContext(DirtyFormContext);

useEffect(() => {
props.currentFormNameHandler(props.curForm);
}, [props]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

useEffect has [props] as a dependency, which could lead to unnecessary re-executions if props object itself changes but not its values. Consider destructuring props outside useEffect and using specific properties in the dependency array
e.g.

const { currentFormNameHandler, curForm } = props;
useEffect(() => {
  currentFormNameHandler(curForm);
}, [currentFormNameHandler, curForm]);


useEffect(() => {
setFormIsDirty(Object.keys(formData).some((field) => formData[field] !== ''));
}, [formData]
);

const handleSubmit = (event) => {
event.preventDefault();
if (!invalidPasswordError && !passwordsDontMatchError) {
axios.post(`${process.env.REACT_APP_BASE_API_URL}/api/auth/users/set_password/`, {
current_password: formData.currentPassword,
new_password: formData.newPassword,
re_new_password: formData.reNewPassword
})
.then(() => toast.success('Пароль успішно змінено'))
.catch(() => toast.error('Виникла помилка. Можливо, вказано невірний поточний пароль'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The .catch() block only displays a generic error message. Consider improving error handling by parsing the error response from the server to provide more specific feedback to the user

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you! Initially, I was outputting the error status code, but the team advised against it as it's not user-friendly. I'll consider better ways to inform the user.

setFormData({ currentPassword: '', newPassword: '', reNewPassword: '' });
}
};

const handleInputChange = (e) => {
e.preventDefault();
const updatedFormData = { ...formData, [e.target.name]: e.target.value };
setFormData(updatedFormData);
(updatedFormData.newPassword !== '' &&
!PASSWORD_PATTERN.test(updatedFormData.newPassword)) ?
setInvalidPasswordError('Пароль не відповідає вимогам') :
setInvalidPasswordError(null);
(updatedFormData.reNewPassword !== '' &&
updatedFormData.newPassword !== updatedFormData.reNewPassword) ?
setPasswordsDontMatchError('Паролі не співпадають') :
setPasswordsDontMatchError(null);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

The function updates state multiple times which can lead to performance issues or unexpected renders. It would be better to consolidate state updates into a single function call if possible

Copy link
Collaborator

Choose a reason for hiding this comment

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

const handleInputChange = (e) => {
    e.preventDefault();
    const { name, value } = e.target;
    const newFormData = { ...formData, [name]: value };

    const newInvalidPasswordError = newFormData.newPassword !== '' && !PASSWORD_PATTERN.test(newFormData.newPassword)
        ? 'Пароль не відповідає вимогам'
        : null;

    const newPasswordsDontMatchError = newFormData.reNewPassword !== '' && newFormData.newPassword !== newFormData.reNewPassword
        ? 'Паролі не співпадають'
        : null;

    // Update all states in one go
    setFormData(newFormData);
    setInvalidPasswordError(newInvalidPasswordError);
    setPasswordsDontMatchError(newPasswordsDontMatchError);
};


return (
<div className={css['form__container']}>
{props.user
?
<form id="ChangePassword" onSubmit={handleSubmit}>
<PasswordField
name="currentPassword"
label="Поточний пароль"
updateHandler={handleInputChange}
value={formData.currentPassword}
requredField={true}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There seems to be a typo in the requredField prop (should be requiredField). Moreover, HTML5 form validation could be utilized by using the required attribute on inputs instead of handling this manually

/>
<PasswordField
name="newPassword"
label="Новий пароль"
updateHandler={handleInputChange}
value={formData.newPassword}
requredField={true}
error={invalidPasswordError}
/>
<PasswordField
name="reNewPassword"
label="Повторіть новий пароль"
updateHandler={handleInputChange}
value={formData.reNewPassword}
requredField={true}
error={passwordsDontMatchError}
/>
</form>
: <Loader />
}
</div>
);
}

ChangePassword.propTypes = {
user: PropTypes.shape({
id: PropTypes.number.isRequired,
email: PropTypes.string.isRequired,
name: PropTypes.string.isRequired,
surname: PropTypes.string.isRequired,
profile_id: PropTypes.number.isRequired,
is_staff: PropTypes.bool.isRequired
}).isRequired,
currentFormNameHandler: PropTypes.func.isRequired,
curForm: PropTypes.string.isRequired
};


Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.form__container {
word-wrap: break-word;
width: 530px;
margin-left: 10px;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { useState } from 'react';
import { PropTypes } from 'prop-types';
import EyeInvisible from '../../../authorization/EyeInvisible';
import EyeVisible from '../../../authorization/EyeVisible';
import css from './PasswordField.module.css';

const PasswordField = (props) => {
const [showPassword, setShowPassword] = useState(false);

const togglePassword = () => {
setShowPassword(!showPassword);
};

return (
<div className={css['password-field__item']}>
<div className={css['password-field__label-wrapper']}>
{props.requredField &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

requredField seems to be misspelled. The correct spelling should be requiredField

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you're right! Initially, I was reusing the existing HalfFormField component in the project where this typo was present (the component was taking prop 'requredField' instead of 'requiredField'), and then I forgot to correct the improper prop passing when I wrote my PasswordField.

<span>
*
</span>}
<label
htmlFor={props.inputId}
>
{props.label}
</label>
</div>
<div className={css['password-field__password']}>
<div className={css['password-field__password__wrapper']}>
<input
id={props.inputId}
name={props.name}
type={showPassword ? 'text' : 'password'}
value={props.value}
onChange={props.updateHandler}
placeholder={props.label}
required={props.requredField}
/>
</div>
<span
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using a <span> for the password visibility toggle is not ideal because spans are not inherently focusable or interactive elements. Consider using a <button> instead, which is more appropriate for actions and improves accessibility
e.g.

<button className={css['password-visibility']} onClick={togglePassword} type="button">
   {!showPassword ? <EyeInvisible /> : <EyeVisible />}
</button>

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 agree with this point. I added a span there because it's done that way in our LoginPage component.

className={css['password-visibility']}
onClick={togglePassword}
>
Copy link
Collaborator

Choose a reason for hiding this comment

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

The toggle button for showing/hiding the password doesn’t have an accessible label or an aria-label attribute. Adding accessible labels should be useful for users who rely on screen readers
e.g.

<span className={css['password-visibility']} onClick={togglePassword} aria-label={showPassword ? "Hide password" : "Show password"}>
   {!showPassword ? <EyeInvisible /> : <EyeVisible />}
</span>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you, will be done.

{!showPassword ? <EyeInvisible /> : <EyeVisible />}
</span>
</div>
{(props.requredField || props.error) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

The error message section will display even if props.error is an empty string and props.requredField is true. It might be more logical to display this section only if there is an error message to show

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you, I'll consider it.

<div className={css['error-message']}>
{props.error}
</div>
}
</div>
);
};

PasswordField.propTypes = {
name: PropTypes.string.isRequired,
label: PropTypes.string.isRequired,
updateHandler: PropTypes.func.isRequired,
value: PropTypes.string,
inputId: PropTypes.string,
requredField: PropTypes.bool,
error: PropTypes.string
};

export default PasswordField;
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
.password-field__item {
display: flex;
width: 257px;
height: 89px;
flex-direction: column;
align-items: flex-start;
}

.password-field__label-wrapper {
padding-bottom: 9px;
}

.password-field__label-wrapper span {
padding-right: 5px;
color: #FF4D4F;
}

.password-field__item input {
border: none;
}
.password-field__item input:focus {
border: none;
outline: none;
}

.password-field__item input::placeholder {
font-style: normal;
font-family: "Inter", sans-serif;
font-size: 14px;
font-weight: 400;
line-height: 22px;
letter-spacing: -0.01em;
text-align: left;
color: #00000040;
}
.password-field__password {
display: flex;
padding: 4px 12px;
align-items: center;
gap: 4px;
align-self: stretch;
border-radius: 2px;
border: 1px solid #d9d9d9;
background: #fff;
}

.password-field__password:focus {
border-radius: 2px;
border: 1px solid #1f9a7c;
background: #fff;
outline: none;
}

.password-field__password:focus-within {
border-radius: 2px;
border: 1px solid #1f9a7c;
background: #fff;
}

.password-field__password__wrapper {
display: flex;
}

.password-visibility {
cursor: pointer;
margin-left: auto;
}

.error-message {
display: flex;
padding: 1px 0px;
align-items: flex-start;
gap: 10px;
align-self: stretch;
flex: 1 0 0;
color: var(--red-red-100, #F34444);
font-family: Roboto;
font-size: 14px;
font-style: normal;
font-weight: 400;
line-height: 22px;
}

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const DESCRIPTIONS = {
'AdditionalInfo': 'Інформація про компанію',
'StartupInfo': 'Інформація про стартап',
'Delete': 'Видалення профілю',
'ChangePassword': 'Зміна паролю користувача'
};

const Description = (props) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import UserInfo from '../FormComponents/UserInfo';
import ProfileFormButton from '../UI/ProfileFormButton/ProfileFormButton';
import MyModal from '../UI/MyModal/MyModal';
import WarnUnsavedDataModal from '../FormComponents/WarnUnsavedDataModal';
import ChangePassword from '../FormComponents/ChangePassword';
import css from './ProfileContent.module.css';


Expand Down Expand Up @@ -53,7 +54,8 @@ const FORM_NAMES = [
'ProductServiceInfo',
'AdditionalInfo',
'StartupInfo',
'Delete'
'Delete',
'ChangePassword'
];

const ProfileContent = (props) => {
Expand Down Expand Up @@ -143,6 +145,11 @@ const ProfileContent = (props) => {
);
})}
<div className={css['divider']}></div>
<Link
to="/profile/change-password"
className={`${css['infolink']}`}>
Змінити пароль
</Link>
<Link
to="/profile/delete"
className={`${css['infolink']} ${css['delete']}`}>
Expand Down Expand Up @@ -191,6 +198,12 @@ const ProfileContent = (props) => {
profile={props.profile}
currentFormNameHandler={props.currentFormNameHandler}
curForm={FORM_NAMES[5]} />} />
<Route
path="/change-password"
element={<ChangePassword
user={props.user}
currentFormNameHandler={props.currentFormNameHandler}
curForm={FORM_NAMES[7]} />} />
</Routes>
</DirtyFormContext.Provider>
</div>
Expand Down
Loading