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.

Sign up for GitHub

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

Jump to bottom

Fix leaderboard UI with fallback avatars #3814

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix leaderboard UI with fallback avatars #3814

wants to merge 5 commits into from

Conversation

vidipsingh
Copy link
Contributor

@vidipsingh vidipsingh commented Mar 4, 2025 •
edited by github-actions bot
Loading

User description

  • Updated the leaderboard UI to use a default avatar when profile images fail to load, replacing unformatted alt text and preventing layout disruptions.
  • Fixes the issue where long alt text was extending beyond the intended layout, as reported in Image Alt Text Disturbs Leaderboard UI Image Alt Text Disturbs Leaderboard UI #3812.

Fixes: #3812

PR Type

Bug fix, Enhancement

Description

  • Added fallback avatars for broken profile images.

  • Prevented layout disruptions caused by long alt text.

  • Enhanced leaderboard UI with default avatar handling.

  • Improved image error handling with onerror attribute.

Changes walkthrough 📝

Relevant files
Bug fix
leaderboard_global.html
Added fallback avatars and improved image error handling 

website/templates/leaderboard_global.html

  • Added CSS styles for fallback avatars and text.
  • Implemented onerror attribute to load default avatars.
  • Updated image handling logic to hide broken images.
  • Enhanced layout to prevent disruptions from missing images.
    		• +55/-20 
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @github-actions GitHub Actions
    Copy link
    Contributor

    github-actions bot commented Mar 4, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    3812 - Partially compliant

    Compliant requirements:

    • Replace long or improperly formatted alt text with a fallback mechanism for broken profile images.
    • Prevent layout disruptions caused by alt text when images fail to load.
    • Add a default avatar for missing or broken profile images.

    Non-compliant requirements:

    • Ensure the leaderboard UI remains visually consistent and functional.

    Requires further human verification:

    • Ensure the leaderboard UI remains visually consistent and functional.
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Issue

    The onerror attribute for fallback avatars may not handle all edge cases, such as invalid URLs or network errors. Ensure this is robust.

     
                            <div class="flex  gap-2 items-center truncate">
                            {% if leader.userprofile.avatar %}
                                <img src="{{ leader.userprofile.avatar }}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% elif leader.socialaccount_set.all.0.get_avatar_url %}
                                <img src="{{ leader.socialaccount_set.all.0.get_avatar_url }}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% else %}
                                <img src="{% gravatar_url leader.email 50 %}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% endif %}
                            <a href="{% url 'profile' slug=leader.username %}"
                               class="text-lg transition-all duration-200">{{ leader.username }}</a>
                            <span><kbd class=" py-1 px-2 select-none rounded-md {{ leader.userprofile.get_title_display|lower }} ">{{ leader.userprofile.get_title_display }}</kbd></span>
                            {% if leader.userprofile.winnings %}
                                <span class="pull-right badge px-2 py-1 select-none">${{ leader.userprofile.winnings|default:""|floatformat }}</span>
                            {% endif %}
                        </div>
                        <span class="pull-right badge bg-gray-100 flex-shrink-0 rounded-md py-1 px-2 select-none">{{ leader.total_score }} Points</span>
                    </div>
                    <div class="border-t border-gray-200 my-1"></div>
                {% endfor %}
            </div>
        {% endif %}
    </div>
</div>
<div class=" flex-1 leaderboard-section">
    <div class=" leaderboard-title">Pull Request Leaderboard</div>
    <div class=" list-group w-full p-4">
        {% if pr_leaderboard %}
            <div class="flex flex-col gap-2">
                {% for leader in pr_leaderboard %}
                    <div class="flex justify-between items-center">
                        <div class="flex  gap-2 items-center truncate">
                            {% if leader.user_profile__github_url %}
                                <img src="https://github.com/{{ leader.user_profile__user__username }}.png"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.user_profile__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% else %}
                                <img src="{% gravatar_url leader.user_profile__user__email 50 %}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.user_profile__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% endif %}
                            <a href="{% url 'profile' slug=leader.user_profile__user__username %}"
                               class="text-lg transition-all duration-200">{{ leader.user_profile__user__username }}</a>
                            <a href="{{ leader.user_profile__github_url }}"
                               target="_blank"
                               class="ml-2">
                                <i class="fab fa-github text-xl"></i>
                            </a>
                        </div>
                        <span class="pull-right badge bg-gray-100 flex-shrink-0 rounded-md py-1 px-2 select-none">{{ leader.total_prs }} PRs</span>
                    </div>
                    <div class="border-t border-gray-200 my-1"></div>
                {% endfor %}
            </div>
        {% else %}
            <p class="text-red-500 text-center font-medium">No pull request data available!</p>
        {% endif %}
    </div>
