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

Once user GitHub authenticated render the avatar in the activity bar #136889

Open
isidorn opened this issue Nov 10, 2021 · 27 comments · May be fixed by #200390
Open

Once user GitHub authenticated render the avatar in the activity bar #136889

isidorn opened this issue Nov 10, 2021 · 27 comments · May be fixed by #200390
Assignees
Labels
authentication Issues with the Authentication platform feature-request Request for new features or functionality
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Nov 10, 2021

Once a user logins to GitHub from VS Code we could render a nice GitHub avatar instead of the soulless authentication icon in the activity bar. I personally like how Chrome and GitHub do this.

Reasons why we should do it:

  • Gives confirmation to the user that she successfully logged in. Without this the experience feels half baked.
  • Makes VS Code feel more personalised
  • It is possible to hide it via context menu for users that dislike seeing their avatar

Screenshot 2021-11-10 at 19 02 56

Screenshot 2021-11-10 at 19 04 39

@isidorn isidorn added feature-request Request for new features or functionality authentication Issues with the Authentication platform labels Nov 10, 2021
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Nov 10, 2021

What would happen if they are logged in to GitHub and Microsoft accounts?

Or GitHub and some 3rd party Auth Provider?

@miguelsolorio
Copy link
Contributor

We did previously discussed this, I personally like it, but people on the team didn't want to look at their face all the time 🤷‍♂️ we could always add a setting to turn this off. We could also default to one of the login providers and allow a setting to set it as well.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 10, 2021

If there are multiple accounts I would say the first one or last one wins (we do the same in API in some places).
Not looking at your face the whole time -> it just has to be done with style like Chrome and GitHub. Nobody complains about those.
I think the biggest challenge is that Activity Bar icons are large and this should not be so large to not be distracting.

@miguelsolorio
Copy link
Contributor

Oh I agree with you, don't get me wrong, this is just the feedback we saw in the past. FWIW, this is what it would look like:

CleanShot 2021-11-10 at 12 48 02@2x

@TylerLeonhardt TylerLeonhardt added this to the Backlog milestone Nov 11, 2021
@isidorn
Copy link
Contributor Author

isidorn commented Nov 11, 2021

I actually think it is pretty neat. Let me ping people on the UX channel so we hear potential negative feedback.

@sana-ajani
Copy link
Contributor

+1. Also, could give more incentive to users for logging in and syncing their settings because it gives more of a complete "profile" feel too. If I see my friend's VS Code with their picture icon, I'd be likely to ask why and do it myself to get the personal feel.

@TylerLeonhardt
Copy link
Member

I would imagine we would need an API for this - something on an Auth Provider that is called:

export interface AuthenticationProvider {
    getAvatar(): Thenable<Uri>;
}

but I think we gotta be careful with that because we don't want an auth provider to simply return the url of the avatar on the internet thus having the account menu depend on the internet... ideally they download the image locally and serve it from there.

Maybe the API could return the image in a raw form but I don't think our API has done this before...

I like the idea from a personal feel... but I don't like it if they're logged in to multiple accounts.

cc @eamodio

@rebornix
Copy link
Member

If users have two accounts, I would prefer to show both of them, one on top of another with some offset, we can always show the last one at the very top. And when an extension requests token for one account, show the account avatar at the very top. Otherwise it might look weird that I'm seeing my github avatar with 1 notification, but after clicking on it it says extensions asking for microsoft token.

With that being said, my 2 cents on if we should show it is Browsers/Websites account systems are different from ours. In browser, or github.com, you authenticate with one single primary account, and there is only one active account per instance. However in VS Code, there is no such primary account concept. I can use ms for setting sync and live share but github account for all github features. It feels weird that we promote one account over the others.

@lramos15
Copy link
Member

I understand why people may want this, but I would definitely be someone who turns this off. To me it works on webpages because of there being different types of UI elements on the same axis as the user profile. In the activity bar it's all icons that are theme-reactive. This means my face would look awfully out of place in the UI. I also agree that multiple accounts pose a weird challenge that most sites don't have.

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Nov 11, 2021

The main point of the avatar is to indicate that "you are signed in", it builds confidence in that it's working. When it's not working, your avatar would be gone and you'd see the badge that says you are signed out. Those indicators are clear.

In terms of which profile to show vs showing both, because there are many different preferences on this topic this should be user configurable. As someone who has the same profile picture everywhere, I wouldn't want to see my face twice.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 12, 2021

Great discussion, thanks for feedback.
Multi account: I agree with what has already been said. Here's a proposed solution:

  • We do image diff and if we see that there are multiple different images we add a new entry "Use as Avatar" above the "Sign Out" in the submenu of each provider.
  • Use the first that signed in, until the user explicitly chooses the one she wants

Screenshot 2021-11-12 at 12 16 05

As for API, I agree with @TylerLeonhardt that this should not depend on the internet ideally. I do not know if returning the raw image would be in the spirit of our API.

