Skip to content
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

Avatar Tweaks #438

Merged
merged 3 commits into from
Sep 27, 2014
Merged

Avatar Tweaks #438

merged 3 commits into from
Sep 27, 2014

Conversation

bengott
Copy link
Member

@bengott bengott commented Sep 27, 2014

Use larger avatar images for Twitter and Facebook (~200x200 instead of ~50x50)

  • Fixes blurry avatar on user profile page
  • File sizes only slightly larger (a few kB), so just use them everywhere
  • Also renamed @shaialon's recently added "gravatar" template/view to "avatar" (because it's for all avatars, not just gravatar)

@shaialon
Copy link

Well done @bengott, thanks for the catch

SachaG added a commit that referenced this pull request Sep 27, 2014
@SachaG SachaG merged commit 102077a into VulcanJS:master Sep 27, 2014
@SachaG
Copy link
Contributor

SachaG commented Sep 27, 2014

Nice to see this avatar stuff coming together :)

@bengott
Copy link
Member Author

bengott commented Sep 28, 2014

image

@bengott
Copy link
Member Author

bengott commented Sep 28, 2014

I also thought about moving the logic from the getAvatarUrl() function directly into the {{> avatar}} template, but I'm assuming getAvatarUrl() and all those other functions in /lib/users.js are there for a reason (namespacing? accessibility from both client & server?). I found only two other references to getAvatarUrl(). One is in /client/views/comments/comment_item.js and is commented out, and the other is in /client/views/posts/modules/post_content.js, but it doesn't look like it's being used (deprecated?). Anyway, I suppose this logic will eventually get exported when we break out all this avatar stuff into its own package, eh?

@SachaG
Copy link
Contributor

SachaG commented Sep 28, 2014

You're right, but that's a good point. Even if we're not using it on the server right now, we do want to make that possible. For example, we might want to include avatars in an email template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants