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

Fixed the page loading takes too much time and implemented lazy loading instead of that on statistic charts on repo details page #3925

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

Fixed the page loading takes too much time and implemented lazy loading instead of that on statistic charts on repo details page #3925

wants to merge 8 commits into from
+235 −233

Conversation

swaparup36
Copy link
Contributor

@swaparup36 swaparup36 commented Mar 11, 2025 •
edited
Loading

Untitled.video.-.Made.with.Clipchamp.mp4

It resolves #3897

I just implemented lazy loading of the statistic chart. That reduced the page's loading time. As the calculation and data gathering for the charts is time-consuming, it takes some time to load after the page is rendered as shown in the video.

@github-actions GitHub Actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The new lazy loading implementation for charts relies on JavaScript to fetch and render data dynamically. Ensure that the fetchActivityData function and the repo_activity_data endpoint handle errors gracefully, especially for cases where the API call fails or returns incomplete data.

 
    const open_closed_issue_ratio = (open_issues / closed_issues).toFixed(2);
    const open_closed_issue_ratio_elem = document.getElementById("open-closed-issue-ratio");
    open_closed_issue_ratio_elem.innerHTML = open_closed_issue_ratio;
</script>
<script>
    let issuesChart, prChart, commitsChart;

    function createCharts(data) {
        document.querySelectorAll(".loader-svg").forEach(loader => {
            loader.classList.add('hidden');
        });

        // Issues Chart
        const issuesCtx = document.getElementById('issuesChart').getContext('2d');
        if (issuesChart) issuesChart.destroy();
        issuesChart = new Chart(issuesCtx, {
            type: 'bar',
            data: {
                labels: data.issues_labels,
                datasets: [{
                    label: 'Opened',
                    data: data.issues_opened,
                    backgroundColor: 'rgba(59, 130, 246, 0.5)',
                    borderColor: 'rgba(59, 130, 246, 1)',
                    borderWidth: 1
                }, {
                    label: 'Closed',
                    data: data.issues_closed,
                    backgroundColor: 'rgba(29, 78, 216, 0.5)',
                    borderColor: 'rgba(29, 78, 216, 1)',
                    borderWidth: 1
                }]
            },
            options: {
                scales: {
                    x: {
                        display: false
                    }
                }
            }
        });

        // PR Chart
        const prCtx = document.getElementById('prChart').getContext('2d');
        if (prChart) prChart.destroy();
        prChart = new Chart(prCtx, {
            type: 'bar',
            data: {
                labels: data.pr_labels,
                datasets: [{
                    label: 'Opened',
                    data: data.pr_opened_data,
                    backgroundColor: 'rgba(192, 38, 211, 0.5)',
                    borderColor: 'rgba(192, 38, 211, 1)',
                    borderWidth: 1
                }, {
                    label: 'Closed',
                    data: data.pr_closed_data,
                    backgroundColor: 'rgba(109, 40, 217, 0.5)',
                    borderColor: 'rgba(109, 40, 217, 1)',
                    borderWidth: 1
                }]
            },
            options: {
                scales: {
                    x: {
                        display: false
                    }
                }
            }
        });

        // Commits Chart
        const commitsCtx = document.getElementById('commitsChart').getContext('2d');
        if (commitsChart) commitsChart.destroy();
        commitsChart = new Chart(commitsCtx, {
            type: 'bar',
            data: {
                labels: data.commits_labels,
                datasets: [{
                    label: 'Pushes',
                    data: data.pushes_data,
                    backgroundColor: 'rgba(234, 88, 12, 0.5)',
                    borderColor: 'rgba(234, 88, 12, 1)',
                    borderWidth: 1
                }, {
                    label: 'Commits',
                    data: data.commits_data,
                    backgroundColor: 'rgba(194, 65, 12, 0.5)',
                    borderColor: 'rgba(194, 65, 12, 1)',
                    borderWidth: 1
                }]
            },
            options: {
                scales: {
                    x: {
                        display: false
                    }
                }
            }
        });

        const openClosedRatio = document.getElementById('open-closed-issue-ratio');
        if (openClosedRatio) {
            openClosedRatio.textContent = (data.open_issues / data.closed_issues).toFixed(2);
        }

        updateTrendIndicator('issue_change_ratio_numbers', data.issue_ratio_change, data.issue_ratio_percentage_change);
        updateTrendIndicator('pr_change_numbers', data.pr_change, data.pr_percentage_change);
        updateTrendIndicator('commit_change_numbers', data.commit_change, data.commit_percentage_change);
    }

    function updateTrendIndicator(elementId, change, percentage_change) {
        const element = document.getElementById(elementId);
        if (element) {
            let arrowSvg = '';

            if(change >= 0) {
                arrowSvg = `
                    <svg class="w-4 h-4 mr-1" fill="currentColor" viewBox="0 0 20 20">
                        <path fill-rule="evenodd" d="M3.293 9.707a1 1 0 010-1.414l6-6a1 1 0 011.414 0l6 6a1 1 0 01-1.414 1.414L11 5.414V17a1 1 0 11-2 0V5.414L4.707 9.707a1 1 0 01-1.414 0z" clip-rule="evenodd" />
                    </svg>
                `
            } else {
                arrowSvg = `
                    <svg class="w-4 h-4 mr-1" fill="currentColor" viewBox="0 0 20 20">
                        <path fill-rule="evenodd" d="M16.707 10.293a1 1 0 010 1.414l-6 6a1 1 0 01-1.414 0l-6-6a1 1 0 111.414-1.414L9 14.586V3a1 1 0 012 0v11.586l4.293-4.293a1 1 0 011.414 0z" clip-rule="evenodd" />
                    </svg>
                `
            }

            const reqClass = change >= 0 ? 'text-green-500 flex items-center' : 'text-red-500 flex items-center';
            element.innerHTML = `
                <p class="text-gray-400">${Math.abs(change).toFixed(2)} (${Math.abs(percentage_change).toFixed(2)}%)</p>
                <p class="${reqClass}">
                    ${arrowSvg} past month
                </p>
            `;
        }
    }

    function fetchActivityData() {
        fetch("{% url 'repo_activity_data' repo.slug %}")
            .then(response => response.json())
            .then(data => {
                createCharts(data);
            })
            .catch(error => console.error('Error fetching activity data:', error));
    }

    document.addEventListener('DOMContentLoaded', fetchActivityData);
