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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"framer-motion": "^11.5.6",
"gsap": "^3.12.5",
"js-cookie": "^3.0.5",
"lucide-react": "^0.453.0",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-icons": "^5.2.1",
Expand Down
152 changes: 152 additions & 0 deletions frontend/src/components/Contributors.jsx
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);
Comment on lines +1 to +7
Copy link
Contributor

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:

interface Contributor {
  id: number;
  login: string;
  avatar_url: string;
  html_url: string;
  contributions: number;
  name?: string;
  bio?: string;
  location?: string;
  company?: string;
}

interface ContributorsState {
  contributors: Contributor[];
  loading: boolean;
  error: string | 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
+    };
+  })
+);

Committable suggestion was skipped due to low confidence.


setContributors(contributorsWithDetails);
setError(null);
} catch (err) {
setError(err.message);
} finally {
setLoading(false);
}
};

fetchContributors();
}, []);
Comment on lines +9 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add API rate limiting protection and environment configuration.

Several potential issues in the data fetching logic:

  1. Multiple API calls could hit GitHub's rate limits
  2. Hardcoded repository URL should be configurable
  3. Missing cleanup for unmounted component
  4. No caching strategy for contributor data

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.

Committable suggestion was skipped due to low confidence.


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
Copy link
Contributor

Choose a reason for hiding this comment

The 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>
   );
 }

Committable suggestion was skipped due to low confidence.


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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Committable suggestion was skipped due to low confidence.

)}
</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;
1 change: 1 addition & 0 deletions frontend/src/components/Shared/Navbar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const Navbar = () => {
{ name: 'RESERVATION', path: '/reservation' },
{ name: 'BOARDGAMES', path: '/boardgame' },
{ name: 'MEMBERSHIP', path: '/membership' }, // Add Membership here
{ name: 'CONTRIBUTORS', path: '/contributors' },
];

useEffect(() => {
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/router/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import VerifyOtp from '../components/Pages/VerifyOtp';
import EmailVerify from '../components/Pages/EmailVerify';
import Membership from '../components/Membership';
import HelpAndSupport from '../components/Pages/HelpAndSupport';
import Contributors from '../components/Contributors';

const router = createBrowserRouter(
createRoutesFromElements(
<Route path="/" element={<App />}>
Expand All @@ -41,7 +43,7 @@ const router = createBrowserRouter(
<Route path="/email-verify" element={<EmailVerify />} />
<Route path="/membership" element={<Membership />} />
<Route path="/help" element={<HelpAndSupport />} />

<Route path="/contributors" element={<Contributors />} />
</Route>
)
);
Expand Down