microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.48k stars 28.65k forks source link

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

Open isidorn opened 2 years ago

isidorn commented 2 years ago

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:

Screenshot 2021-11-10 at 19 02 56

Screenshot 2021-11-10 at 19 04 39

TylerLeonhardt commented 2 years ago

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

Or GitHub and some 3rd party Auth Provider?

miguelsolorio commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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
isidorn commented 2 years ago

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

sana-ajani commented 2 years ago

+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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

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 commented 2 years ago

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 commented 2 years ago

some others from the team that are not so professional

...

Miguel chooses mine

insulted

😛

eamodio commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

After discussion in UX:

Based on the above I propose the following:

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

TylerLeonhardt commented 2 years ago

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

isidorn commented 2 years ago

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

TylerLeonhardt commented 2 years ago

What do you all think of @misolori's mockup here: https://github.com/microsoft/vscode/issues/97168#issuecomment-628794836

Kapture 2020-05-14 at 10 56 31

eamodio commented 2 years ago

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

isidorn commented 2 years ago

@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 commented 2 years ago

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

image

TylerLeonhardt commented 2 years ago

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

gjsjohnmurray commented 2 years ago

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

TylerLeonhardt commented 2 years ago

@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 commented 2 years ago

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

illiaNP commented 1 year ago

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.