</script>
Performance Concern

The repo_activity_data function fetches data for repository statistics. Verify that this function is optimized for performance and does not introduce significant delays or load on the server, especially when handling multiple concurrent requests.

 
def repo_activity_data(request, slug):
    """API endpoint for repository activity data"""
    repo = get_object_or_404(Repo, slug=slug)
    owner_repo = repo.repo_url.rstrip("/").split("github.com/")[-1]
    owner, repo_name = owner_repo.split('/')

    # Get activity data
    activity_data = RepoDetailView().fetch_activity_data(owner, repo_name)

    return JsonResponse(activity_data)
Accessibility Issue

The new charts and loading indicators rely heavily on visual elements. Ensure that these elements are accessible to users with disabilities, such as screen reader users, and provide appropriate ARIA labels or alternative text.

 
                </div>
            </div>
        </div>
    </div>
</section>
<!-- Statistics Charts -->
<section class="bg-white py-6 px-4 rounded-lg shadow-lg">
    <h2 class="text-pink-500 font-bold text-lg">{{ repo.contributor_count|intcomma }} Contributions in the Last 30 Days</h2>
    <div class="grid grid-cols-3 gap-2 mt-4">
        <div class="p-4 border rounded-lg">
            <span class="text-blue-500 text-sm flex w-full items-center justify-start">
                <p class="mr-2" id="open-closed-issue-ratio"></p>
                <p>Opened/Closed Issue Ratio</p>
            </span>
            <span class="text-xs flex justify-between items-center mt-3" id="issue_change_ratio_numbers">Loading..</span>
        </div>
        <div class="p-4 border rounded-lg">
            <span class="text-purple-500 text-sm flex w-full items-center justify-start">
                <p class="mr-2">{{ repo.open_pull_requests|intcomma }}</p>
                <p>Pull Requests Opened</p>
            </span>
            <span class="text-xs flex justify-between items-center mt-3" id="pr_change_numbers">Loading..</span>
        </div>
        <div class="p-4 border rounded-lg">
            <span class="text-blue-500 text-sm flex w-full items-center justify-start">
                <p class="mr-2">{{ repo.commit_count|intcomma }}</p>
                <p>Commits</p>
            </span>
            <span class="text-xs flex justify-between items-center mt-3" id="commit_change_numbers">Loading..</span>
        </div>
    </div>  
    <div class="grid grid-cols-3 gap-4 mt-6">
        <div class="p-4 border rounded-lg">
            <div class="flex items-center justify-start">
                <span class="p-2 rounded-full mr-2">
                    <svg class="w-5 h-5 text-blue-500"
                        fill="none"
                        stroke="currentColor"
                        viewBox="0 0 24 24">
                        <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 8v4m0 4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z" />
                    </svg>
                </span>
                <p class="text-blue-500">Issues</p>
            </div>
            <span class="loader-svg ml-4 flex items-center text-gray-400">
                <svg class="animate-spin h-4 w-4 mr-2" viewBox="0 0 24 24">
                    <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" fill="none"></circle>
                    <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path>
                </svg>
                <span>Loading...</span>
            </span>
            <canvas id="issuesChart"></canvas>
        </div>
        <div class="p-4 border rounded-lg">
            <div class="flex items-center justify-start">
                <span class="p-2 rounded-full">
                    <svg class="w-5 h-5 text-purple-500"
                         fill="none"
                         stroke="currentColor"
                         viewBox="0 0 24 24">
                        <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M8 7h12m0 0l-4-4m4 4l-4 4m0 6H4m0 0l4 4m-4-4l4-4" />
                    </svg>
                </span>
                <p class="text-purple-500">Pull Requests</p>
            </div>
            <span class="loader-svg ml-4 flex items-center text-gray-400">
                <svg class="animate-spin h-4 w-4 mr-2" viewBox="0 0 24 24">
                    <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" fill="none"></circle>
                    <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path>
                </svg>
                <span>Loading...</span>
            </span>
            <canvas id="prChart"></canvas>
        </div>
        <div class="p-4 border rounded-lg">
            <div class="flex items-center justify-start">
                <span class="p-2 rounded-full">
                    <svg class="w-5 h-5 text-orange-500"
                         fill="none"
                         stroke="currentColor"
                         viewBox="0 0 24 24">
                        <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z" />
                    </svg>
                </span>
                <p class="text-orange-500">Pushes & Commits</p>
            </div>
            <span class="loader-svg ml-4 flex items-center text-gray-400">
                <svg class="animate-spin h-4 w-4 mr-2" viewBox="0 0 24 24">
                    <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" fill="none"></circle>
                    <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path>
                </svg>
                <span>Loading...</span>
            </span>
            <canvas id="commitsChart"></canvas>
        </div>
    </div>
</section>
<!-- Community Section -->

@github-actions GitHub Actions
Copy link
Contributor

PR Code Suggestions ✨

@CodeWithBishal
Copy link
Contributor

@swaparup36

  • What actions could be taken to reduce the page loading time
  • re-commit with pre-commit

@swaparup36
Copy link
Contributor Author

@CodeWithBishal

As we need to get very detailed data of 2 months for making the chart, so we have to call the API of GitHub again and again to get all the required data. So, a possible solution could be caching these data into redis, it might help the page to load faster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Review effort 4/5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add statistic charts to the repo detail page
2 participants
@swaparup36 @CodeWithBishal