Conversation
|
Merge conflict needs resolution |
There was a problem hiding this comment.
Everything is pointed at the correct (exclusive) fields. I've made some suggestions to silence the warnings and the email field needs to be split from the username field (see specific notes). Feel free to ping me if you'd like to discuss it more offline.
| "forms": [ | ||
| { | ||
| "category": "account-creation", | ||
| "container": ["#registerform-virtual"], |
There was a problem hiding this comment.
| "container": ["#registerform-virtual"], | |
| "container": ["form#registerform-virtual"], |
| "category": "account-creation", | ||
| "container": ["#registerform-virtual"], | ||
| "fields": { | ||
| "birthdateDay": ["#gamespot-bday-day"], |
There was a problem hiding this comment.
| "birthdateDay": ["#gamespot-bday-day"], | |
| "birthdateDay": ["select#gamespot-bday-day"], |
| "container": ["#registerform-virtual"], | ||
| "fields": { | ||
| "birthdateDay": ["#gamespot-bday-day"], | ||
| "birthdateMonth": ["#gamespot-bday-month"], |
There was a problem hiding this comment.
| "birthdateMonth": ["#gamespot-bday-month"], | |
| "birthdateDay": ["select#gamespot-bday-month"], |
| "fields": { | ||
| "birthdateDay": ["#gamespot-bday-day"], | ||
| "birthdateMonth": ["#gamespot-bday-month"], | ||
| "birthdateYear": ["#gamespot-bday-year"], |
There was a problem hiding this comment.
| "birthdateYear": ["#gamespot-bday-year"], | |
| "birthdateDay": ["select#gamespot-bday-year"], |
| "username": ["#user_email_register", "#user_reg_register"] | ||
| }, | ||
| "actions": { | ||
| "submit": ["#wp-submit-register"] |
There was a problem hiding this comment.
| "submit": ["#wp-submit-register"] | |
| "submit": ["input#wp-submit-register[type='submit']"] |
| }, | ||
| { | ||
| "category": "account-login", | ||
| "container": ["#loginform-virtual"], |
There was a problem hiding this comment.
| "container": ["#loginform-virtual"], | |
| "container": ["form#loginform-virtual"], |
| "category": "account-login", | ||
| "container": ["#loginform-virtual"], | ||
| "fields": { | ||
| "password": ["#user_pass"], |
There was a problem hiding this comment.
| "password": ["#user_pass"], | |
| "password": ["input#user_pass"], |
| "container": ["#loginform-virtual"], | ||
| "fields": { | ||
| "password": ["#user_pass"], | ||
| "username": ["#user_login"] |
There was a problem hiding this comment.
| "username": ["#user_login"] | |
| "username": ["input#user_login"], |
| "username": ["#user_login"] | ||
| }, | ||
| "actions": { | ||
| "submit": ["#wp-submit"] |
There was a problem hiding this comment.
| "submit": ["#wp-submit"] | |
| "submit": ["input#wp-submit[type='submit']"] |
| "birthdateDay": ["#gamespot-bday-day"], | ||
| "birthdateMonth": ["#gamespot-bday-month"], | ||
| "birthdateYear": ["#gamespot-bday-year"], | ||
| "username": ["#user_email_register", "#user_reg_register"] |
There was a problem hiding this comment.
hrm, this is an tricky case (bear with me while I think through it):
Both can be used interchangeably for login, but also both are required. They are both "usernames" for auth concerns, but distinct values (they don't represent the same value to be entered, so they should not be represented in a shared field selector array).
Our doc guidance on Field Keys says (emphasis mine):
Field keys are constrained to the following set. Keys are grouped here for readability; the groupings carry no semantic meaning in the schema.
That is, while email is grouped under the "Contact" heading in the README, it can represent any relevant contextual concern. And even though our consumer (the Bitwarden client) treats emails as a kind of username (an association bourne out of usage), this project map does not.
Similarly, a username doesn't have to represent an auth concern; it could simply be a handle (Valve's Steam platform has this concept- you have an account username which is public-facing, but it is not the username you log in with).
For now, I think #user_email_register should be an email field; consumers should be expected to check on the form category (e.g. account-login) to understand the relevant usage context for the field.
We should probably clarify in the forms README that the username key is meant exclusively for identifiers/handles not otherwise described by other keys (for example, I'm also thinking about forms which allow registration and login via phone number, which would still be described by a phone field). Additionally, none of this is really clear with the current field groupings, and they serve only to muddy the waters. I'll work on getting some doc clarifications up around these scenarios.
| "username": ["#user_email_register", "#user_reg_register"] | |
| "username": ["input#user_reg_register"], | |
| "email": ["input#user_email_register"], |
There was a problem hiding this comment.
fwiw, I gave @dan-livefront conflicting guidance here, so you were consistent with prior art in this PR.
I still think this latest thinking is the right approach, though it may affect our consuming implementation
I've put up the updated guidance in the README in #19
🎟️ Tracking
PM-36091