-
Notifications
You must be signed in to change notification settings - Fork 556
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 a new type of additionalSignUpField: hidden #1459
Conversation
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 questions and needs a better description here. Maybe an example of how to add a hidden field and a link to the docs? Also, it looks like you're adding value support to all fields, is that correct? If so, would be good to call that out as well.
describe('for database connection without username required', () => { | ||
const model = getModel('[email protected]', null, false); | ||
describe('for database connection without username required', () => { | ||
const model = getModel('[email protected]', null, false); |
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.
Why not [email protected]
?
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.
just copy/pasting. will change.
}); | ||
expect(modelOutJS.database.additionalSignUpFields).toEqual([ | ||
{ | ||
name: 'hidden_field', |
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.
Super-minor but this would look nicer if the values were in same order each time:
{
"type" : "hidden",
"name" : "hidden_field",
"value" : "hidden_value",
}
@@ -22,6 +22,14 @@ exports[`CustomInput when type == checkbox renders correctly as a CheckBoxInput | |||
</div> | |||
`; | |||
|
|||
exports[`CustomInput when type == hidden renders correctly as a input[type=hidden] 1`] = ` | |||
<input | |||
name="custom_input" |
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.
Same here ... order of attributes
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.
we can't control how snapshots are rendered
src/connection/database/index.js
Outdated
if (type === 'hidden' && !value) { | ||
l.warn( | ||
opts, | ||
'Ignoring an element of `additionalSignUpFields` because it has a "hidden" `type` but does not specify a `value` string' |
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 add what element is being ignored here? Saying "an element" is vague.
@@ -136,7 +136,7 @@ function processDatabaseOptions(opts) { | |||
prefill = undefined; | |||
} | |||
|
|||
const types = ['select', 'text', 'checkbox']; | |||
const types = ['select', 'text', 'checkbox', 'hidden']; |
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.
Would it be easy to add email
, url
, and password
here? Would be nice to have that built-in browser validation.
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.
that's another feature request 😬
src/connection/database/index.js
Outdated
? resolveAdditionalSignUpSelectField(r, x) | ||
: resolveAdditionalSignUpTextField(r, x); | ||
const type = x.get('type'); | ||
switch (type) { |
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.
How is the checkbox
type resolved?
src/connection/database/index.js
Outdated
return x.get('type') === 'select' | ||
? resolveAdditionalSignUpSelectField(r, x) | ||
: resolveAdditionalSignUpTextField(r, x); | ||
const type = x.get('type'); |
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.
Why assign a const
here if it's only used once?
src/connection/database/index.js
Outdated
@@ -475,3 +488,8 @@ function resolveAdditionalSignUpTextField(m, x) { | |||
|
|||
return m; | |||
} | |||
|
|||
function resolveAdditionalSignUpHiddenField(m, x) { | |||
const name = x.get('name'); |
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.
Why assign a const
here if it's only used once? Why not x.get('value')
?
I am glad that I found that this feature exist by digging through this Github repo. But this isn't documented on the Lock Configuration Page Could this be added in the |
@arvinsim PR here: auth0/docs#6933 |
fix #1324
adds a new type (hidden) of custom signup fields:
Hidden field
To specify a hidden field use:
type: "hidden"
. Both thevalue
andname
properties are required.