Member avatar (no photo variant) #128
Conversation
|
Awesome. I need this exact variant for |
| @@ -1,6 +1,9 @@ | |||
| import React from 'react'; | |||
| import cx from 'classnames'; | |||
| import Avatar from './Avatar'; | |||
| import { getIconAsBase64Uri } from './utils/base64'; | |||
|
|
|||
| const NO_PHOTO_SRC = getIconAsBase64Uri('profile'); | |||
There was a problem hiding this comment.
I'm interested in this choice as opposed to <img> with an SVG src? base64 strings can't be cached like other external assets, so this approach saves an http request but only in the first instance.
There was a problem hiding this comment.
Because the icon is used as in a background-image rule
There was a problem hiding this comment.
apparently it's possible to use external SVGs in background-image rules - http://stackoverflow.com/a/9193203
Are there technical blockers for that? It's not a huge deal, but I am curious
There was a problem hiding this comment.
We're rendering the icons from an SVG sprite, so there isn't a static SVG asset to point to in the background-image rule
There was a problem hiding this comment.
you're blowing my mind @mperrotti
| )); | ||
| )).add('no photo', () => { | ||
| const MOCK_MEMBER_NO_PHOTO = { ...MOCK_MEMBER }; // treat the mock as immutable | ||
| MOCK_MEMBER_NO_PHOTO.photo = {}; |
There was a problem hiding this comment.
another way to write this is const MOCK_MEMBER_NO_PHOTO = { ...MOCK_MEMBER, photo: {} };
|
@mperrotti @mmcgahan -- Any chance this could get merged this week? Member growth could really use this for an upcoming launch, potentially next week. |
| try { | ||
| return require(`base64-image!swarm-icons/dist/optimized/${name}.svg`); | ||
| } catch(e) { | ||
| return new Error(`Could not construct base64 uri for icon "${name}" \n ${e}`); |
There was a problem hiding this comment.
does this catch work in practice? since the base-64 image is created by webpack at build time, the build should fail in case of an error, but this doesn't throw the error so I think it's probably not doing what it's intended to do at runtime
There was a problem hiding this comment.
I think the try/catch was my idea, but now it's obvious that it doesn't really do anything. We should just return the stuff inside the try
| @@ -21,15 +24,16 @@ class AvatarMember extends React.Component { | |||
| { | |||
| 'avatar--org': org, | |||
| 'avatar--fbFriend': fbFriend, | |||
| 'avatar--noPhoto': (member.photo || {}).photo_link == undefined | |||
There was a problem hiding this comment.
just use 'avatar--noPhoto': !(member.photo || {}).photo_link - no need to check that it's a particular kind of falsy.
| className={classNames} | ||
| {...other} /> | ||
| {...other}/> |
|
ah, just saw that @akdetrick merged. @mperrotti could you submit a PR to fix up the |
Related issues
Fixes https://meetup.atlassian.net/browse/SDS-186
Needed for mug/mup comm work
Description
Displays a placeholder avatar for users without a photo
Screenshots (if applicable)