Skip to content

Adding image: to user %Info#34

Merged
doomspork merged 1 commit intoueberauth:masterfrom
willykaram:master
Sep 1, 2017
Merged

Adding image: to user %Info#34
doomspork merged 1 commit intoueberauth:masterfrom
willykaram:master

Conversation

@willykaram
Copy link
Copy Markdown
Contributor

Pretty much all other Ueberauth strategies use image: for purposes of the user's avatar. So I suggest doing the same with the GitHub strategy, so as to avoid needing to customize the templating for this strategy relative to the other in order to display this avatar

Currently, when a developer is trying to display the user's avatar in a template, they need to create a tweak / custom function in order to access the github avatar with auth.info.urls.avatar_url instead of just auth.info.image.

As an example of what I mean, see these other strategies:

LINE 111 in https://github.com/ueberauth/ueberauth_facebook/blob/master/lib/ueberauth/strategy/facebook.ex

LINE 109 in https://github.com/fajarmf/ueberauth_linkedin/blob/master/lib/ueberauth/strategy/linkedin.ex

LINE 76 https://github.com/ueberauth/ueberauth_twitter/blob/master/lib/ueberauth/strategy/twitter.ex

I came across this in the course of working on an updated step-by-step guide, screencast and sample app that I'm creating to walk through implementing Ueberauth + Guardian in Phx 1.3.

Pretty much all other Ueberauth strategies use image: for purposes of the user avatar. I suggest doing this same with the GitHub strategy, so as to avoid needing to customize the templating for this strategy.

As an example see: these other strategies

LINE 111 in https://github.com/ueberauth/ueberauth_facebook/blob/master/lib/ueberauth/strategy/facebook.ex

LINE 109 in https://github.com/fajarmf/ueberauth_linkedin/blob/master/lib/ueberauth/strategy/linkedin.ex

LINE 76 https://github.com/ueberauth/ueberauth_twitter/blob/master/lib/ueberauth/strategy/twitter.ex
@doomspork doomspork self-requested a review September 1, 2017 15:53
@doomspork
Copy link
Copy Markdown
Member

Howdy @willykaram! Back from the dead! Long time no see.

Copy link
Copy Markdown
Member

@doomspork doomspork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change LGTM 👍

@doomspork doomspork merged commit 28e18a7 into ueberauth:master Sep 1, 2017
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.

2 participants