-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Add GitHub profile picture from github-data.json #943
Conversation
Can you go ahead and change the css file to make it the image look more similar |
CSS to alter height and width have now been removed |
wins.html
Outdated
/* if github url is provided in wins-data and permission | ||
is granted, ghId is defined & we can use their github avatar */ | ||
|
||
let githubAvaUrl = ghId ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might be cleaner if you wrote it something like this.
let githubAvaUrl = `https://avatars1.githubusercontent.com/u/${ghId}?v=4`;
let defaultAvaUrl = "/assets/images/wins-page/avatar-default.svg"
let profileImgSrc = ghId ? githubAvaUrl : defaultAvaUrl;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we just resolve that to one line with a comment? i.e.
/* if github url is provided in wins-data and permission
is granted, ghId is defined & we can use their github avatar
otherwise, use default avatar */
let profileImgSrc = ghId ?
`https://avatars1.githubusercontent.com/u/${ghId}?v=4` :
"/assets/images/wins-page/avatar-default.svg";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think that is a great solution as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, pushed a new commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay looks good!
Fixes #925
See issue comments for screenshots and additional issues