-
-
Notifications
You must be signed in to change notification settings - Fork 361
Add Change Password section to account settings
#2419
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
base: dev
Are you sure you want to change the base?
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
Preview: https://2419--pr-cinny.netlify.app |
ajbura
left a comment
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.
Apart from the requested changes can we remove the inline comment added at various places as most of the changes are self explanatory and doesn't really need comment.
| Your password has been successfully changed. Your other devices may need to be | ||
| re-verified. |
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 is it mean to "other devices may need to be re-verified"?
|
|
||
| <Box direction="Column" gap="100"> | ||
| <Box alignItems="Center" gap="200"> | ||
| <input type="checkbox" id="logoutDevices" name="logoutDevices" defaultChecked /> |
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.
please use the Checkbox component from folds
| <Box alignItems="Center" gap="200"> | ||
| <input type="checkbox" id="logoutDevices" name="logoutDevices" defaultChecked /> | ||
| <Text as="label" htmlFor="logoutDevices" size="T300"> | ||
| Sign out all other devices |
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.
Cinny uses "Logout" instead of term "Signout" in all places.
| Recommended for security. Unchecking this may leave your other devices logged | ||
| in. |
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.
We can remove recommendation as it is already widely known, and set the default checkbox to true.
| <Box gap="200" justifyContent="End"> | ||
| <Button type="button" variant="Secondary" onClick={onCancel} disabled={isLoading}> | ||
| <Text as="span" size="B400"> | ||
| Cancel | ||
| </Text> | ||
| </Button> | ||
| <Button variant="Primary" type="submit" disabled={isLoading}> | ||
| {isLoading && <Spinner variant="Primary" size="300" />} | ||
| <Text as="span" size="B400"> | ||
| Change Password | ||
| </Text> | ||
| </Button> | ||
| </Box> |
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.
Cinny design language uses the "Column" layout for button. And we do not need the cancel button as dialog header already has the Cross button.
| * @param logoutDevices Whether to logout other devices (defaults to true for security) | ||
| * @returns Tuple with either auth data (for UIA continuation) or success response | ||
| */ | ||
| export const changePassword = async ( |
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.
As this function will not be reused at multiple places it is not counted as utility. We should just moved it to the same file it is used.
Description
Implement change password component in Settings/Account. The component uses matrix-js-sdk's setpassword function, and completes the full authentication flow to set a new password. No new dependencies are needed.
Further changes might be necessary to properly match the visual style of Cinny, but I'm not great at frontend development.
Closes #2417
Type of change
Checklist: