-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Add git contributors page #397
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,152 @@ | ||
import React, { useState, useEffect } from 'react'; | ||
import { Github, Loader2 } from 'lucide-react'; | ||
|
||
const Contributors = () => { | ||
const [contributors, setContributors] = useState([]); | ||
const [loading, setLoading] = useState(true); | ||
const [error, setError] = useState(null); | ||
|
||
useEffect(() => { | ||
const fetchContributors = async () => { | ||
try { | ||
setLoading(true); | ||
const response = await fetch('https://api.github.com/repos/RamakrushnaBiswal/PlayCafe/contributors'); | ||
if (!response.ok) { | ||
throw new Error('Failed to fetch contributors'); | ||
} | ||
const data = await response.json(); | ||
|
||
// Fetch additional user details for each contributor | ||
const contributorsWithDetails = await Promise.all( | ||
data.map(async (contributor) => { | ||
const userResponse = await fetch(contributor.url); | ||
const userData = await userResponse.json(); | ||
return { | ||
...contributor, | ||
name: userData.name || userData.login, | ||
bio: userData.bio, | ||
location: userData.location, | ||
company: userData.company | ||
}; | ||
}) | ||
); | ||
Comment on lines
+20
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize contributor details fetching. The current implementation makes N+1 API calls, which is inefficient and could hit rate limits quickly. Consider batching the requests or limiting the number of contributors shown: -const contributorsWithDetails = await Promise.all(
- data.map(async (contributor) => {
- const userResponse = await fetch(contributor.url);
- const userData = await userResponse.json();
- return {
- ...contributor,
- name: userData.name || userData.login,
- bio: userData.bio,
- location: userData.location,
- company: userData.company
- };
- })
-);
+// Limit to top 10 contributors to reduce API calls
+const topContributors = data.slice(0, 10);
+const contributorsWithDetails = await Promise.all(
+ topContributors.map(async (contributor) => {
+ const userResponse = await fetch(contributor.url, {
+ signal: controller.signal,
+ headers: {
+ 'Authorization': `token ${process.env.REACT_APP_GITHUB_TOKEN}`,
+ }
+ });
+ const userData = await userResponse.json();
+ return {
+ ...contributor,
+ name: userData.name || userData.login,
+ bio: userData.bio,
+ location: userData.location,
+ company: userData.company
+ };
+ })
+);
|
||
|
||
setContributors(contributorsWithDetails); | ||
setError(null); | ||
} catch (err) { | ||
setError(err.message); | ||
} finally { | ||
setLoading(false); | ||
} | ||
}; | ||
|
||
fetchContributors(); | ||
}, []); | ||
Comment on lines
+9
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add API rate limiting protection and environment configuration. Several potential issues in the data fetching logic:
Consider these improvements: useEffect(() => {
+ const controller = new AbortController();
const fetchContributors = async () => {
try {
setLoading(true);
- const response = await fetch('https://api.github.com/repos/RamakrushnaBiswal/PlayCafe/contributors');
+ const response = await fetch(`${process.env.REACT_APP_GITHUB_API_URL}/repos/${process.env.REACT_APP_REPO_OWNER}/${process.env.REACT_APP_REPO_NAME}/contributors`, {
+ signal: controller.signal,
+ headers: {
+ 'Authorization': `token ${process.env.REACT_APP_GITHUB_TOKEN}`,
+ }
+ });
// ... rest of the code
}
};
fetchContributors();
+ return () => controller.abort();
}, []); Also consider implementing a caching strategy using React Query or SWR to prevent unnecessary API calls.
|
||
|
||
if (error) { | ||
return ( | ||
<div className="min-h-screen bg-gray-50 dark:bg-gray-900 py-12 px-4"> | ||
<div className="max-w-3xl mx-auto text-center"> | ||
<div className="bg-red-50 dark:bg-red-900/20 text-red-600 dark:text-red-400 p-4 rounded-lg"> | ||
Error loading contributors: {error} | ||
</div> | ||
</div> | ||
</div> | ||
); | ||
} | ||
Comment on lines
+46
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling with specific error cases and retry mechanism. The current error handling is basic and could be more informative to users. Consider implementing more specific error handling: if (error) {
return (
<div className="min-h-screen bg-gray-50 dark:bg-gray-900 py-12 px-4">
<div className="max-w-3xl mx-auto text-center">
- <div className="bg-red-50 dark:bg-red-900/20 text-red-600 dark:text-red-400 p-4 rounded-lg">
- Error loading contributors: {error}
+ <div className="bg-red-50 dark:bg-red-900/20 text-red-600 dark:text-red-400 p-4 rounded-lg space-y-4">
+ <p>{error.includes('rate limit') ? 'GitHub API rate limit exceeded. Please try again later.' : 'Failed to load contributors.'}</p>
+ <button
+ onClick={() => {
+ setError(null);
+ fetchContributors();
+ }}
+ className="px-4 py-2 bg-red-600 text-white rounded hover:bg-red-700"
+ >
+ Retry
+ </button>
</div>
</div>
</div>
);
}
|
||
|
||
return ( | ||
<div className="min-h-screen bg-gray-50 dark:bg-gray-900 py-12 px-4"> | ||
{/* Header Section */} | ||
<div className="max-w-3xl mx-auto"> | ||
<div className="text-center mb-8"> | ||
<h1 className="text-4xl font-bold text-gray-900 dark:text-white mb-4 flex items-center justify-center gap-2"> | ||
<Github className="w-8 h-8" /> | ||
GitHub Contributors | ||
</h1> | ||
|
||
{/* Gradient Bar */} | ||
<div className="h-1 w-full bg-gradient-to-r from-red-500 via-yellow-500 via-green-500 to-blue-500 mb-6"></div> | ||
|
||
<p className="text-gray-600 dark:text-gray-300 text-lg mb-8"> | ||
Thanks to our amazing contributors who help build and improve this project! 🎉 | ||
</p> | ||
</div> | ||
|
||
{/* Loading State */} | ||
{loading ? ( | ||
<div className="flex justify-center items-center py-12"> | ||
<Loader2 className="w-8 h-8 animate-spin text-blue-500" /> | ||
</div> | ||
) : ( | ||
Comment on lines
+77
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve accessibility and loading states. While the UI is well-structured, it needs accessibility improvements and could benefit from a better loading experience. Consider these improvements: {loading ? (
<div className="flex justify-center items-center py-12">
- <Loader2 className="w-8 h-8 animate-spin text-blue-500" />
+ <Loader2
+ className="w-8 h-8 animate-spin text-blue-500"
+ aria-label="Loading contributors"
+ role="status"
+ />
+ <span className="sr-only">Loading contributors</span>
</div>
) : (
// ...
)}
<img
src={contributor.avatar_url}
alt={contributor.login}
- className="w-12 h-12 rounded-full"
+ className="w-12 h-12 rounded-full"
+ loading="lazy"
+ aria-label={`${contributor.name || contributor.login}'s avatar`}
/> Also consider implementing skeleton loading for a better user experience: const SkeletonLoader = () => (
<div className="space-y-3">
{[...Array(5)].map((_, i) => (
<div key={i} className="animate-pulse flex items-center bg-white dark:bg-gray-800 rounded-lg p-4">
<div className="w-12 h-12 bg-gray-200 dark:bg-gray-700 rounded-full" />
<div className="ml-4 space-y-2 flex-1">
<div className="h-4 bg-gray-200 dark:bg-gray-700 rounded w-1/4" />
<div className="h-4 bg-gray-200 dark:bg-gray-700 rounded w-1/2" />
</div>
</div>
))}
</div>
); Also applies to: 89-93 |
||
/* Contributors List */ | ||
<div className="space-y-3"> | ||
{contributors.map((contributor) => ( | ||
<div | ||
key={contributor.id} | ||
className="flex items-center bg-white dark:bg-gray-800 rounded-lg p-4 shadow-sm hover:shadow-md transition-shadow duration-200" | ||
> | ||
<img | ||
src={contributor.avatar_url} | ||
alt={contributor.login} | ||
className="w-12 h-12 rounded-full" | ||
/> | ||
<div className="ml-4 flex-1"> | ||
<div className="flex items-center justify-between"> | ||
<div> | ||
<a | ||
href={contributor.html_url} | ||
target="_blank" | ||
rel="noopener noreferrer" | ||
className="text-blue-600 dark:text-blue-400 hover:underline font-medium" | ||
> | ||
{contributor.name || contributor.login} | ||
</a> | ||
<p className="text-sm text-gray-500 dark:text-gray-400"> | ||
@{contributor.login} | ||
</p> | ||
</div> | ||
<span className="text-green-600 dark:text-green-400 font-semibold"> | ||
{contributor.contributions} contributions | ||
</span> | ||
</div> | ||
{(contributor.bio || contributor.location) && ( | ||
<div className="mt-2 text-sm text-gray-600 dark:text-gray-300"> | ||
{contributor.bio && <p>{contributor.bio}</p>} | ||
{contributor.location && ( | ||
<p className="text-gray-500 dark:text-gray-400 mt-1"> | ||
📍 {contributor.location} | ||
</p> | ||
Comment on lines
+115
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure proper content sanitization for user-provided data. User-provided content (bio, location) is being rendered directly, which could potentially lead to XSS attacks. Consider using a sanitization library: +import DOMPurify from 'dompurify';
-{contributor.bio && <p>{contributor.bio}</p>}
+{contributor.bio && <p>{DOMPurify.sanitize(contributor.bio)}</p>}
-{contributor.location && <p className="text-gray-500 dark:text-gray-400 mt-1">📍 {contributor.location}</p>}
+{contributor.location && <p className="text-gray-500 dark:text-gray-400 mt-1">📍 {DOMPurify.sanitize(contributor.location)}</p>} Also, ensure your Content Security Policy (CSP) is configured to allow GitHub API requests and avatar images.
|
||
)} | ||
</div> | ||
)} | ||
</div> | ||
</div> | ||
))} | ||
</div> | ||
)} | ||
|
||
{/* Repository Link */} | ||
<div className="mt-12 text-center"> | ||
<div className="text-gray-700 dark:text-gray-300 font-medium mb-4"> | ||
Explore more on our GitHub repository: | ||
</div> | ||
<a | ||
href="https://github.com/RamakrushnaBiswal/PlayCafe" | ||
target="_blank" | ||
rel="noopener noreferrer" | ||
className="inline-flex items-center justify-center gap-2 bg-gray-800 dark:bg-gray-700 text-white px-6 py-3 rounded-lg hover:bg-gray-700 dark:hover:bg-gray-600 transition-colors duration-200" | ||
> | ||
<Github className="w-5 h-5" /> | ||
Visit Repository | ||
</a> | ||
</div> | ||
|
||
{/* Bottom Gradient Bar */} | ||
<div className="h-1 w-full bg-gradient-to-r from-red-500 via-yellow-500 via-green-500 to-blue-500 mt-12"></div> | ||
</div> | ||
</div> | ||
); | ||
}; | ||
|
||
export default Contributors; |
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.
🛠️ Refactor suggestion
Consider adding PropTypes or TypeScript for better type safety.
While the component's state management is well-structured, adding type definitions would improve code maintainability and catch potential runtime errors during development.
Consider converting to TypeScript or adding PropTypes: