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

Comments

@dbkr
Copy link
Member

commented Aug 9, 2019

No description provided.

@jryans jryans added phase:1 bug defect and removed p1 🔥 Fire 🔥 labels Aug 9, 2019

@jryans jryans self-assigned this Aug 12, 2019

@jryans jryans added this to In Progress in Workflow via automation Aug 12, 2019

@jryans

This comment has been minimized.

Copy link
Member

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.)

@jryans jryans removed this from In Progress in Workflow Aug 12, 2019

@jryans jryans removed their assignment Aug 12, 2019

@nadonomy

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 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.)

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

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

This comment has been minimized.

Copy link
Member

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 jryans added this to In Progress in Workflow via automation Aug 14, 2019

jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Aug 19, 2019

Hide 3PID discovery sections when no identity server
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 vector-im/riot-web#10528

jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Aug 19, 2019

Show the default IS as a placeholder in Settings
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 vector-im/riot-web#10528

@jryans jryans moved this from In Progress to In Review in Workflow Aug 19, 2019

Workflow automation moved this from In Review to In Test Aug 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.