@misolori your avatar is actually good quality and has shades of dark that go well with the dark theme. Can you do some mockups of the default GitHub avatar and some others from the team that are not so professional so we also get a feeling how the worse case scenario would look. Thanks!

@miguelsolorio
Copy link
Contributor

Here's examples with various profile photos on different themes. I think we should add a 1px border w/ a slight opacity (4%) to handle the cases where the avatar blends in with the activity bar bg. The mockups below each have one:

CleanShot 2021-11-15 at 09 22 58@2x

CleanShot 2021-11-15 at 09 22 43@2x

@TylerLeonhardt
Copy link
Member

some others from the team that are not so professional

...

Miguel chooses mine

insulted

😛

@eamodio
Copy link
Contributor

eamodio commented Nov 16, 2021

While I like the idea of having avatars (even with an option of showing it on the bar) -- I'd be a little concerned because in these other sites/apps, Chrome, Slack, GitHub, you only log in with a single user account, but in VS Code the account system isn't [just] for logging VS Code into an account, it's an account management tool/UI for both VS Code and extensions to leverage (and consolidate).

Maybe it can be split up, so that settings sync (and whatever else gets associated with a "VS Code" account, e.g. profiles?) gets elevated to a top-level login -- that would give you the single login with the avatar etc. And then move the additional accounts into a new place.

Personally, I would love to see the accounts UI move away from a menu and instead have a tailored custom fly-out to see and manage accounts (with avatars) and their usages more easily, e.g. an account overview UI. With that you could still have a combined icon (with a single "login" for vs code) and then have distinct sections for the vs code login vs other accounts.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 16, 2021

@misolori ok looks cool for all avatars, great work. We bring this up in the UX meeting?

@eamodio makes sense. The fly-out menu would be cool, until we have that I think my proposal from above might work just fine.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 17, 2021

After discussion in UX:

  • We should use a different codicon when a user is logged in
  • We need to make showing pictures configurable, since hiding the activity bar item is too nuclear
  • There might be more corner cases (like not having internet connection, some authentication not being valid) - what should we do then
  • Showing an avatar would give confirmation to the user that they are logged in
  • @devinvalenciano verified that only 0.6% of Authenticated users are authenticated to 2+ accounts. Thus I think the naive solution I proposed would work since there are not that many users that are 2+ authenticated.

Based on the above I propose the following:

  • We "hack" this up in Insiders so we can really try it out for a week and see if we hate it internally as a team
  • If people actually turn out to like it then:
    • Let's design the API that makes sense
    • Let's add a configuration that would make this configurable
  • If people do not like it:
    • Let's try to design a codicon that would show logged in state

@TylerLeonhardt what do you think? Did I miss something?

@TylerLeonhardt
Copy link
Member

Yes I think this is a fine plan for when we want to explore this. Maybe we can try in December.

@TylerLeonhardt TylerLeonhardt modified the milestones: Backlog, December 2021 Nov 17, 2021
@isidorn
Copy link
Contributor Author

isidorn commented Nov 18, 2021

Sounds great. Let me also assign myself and let me know the best way I can help. Though let's talk in December when we find time...

@isidorn isidorn self-assigned this Nov 18, 2021
@TylerLeonhardt TylerLeonhardt modified the milestones: January 2022, Backlog Jan 13, 2022
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Feb 4, 2022

What do you all think of @misolori's mockup here:
#97168 (comment)

Kapture 2020-05-14 at 10 56 31

@eamodio
Copy link
Contributor

eamodio commented Feb 4, 2022

Oooh. I really like it except for having to hit the x to close it.

@isidorn
Copy link
Contributor Author

isidorn commented Feb 4, 2022

@TylerLeonhardt looks great! I agree with @eamodio The whole Account and the X part can be dropped. I think it is pretty obvious without it and would make it leaner.
We still plan to render the Avatar in the activity bar but the mock just does not cover it, right?

@gjsjohnmurray
Copy link
Contributor

When working in this area please cater for authentication providers with which multiple logins may exist concurrently, e.g.

image

@TylerLeonhardt
Copy link
Member

@gjsjohnmurray I'm curious how this is implemented. Is it a single AuthProvider that supports multiple accounts?

@gjsjohnmurray
Copy link
Contributor

@TylerLeonhardt yes, a single provider. Currently on the prerelease branch here.

@TylerLeonhardt
Copy link
Member

@gjsjohnmurray that's very cool. So each server has potentially different credentials so you need to log in to each individual one? How do you use this AuthProvider? How do you differentiate between sessions?

@gjsjohnmurray
Copy link
Contributor

scopes[0] is the server name, scopes[1] is the user name. The session accessToken holds the password.

@illiaNP
Copy link

illiaNP commented Dec 12, 2022

Is it possible to see it instead of the Accounts icon?
For example, let's see our Github avatar instead of the Accounts icon in our VS Code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication Issues with the Authentication platform feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants