Issue 2421 post users avatar #2430
Conversation
|
|
| }) | ||
| ); | ||
|
|
||
| const getUserImage = (user: string) => { |
There was a problem hiding this comment.
Two things:
- If your function returns the value of an expression, you don't need
{ return ... }and can simply do=> expr:
const getUserImage = (user: string) => `https://github.com/${user}.png`- Add an argument here to accept a size value, and include it in the URL so we don't load an image larger than we need:
https://github.com/${user}/png?size={size}. You can default it to25.
const getUserImage = (user: string, size: number = 25) => `https://github.com/${user}.png?size={size}`There was a problem hiding this comment.
I played around with this a bit.
I wanted to make the code can be reused elsewhere, where the size may change. I made the avatarsize prop for the whole component, so we will pull the image from github in that size, but also pass that same size to the Telescope avatar. the prop avatarsize is optional, but the default will be 25.
| <li key={user} className={classes.user}> | ||
| <div> | ||
| <TelescopeAvatar | ||
| img={getUserImage(user)} |
There was a problem hiding this comment.
Let's pass in a name here of user
| @@ -2,7 +2,7 @@ import { MouseEventHandler } from 'react'; | |||
| import { Avatar } from '@material-ui/core'; | |||
|
|
|||
| type TelescopeAvatarProps = { | |||
| name: string; | |||
| name?: string; | |||
There was a problem hiding this comment.
Let's not do these changes to the TelescopeAvatar
|
Here's what it looks like, for other reviewers:
|
|
@AmasiaNalbandian don't merge master into your branch, rebase onto master. See workflow docs https://github.com/Seneca-CDOT/telescope/blob/master/docs/git-workflow.md |
package.json
Outdated
| @@ -52,7 +52,7 @@ | |||
| "homepage": "https://github.com/Seneca-CDOT/telescope#readme", | |||
| "dependencies": { | |||
| "@bull-board/api": "3.6.0", | |||
| "@bull-board/express": "3.6.0", | |||
| "@bull-board/express": "3.7.0", | |||
There was a problem hiding this comment.
You've got changes in here that don't belong. Please rebase and squash your branch on master:
git checkout upstream
git pull upstream master
git checkout issue-2421-post-users-avatar
git rebase -i master
# fix conflicts if any and finish rebase...
git push origin -f issue-2421-post-users-avatar
There was a problem hiding this comment.
checkout upstream was not working... but i believe it's gone now.
|
@AmasiaNalbandian you have 26 commits here, it should be 1. Please squash. |
AmasiaNalbandian
force-pushed
the
issue-2421-post-users-avatar
branch
from
cc50a20 to
58ecf7f
Compare
AmasiaNalbandian
force-pushed
the
issue-2421-post-users-avatar
branch
from
58ecf7f to
c50ff50
Compare
|
The staging server is down, I can't test this right now. |
|
This is nice. In a follow-up, it'd be nice to add displaying a user name when you hover over a user avatar (right now it's hard to know what you're clicking on) |
|
Filed #2442 on the hover issue. Can I get another approval so we can merge this tonight? |
|
@AmasiaNalbandian can you rebase this? |
AmasiaNalbandian
force-pushed
the
issue-2421-post-users-avatar
branch
from
c50ff50 to
8d22b74
Compare
Issue This PR Addresses
Fixes #2421
Type of Change
Description
This PR adds a new feature which creates avatars of all users, repositories, and organizations involved in a blog post. The avatars are interactable(click event) and will open the users page in a new tab.
Below are some screenshots:
Dark Mode:
Light Mode:
Checklist