Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add checkbox to control whether email is bound #3133

Closed
wants to merge 4 commits into from

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jun 21, 2019

NB. This PR does not add a corresponding checkbox for binding the phone number.

Screenshot 2019-06-21 at 19 18 37

Fixes part of element-hq/element-web#10091

@dbkr dbkr requested a review from a team June 21, 2019 18:18
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also you seem to have angered every possible test

src/components/structures/auth/Registration.js Outdated Show resolved Hide resolved
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming the tests pass

{_t("Use an email address to recover your account.") + " "}
{_t("Other users can invite you to rooms using your contact details.")}
<p>{_t("Use an email address to recover your account.")}</p>
{ this.renderBindEmail() }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably only be shown if you actually fill in the e-mail box otherwise it makes no sense. (the email box has (optional) written in it)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, although then it would have to appear after you'd entered an email address, which is a bit weird. IMO people will probably get the idea if something says it'll use their email address for something an they don't enter one.

@lampholder
Copy link
Member

@nadonomy do you have thoughts as to whether the 'Let other users invite you' checkbox should only appear when the email field is filled? Personally I prefer when forms stay the same shape, so perhaps visible-but-disabled (disabled, not unchecked) would work?

Thinking too about your msisdn - if we are maintaining support in Riot for registering with a msisdn when the homeserver supports it, we should have the same sort of checkbox-to-publish-association for msisdn, too. This could either be:

  1. a checkbox + microcopy beneath each half-width field
  2. one checkbox that aggregates the decision to publish 'both or neither' threepids on registration (the copy could update depending on whether one or both of the threepid fields had been populated)
  3. two rows of checkboxes beneath the form - one for email and one for msisdn
  4. make email and msisnd full width and put each agreement-to-publish beneath its corresponding field

@nadonomy
Copy link
Contributor

nadonomy commented Jun 24, 2019

@nadonomy do you have thoughts as to whether the 'Let other users invite you' checkbox should only appear when the email field is filled? Personally I prefer when forms stay the same shape, so perhaps visible-but-disabled (disabled, not unchecked) would work?

Agree re forms not changing shape.

That withstanding I'm concerned the copy itself uses jargon (what's an identity server? a room?) at the literal first touch of creating an account, which is particularly sensitive for new users, and that we may need to split this decision off into a second screen where we can explain the value prop clearer.

Rather than working on any revisions immediately, my sense is it'd be better to bundle design/copy suggestions into the privacy related UX I'm working on tomorrow, and then look at any suggestions holistically as a part of all the changes we plan to make.

Are we OK to hold off from merging this PR until we review/iterate on privacy UX on the whole?

@jryans
Copy link
Collaborator

jryans commented Nov 25, 2019

I believe we decided to take a different path here: the email is never shared from registration, but can opt-in to sharing via Settings. I'll close this to reflect that, but we can also recover the work from history if needed.

@jryans jryans closed this Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants