-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add hidden extra sign up fields #552
Conversation
* | ||
* @param key the key to store this field as. | ||
* @param value the fixed value to set for this field | ||
* @param storage the location to store this value into. |
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.
Document the supported "keys" when the storage is "root profile". (or link to docs)
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.
Resolving because this is already added here https://github.com/auth0/Lock.Android/blob/master/lib/src/main/java/com/auth0/android/lock/utils/CustomField.java#L69
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!
Changes
Previously the extra sign up fields were limited to visible fields of type: name, email, phone number and number. This PR extends the support to hidden fields that are not being displayed in the sign up form but are still sent with a fixed value to the backend as root profile or user_metadata attributes.
I'm not a fan of keeping reference to
CustomField
type constants from theHiddenField
class. Ideally these interface definition should be outside or in the parent abstract class. But doing that now would be a breaking change.Usage:
Same builder method as before now accepts subclasses of
SignUpField
class.Previously
Now
Note the method will still work accepting the
List<CustomField>
argument, but if you want to includeHiddenField
and any other future field you'd need to change the list type toList<SignUpField>
.References
Testing
This change adds unit test coverage
This change has been tested on the latest version of the platform/language or why not
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors
The correct base branch is being used