Feat: add usage of gravatar for user avatars#495
Conversation
SpeedyD
left a comment
SpeedyD
left a comment
There was a problem hiding this comment.
Looks fine except for the jpeg vs jpg file extension..
| $headers = @get_headers($url); | ||
| return $url; | ||
| if (!preg_match("|200|", $headers[0])) { | ||
| return url('assets/images/avatars/default.jpeg'); |
There was a problem hiding this comment.
...Isn't this a problem? Filename is default.jpeg, while the default file is default.jpg.
There was a problem hiding this comment.
I wanted to look into this more, and also realised that there's also return $url; on line 348, which means the if-else wouldn't be reached.
That said, url('assets/images/avatars/default.jpeg') should also be url('images/avatars/default.jpg') (inclusive of the jpeg to jpg change.
Draginraptor
left a comment
Draginraptor
left a comment
There was a problem hiding this comment.
Check over the comments on the User model's getAvatarUrlAttribute function
|
Looking through unmerged PRs, I noticed this one still hasn't been touched, and I realize something.. This is not a toggleable feature. I know of a few sites who are very fond of their default image, and would rather not have Gravatars pick it up by default. I would like to suggest that, once the other points are fixed, that it's also made to become a toggleable feature. And Newt? Put an entry on the credits page too, you deserve credit for this. |
# Conflicts: # resources/views/comments/_comment.blade.php # resources/views/comments/_perma_comments.blade.php # resources/views/galleries/submission.blade.php # resources/views/user/profile.blade.php
itinerare
added
reviewed
5 participants
No description provided.