refactor: change login form to react hook form #333
Conversation
gab-gil
requested review from
Samuel-Therrien-Beslogic,
theo-auffeuvre,
BesOlivierBouchard,
NicolasDontigny and
maximeBAY
as code owners
| id='email-input' | ||
| onBlur={() => validateEmail()} | ||
| onChange={event => setEmail(event.target.value)} | ||
| type='email' |
There was a problem hiding this comment.
See above comment for the id linked to the label
You should keep at the very least the name or type to email. type is even better.
This will tell screen readers (including form autocompletion browser plugins and mobile apps) that this is an email field. It'll also update the keyboard on mobile device to their email mode (where symbols like @ are shown in quick selection)
| {/* TODO: Make a component, this is extracted from the Checkbox */} | ||
| {/* component until everything uses React Hook Forms */} |
There was a problem hiding this comment.
If this is planned as the immediate follow-up PR to update the Checkbox component, I'm fine with that being done separately.
There was a problem hiding this comment.
Not immediate but I saw that the only other place we used was the SiteSummaryAction. I was planning on probably doing a refactoring of that next so that I can refactor the component. Is that ok with you?
There was a problem hiding this comment.
Yeah ok that sounds good. I just didn't want to end up with two ways of handling checkboxes for too long. Or forgetting about them.
Make sure that the code wasn't copied from elsewhere (check one):
Add a description of your changes, including visual aid and useful links