Render avatar for Accounts #200390

isidorn · 2023-12-08T17:54:00Z

To do list:

Do the plumbing so the avatar is surfaced in the API and it passes from the Authentication session
Adopt in GitHub Authentication provider
Adopt in Microsoft Authentication provider
Figure out how to render the image in the Global Activity Bar. Will discuss with @sandy081 next week. The challenge is that right now we only render icons in the :before element, and I want to apply custom css with image.
Add menu actions and storage service writing for user preference on what avatar to render
Figure out what happens with already signed in users (ideally we can render the avatar also for them)

@TylerLeonhardt if time allows feedback is welcome. I have verified that the avatar URL nicely flows from extension to renderer. Though if you are busy I am perfectly happy if you review once I figure out the AccountsActivityActionViewItem part.
Is it ok to add an optional parameter directly in vscode.d.ts, or should I create a proposed file?

Here's how it looks (I edited CSS directly in out of source). Here's the css I plan to use:

    width: 60%;
    height: 60%;
    background-image: url(https://avatars.githubusercontent.com/u/1926584?v=4);
    background-size: cover;
    background-position: center;
    background-repeat: no-repeat;
    border-radius: 50%;

And when activity bar is at top.

TylerLeonhardt · 2023-12-09T00:35:49Z

extensions/github-authentication/src/githubServer.ts

@@ -233,7 +233,7 @@ export class GitHubServer implements IGitHubServer {
 			try {
 				const json = await result.json();
 				this._logger.info('Got account info!');
-				return { id: json.id, accountName: json.login };
+				return { id: json.id, accountName: json.login, avatar: json.avatar_url };


Does this come back when we ask for user:email scope or do we need it to have read:user?

Based on the GitHub docs: "If the authenticated user is authenticated through OAuth without the user scope, then the response lists only public profile information."
Since a users profile picture is publicly available information. I expect that we can get it just with user:email.

To be sure. I just changed from source that we do not request read:user, and indeed I got back the avatar nicely.

TylerLeonhardt · 2023-12-11T17:50:50Z

Adopt in Microsoft Authentication provider

If you want to call this out of scope for this PR, I am ok with that... that said, it's probably not a crazy amount of work on top of this item.

TylerLeonhardt · 2023-12-11T17:53:32Z

I would love it if this image is cached locally somehow so that if I get on a plane, the icon doesn't change.

isidorn · 2023-12-12T16:05:22Z

Good points. I will try to tackle MS as part of this PR.
As for caching the image - I agree we should do it. Probably independent from this PR. Thank you

TylerLeonhardt · 2023-12-12T18:03:52Z

If you don't cache the image, at the very least verify that while offline it shows something reasonable

isidorn · 2023-12-15T16:14:39Z

Tried to wire up the image for the MS account. However Copilot says that this can only be obtained through MS Graph (and browsing the internet confirms this). I remember you told me that we can not access the MS graph due to us creating our account with a type that does not have MS Graph support. Let me know if I am missing something here?

For context, here's Copilot:
"In the Microsoft Authentication API, the user's avatar image URL is not directly included in the claims. However, you can use the Microsoft Graph API to get the user's profile photo after you've obtained an access token.

Please note that you need to have the User.Read permission in your Azure AD app registration to access the user's profile photo, and the user needs to have granted this permission during the sign-in process. Here's a basic example of how you can do this in TypeScript:"

import fetch from 'node-fetch';

async function getProfilePhoto(accessToken: string) {
  const response = await fetch('https://graph.microsoft.com/v1.0/me/photo/$value', {
    headers: {
      'Authorization': `Bearer ${accessToken}`
    }
  });

  if (!response.ok) {
    throw new Error('Failed to get profile photo');
  }

  const blob = await response.blob();
  const url = URL.createObjectURL(blob);

  return url;
}

TylerLeonhardt · 2023-12-15T18:07:40Z

Yeah this gets a little tricky... it's doable, but maybe skip Microsoft auth for now.

isidorn · 2023-12-19T15:30:09Z

I think I got this into a decent state -> ready for review. But I plan to merge this in January. Here's a demo. Tomorrow I will show in Zurich and share in Slack for feedback:

Screen.Recording.2023-12-19.at.16.19.31.mov

To-do list:

Fix rendering of the avatar in the Activity Bar. Currently I am setting width, height, margin-left and margin-bottom to decrease the size of the icon. However this breaks the badge rendering. Ideally I would just change the background size, and not affect the element. But I did not figure out how to change background-size while also making sure it renders oval (border-radius: 50%).
Microsoft authentication
Verify vscode.dev and codespaces (might not be happy how I am downloading the image and passing fspath)

bpasero · 2023-12-19T16:35:44Z

💯 , should it be "Show Avatar" in the menu?

isidorn · 2023-12-19T16:51:35Z

@bpasero thanks, I like your suggested wording better - updating.

@bpasero

fyi @bpasero on potential advice

TylerLeonhardt · 2023-12-20T18:11:12Z

Some follow ups for Jan:

make sure vscode.dev is happy with where you put the icon
follow up after this PR goes in, does Codespaces' embedder already have the icon and can it be fetched already while loading the Codespace?

rasborgdk · 2023-12-20T18:12:45Z

[like] Peter Rasborg reacted to your message:

…

________________________________ From: Tyler James Leonhardt ***@***.***> Sent: Wednesday, December 20, 2023 6:11:34 PM To: microsoft/vscode ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [microsoft/vscode] Render avatar for Accounts (PR #200390) Some follow ups for Jan: * make sure vscode.dev is happy with where you put the icon * follow up after this PR goes in, does Codespaces' embedder already have the icon and can it be fetched already while loading the Codespace? — Reply to this email directly, view it on GitHub<#200390 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A6OHGTICACA6RIT5WK4AENDYKMS5NAVCNFSM6AAAAABAM7XNVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUHEYTOOJSHA>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

burkeholland · 2024-01-02T16:27:41Z

I love this so much. Is the "Show Avatar" the default?

sandy081 · 2024-01-03T09:52:18Z

src/vs/workbench/browser/parts/globalCompositeBar.ts

+
+		const avatarId = this.storageService.get(AccountsActivityActionViewItem.AVATAR_STORAGE_KEY, StorageScope.PROFILE);
+		// account.id is sometimes a number and sometimes a string so we need to convert to string
+		if (account.id.toString() === avatarId || (avatarId === undefined && providerId === 'github')) {


I would not hardcode github in code. Read it from product.json

sandy081 · 2024-01-03T09:53:50Z

src/vs/workbench/browser/parts/globalCompositeBar.ts

+	protected override updateLabel(): void {
+		super.updateLabel();
+		this.label.classList.toggle('show-avatar', !!this.accountForAvatar);
+		this.label.style.setProperty('background-image', this.accountForAvatar?.iconPath ? `url("${URI.from(this.accountForAvatar.iconPath).fsPath}")` : '');


Why is this fsPath ? Is the file stored on disk? How does this work in web version then?

sandy081 · 2024-01-03T09:56:47Z

src/vs/workbench/browser/parts/globalCompositeBar.ts

@@ -523,6 +555,16 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction
 		}
 	}

+	private updateActionItem(): void {


Should not this method has to be called when

created

profile changed

avatar state changed

vivodi · 2024-12-29T17:10:16Z

Is this still relevant?

Do the plumming for the account avatar

6590453

isidorn self-assigned this Dec 8, 2023

isidorn added this to the December / January 2024 milestone Dec 8, 2023

TylerLeonhardt reviewed Dec 9, 2023

View reviewed changes

Merge branch 'main' into isidorn/account-avatar

e260a71

isidorn added 6 commits December 18, 2023 18:37

Add avatar to readSessions in GitHubAuthenticationProvider

d280bc6

rendered avatar in activity bar account

668c3a5

Merge branch 'main' into isidorn/account-avatar

1ffdcc8

Add authenticationSessionAccountInformationAdditions proposal

6276000

properly render the avatar in the activity bar

42a0c36

fix issue when logging out that the avatar gets nicely updated

2bc9166

isidorn marked this pull request as ready for review December 19, 2023 15:22

fix typigings since github authentication is using proposed avatar api

634c735

isidorn requested review from TylerLeonhardt and sandy081 December 19, 2023 15:48

isidorn added 5 commits December 19, 2023 17:53

"use as avatar" -> "show avatar"

ebdc350

fix css when activity bar at top

6f7eb3b

rename "avatar" to "iconPath" to align with other API

e74063d

if we do not have an iconPath for a user get the user info again

48612a1

github auth - download the image, so we serve it local

ed816f4

fyi @bpasero on potential advice

sandy081 requested changes Jan 3, 2024

View reviewed changes

isidorn modified the milestones: December / January 2024, February 2024 Jan 22, 2024

isidorn modified the milestones: February 2024, March 2024 Feb 19, 2024

isidorn modified the milestones: March 2024, April 2024 Mar 27, 2024

isidorn modified the milestones: April 2024, May 2024 Apr 23, 2024

isidorn modified the milestones: May 2024, June 2024 May 8, 2024

isidorn modified the milestones: June 2024, Backlog Jun 21, 2024

Render avatar for Accounts #200390

Are you sure you want to change the base?

Render avatar for Accounts #200390

Uh oh!

Conversation

isidorn commented Dec 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TylerLeonhardt Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

isidorn Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

TylerLeonhardt commented Dec 11, 2023

Uh oh!

TylerLeonhardt commented Dec 11, 2023

Uh oh!

isidorn commented Dec 12, 2023

Uh oh!

TylerLeonhardt commented Dec 12, 2023

Uh oh!

isidorn commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TylerLeonhardt commented Dec 15, 2023

Uh oh!

isidorn commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpasero commented Dec 19, 2023

Uh oh!

isidorn commented Dec 19, 2023

Uh oh!

TylerLeonhardt commented Dec 20, 2023

Uh oh!

rasborgdk commented Dec 20, 2023 via email

Uh oh!

burkeholland commented Jan 2, 2024

Uh oh!

sandy081 Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

sandy081 Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

sandy081 Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

vivodi commented Dec 29, 2024

Uh oh!

Uh oh!

isidorn commented Dec 8, 2023 •

edited

Loading

isidorn commented Dec 15, 2023 •

edited

Loading

isidorn commented Dec 19, 2023 •

edited

Loading