Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor: change login form to react hook form #333
refactor: change login form to react hook form #333
Changes from 1 commit
ef6641f
963eb8e
3b33c06
2adc824
fd6aece
4d74ea5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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)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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.