Skip to content
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

don't try & show threepids in discovery section if we don't have an ID server #10528

Closed
dbkr opened this issue Aug 9, 2019 · 5 comments · Fixed by matrix-org/matrix-react-sdk#3322
Assignees

Comments

@dbkr
Copy link
Member

dbkr commented Aug 9, 2019

No description provided.

@jryans
Copy link
Collaborator

jryans commented Aug 12, 2019

@nadonomy, when the user has no identity server, should the Discovery section disappear entirely, or should it be present but with some kind of "⚠️ You need to select an Identity Server before you can share your email addresses and phone numbers."?

(This is similar but not the same as #10522.)

@nadonomy
Copy link
Contributor

@nadonomy, when the user has no identity server, should the Discovery section disappear entirely, or should it be present but with some kind of "⚠️ You need to select an Identity Server before you can share your email addresses and phone numbers."?

(This is similar but not the same as #10522.)

Is there any reason to not expose the same state as comped here? #10522 (comment)

I'm working from the assumption that the client should always have a default identity server specified, if it doesn't, then we could tweak the copy accordingly. As per the other comment, feedback is welcome/I'd be very glad to rubber duck through the copy and minutiae on this.

@jryans
Copy link
Collaborator

jryans commented Aug 14, 2019

Hmm, I guess it's not entirely clear to me how the Discovery section should behave when the user currently has no Identity Server...

Over in a related privacy issue, @lampholder said:

We should no longer have an IS 'supplied at login time' - rather there is a 'default' that is 'the Identity Server the client would prompt to use if asked to do something identity servery and the user has not made an active choice to use either a different identity server or no identity servrer at all'.

...but it's not immediately clear to me how to interpret that for the Discovery section. @lampholder, does it mean the Discovery section always has some IS set: either the one you are actively using, or else the default IS?

@lampholder
Copy link
Member

With IS disabled, my Discovery section looks like:

image

Would it not just be appropriate to hide the Email addresses and Phone numbers sections if no IS is connected?

I think the text under Identity Server is good, but it is a little confusing that the Identity Server text box appears to have defaulted to something - might it be clearer if this was either proper placeholder text (such as e.g. vector.im) or nothing at all?

@jryans
Copy link
Collaborator

jryans commented Aug 14, 2019

Would it not just be appropriate to hide the Email addresses and Phone numbers sections if no IS is connected?

Yes, I think that could work.

I think the text under Identity Server is good, but it is a little confusing that the Identity Server text box appears to have defaulted to something - might it be clearer if this was either proper placeholder text (such as e.g. vector.im) or nothing at all?

Yes, I agree the current state of that text box is confusing, and maybe proper placeholder text is a good route to go so people have some clue of what would work there.

@jryans jryans self-assigned this Aug 14, 2019
jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Aug 19, 2019
This hides the email and phone sections of Discovery in the Settings when there
is no IS, as they can't meaningfully be used.

Part of element-hq/element-web#10528
jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Aug 19, 2019
This changes the UX for the set IS field to show the default IS as a placeholder
value (as opposed to an initial value as if the user had actually entered it).

Fixes element-hq/element-web#10528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants