-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
WIP: Revisit - Update password reset page to use only react-form capabilities #23147
Conversation
While I certainly support the direction of these code changes, I have a few suggestions. I will write after this. |
|
||
export const PasswordPage = () => { | ||
const [password, setPassword] = useState(''); | ||
const [newPassword, setNewPassword] = useState(''); |
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.
Using React Hooks Form's getValues
or useWatch
seems more natural.
labelPlaceholderKey="global.form.currentpassword.label" | ||
inputPlaceholderKey="global.form.currentpassword.placeholder" |
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.
I believe the original code label={translate('global.form.newpassword.label')}
still works well. The code volume remains roughly the same, and it allows for direct text without using i18n for the label.
touchedFields={ touchedFields } | ||
errors={ errors } | ||
setValue={ setValue } | ||
nameIdCy="currentPassword" |
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.
Instead of nameIdCy
, the attribute name
feels more natural.
A PR to continue the discussion from #19560
I've made changes to the codes by @Tcharl to incorporate the following features:
Currently, all tests pass when running
npm run e2e
.Refer to the following information:
ValidatedTextInput
.