-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Display validation errors on NewPassword page #471
Display validation errors on NewPassword page #471
Conversation
Codecov Report
@@ Coverage Diff @@
## master #471 +/- ##
==========================================
+ Coverage 72.33% 72.35% +0.01%
==========================================
Files 658 658
Lines 7428 7436 +8
Branches 1225 1227 +2
==========================================
+ Hits 5373 5380 +7
- Misses 1818 1819 +1
Partials 237 237
Continue to review full report at Codecov.
|
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.
LGTM except this little note above. Thanks for the contribution!
src/auth/views/NewPassword.tsx
Outdated
import { NewPasswordUrlQueryParams } from "../urls"; | ||
|
||
const NewPassword: React.FC<RouteComponentProps> = ({ location }) => { | ||
const navigate = useNavigator(); | ||
const { loginByToken } = useUser(); | ||
const [errors, setErrors] = React.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.
The state here is not needed. SetPasswordMutation
returns setPasswordOpts
, which has data
property, allowing us to access data.setPassword.errors
, which we can directly convey to NewPasswordPage
as prop. I'll look like this
<NewPasswordPage
errors={setPasswordOpts.data.?.setPassword.errors || []}
...
/>
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.
You're right. done.
Thank you ;)
I want to merge this change because it displays the validation errors on the New Password Page.
fix #468
Screenshots
Pull Request Checklist