-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Direct avatar rendering #13649
Direct avatar rendering #13649
Conversation
0d1a073
to
db9f4c2
Compare
Besides removal of dead code which I still plan to do, this should now be fully functional. |
16bfe45
to
3ab0489
Compare
Gravatar avatars are not yet tested. Not sure yet if the changes from #10540 need further adjustments, the avatar code is highly convoluted unfortunately. |
b8d0cd0
to
c5d950a
Compare
Gravatar is working as well. Initially the images were a bit blurry on my high-dpi screen and I fixed that by increasing the requested size by two times. |
Org avatars are not working yet, they still emit those redirect URLs, not sure yet where to fix them. |
94f2a58
to
9fa41a1
Compare
|
This is ready for review now. Turns out org avatars were already working and caching, I was just confused by their different URL scheme (they use I don't think there is anything left to clean up from the old avatar code, bits and pieces are still in use in some places. |
dcf2b12
to
d1b7f11
Compare
This comment has been minimized.
This comment has been minimized.
@6543 hmm I thought I fixed that earlier, will recheck. |
Actually I got around by moving everything into This is ready again, federated avatars are uncached and working just like before. Gravatar and self-hosted avatars are of course caching. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@silverwind ha, I made a mistake.
Planning to do one more small refactor later today because there is some duplication still left between |
@silverwind should we wait or start a follow up pull? |
i'll do it in the next few hours, it should be rather trivial, please wait. |
Dedupe done, ready now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
realy nice refactor!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome ! 🖼
This adds new template helpers for avatar rendering which output image elements with direct links to avatars which makes them cacheable by the browsers.
This should be a major performance improvment for pages with many avatars.