-
Notifications
You must be signed in to change notification settings - Fork 253
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
profile: show warning and option to reactivate when contact method is disabled #28
Conversation
- using a string as input as this allows us to expand past digits in the future if we want to, as well as the fact that in a browser a user can still type "e" in as a valid number in a type="number" textfield
Tests passed. |
…ct-method-warning-disabled
…d use within the opened transaction
Comments addressed. Outstanding work is to create a new migration for the
|
update mutation variable on UI and update relevant store functions
…ct-method-warning-disabled
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.
A few things I noticed by functionally testing the PR:
- Should we add a validation for checking if code entered on UI only consists of numbers and of length 6? Right now, if we enter a string, it goes all the way to the backend and returns unexpected error. It would be better if we avoided it going to the backend and masked the error with some helpful text too.
This is what we see currently:
It would be nice to instead see a validation error similar to
-
One nice-to-have validation we could maybe add is to disable the
Submit
button on the dialog unless the user has hit theSend code
button at least once. What do you think? -
If a user has 2 contact methods, clicking on
Verify/Reactivate
for the first contact method shows the dialog for actually the second one.
@arurao Looks like I forgot to change the type of the textfield to |
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.
Can you merge latest from master into this? Looks like there's a conflict.
# Conflicts: # web/src/app/users/UserContactMethodList.js
make check
to catch common errors. Fixed any that came up.Description:
This PR adds a warning to a contact method if it is currently disabled. The warning icon should have a tooltip directing the user on how to reactivate said contact method.
Additional Info:
This PR also refactors the verification form/dialog to use React hooks, as well as updates the verification mutations to use our latest graphql library.