</div>
<div class=" flex-1 leaderboard-section">
    <div class="leaderboard-title">Code Review Leaderboard</div>
    <div class="list-group p-4">
        {% if code_review_leaderboard %}
            <div class="flex flex-col gap-2">
                {% for leader in code_review_leaderboard %}
                    <div class="flex justify-between items-center">
                        <div class="flex  gap-2 items-center truncate">
                            {% if leader.reviews__reviewer__user__username %}
                                <img src="https://github.com/{{ leader.reviews__reviewer__user__username }}.png"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.reviews__reviewer__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% else %}
                                <img src="{% gravatar_url leader.reviews__reviewer__user__email 50 %}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.reviews__reviewer__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% endif %}
                            <a href="{% url 'profile' slug=leader.reviews__reviewer__user__username %}"
                               class="text-lg transition-all duration-200">
                                {{ leader.reviews__reviewer__user__username }}
    Performance Concern

    The repeated inline onerror attribute for multiple image tags could lead to maintainability issues. Consider centralizing this logic.

     
                            <div class="flex  gap-2 items-center truncate">
                            {% if leader.userprofile.avatar %}
                                <img src="{{ leader.userprofile.avatar }}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% elif leader.socialaccount_set.all.0.get_avatar_url %}
                                <img src="{{ leader.socialaccount_set.all.0.get_avatar_url }}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% else %}
                                <img src="{% gravatar_url leader.email 50 %}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% endif %}
                            <a href="{% url 'profile' slug=leader.username %}"
                               class="text-lg transition-all duration-200">{{ leader.username }}</a>
                            <span><kbd class=" py-1 px-2 select-none rounded-md {{ leader.userprofile.get_title_display|lower }} ">{{ leader.userprofile.get_title_display }}</kbd></span>
                            {% if leader.userprofile.winnings %}
                                <span class="pull-right badge px-2 py-1 select-none">${{ leader.userprofile.winnings|default:""|floatformat }}</span>
                            {% endif %}
                        </div>
                        <span class="pull-right badge bg-gray-100 flex-shrink-0 rounded-md py-1 px-2 select-none">{{ leader.total_score }} Points</span>
                    </div>
                    <div class="border-t border-gray-200 my-1"></div>
                {% endfor %}
            </div>
        {% endif %}
    </div>
</div>
<div class=" flex-1 leaderboard-section">
    <div class=" leaderboard-title">Pull Request Leaderboard</div>
    <div class=" list-group w-full p-4">
        {% if pr_leaderboard %}
            <div class="flex flex-col gap-2">
                {% for leader in pr_leaderboard %}
                    <div class="flex justify-between items-center">
                        <div class="flex  gap-2 items-center truncate">
                            {% if leader.user_profile__github_url %}
                                <img src="https://github.com/{{ leader.user_profile__user__username }}.png"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.user_profile__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% else %}
                                <img src="{% gravatar_url leader.user_profile__user__email 50 %}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.user_profile__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% endif %}
                            <a href="{% url 'profile' slug=leader.user_profile__user__username %}"
                               class="text-lg transition-all duration-200">{{ leader.user_profile__user__username }}</a>
                            <a href="{{ leader.user_profile__github_url }}"
                               target="_blank"
                               class="ml-2">
                                <i class="fab fa-github text-xl"></i>
                            </a>
                        </div>
                        <span class="pull-right badge bg-gray-100 flex-shrink-0 rounded-md py-1 px-2 select-none">{{ leader.total_prs }} PRs</span>
                    </div>
                    <div class="border-t border-gray-200 my-1"></div>
                {% endfor %}
            </div>
        {% else %}
            <p class="text-red-500 text-center font-medium">No pull request data available!</p>
        {% endif %}
    </div>
</div>
<div class=" flex-1 leaderboard-section">
    <div class="leaderboard-title">Code Review Leaderboard</div>
    <div class="list-group p-4">
        {% if code_review_leaderboard %}
            <div class="flex flex-col gap-2">
                {% for leader in code_review_leaderboard %}
                    <div class="flex justify-between items-center">
                        <div class="flex  gap-2 items-center truncate">
                            {% if leader.reviews__reviewer__user__username %}
                                <img src="https://github.com/{{ leader.reviews__reviewer__user__username }}.png"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.reviews__reviewer__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% else %}
                                <img src="{% gravatar_url leader.reviews__reviewer__user__email 50 %}"
                                     class=" size-11 select-none profileimage"
                                     alt="{{ leader.reviews__reviewer__user__username }}"
                                     width="50px"
                                     height="50px"
                                     onerror="this.src='{% static "images/default-avatar.png" %}'; this.onerror=null;">
                            {% endif %}
                            <a href="{% url 'profile' slug=leader.reviews__reviewer__user__username %}"
                               class="text-lg transition-all duration-200">
                                {{ leader.reviews__reviewer__user__username }}

    @github-actions GitHub Actions
    Copy link
    Contributor

    github-actions bot commented Mar 4, 2025

    PR Code Suggestions ✨

    DonnieBLT
    DonnieBLT previously approved these changes Mar 4, 2025
    View reviewed changes
    @vidipsingh vidipsingh requested a review from DonnieBLT March 5, 2025 16:05
    @vidipsingh
    Copy link
    Contributor Author

    Hey @DonnieBLT, I think this PR is ready for merge as all the checks have passed. Let me know if anything else is needed!

    @vidipsingh vidipsingh mentioned this pull request Mar 7, 2025
    Open
    @vidipsingh vidipsingh requested a review from DonnieBLT March 10, 2025 10:40
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @DonnieBLT DonnieBLT Awaiting requested review from DonnieBLT

    Assignees
    No one assigned
    Labels
    Review effort 3/5
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    Image Alt Text Disturbs Leaderboard UI
    2 participants
    @vidipsingh @DonnieBLT