-
-
Notifications
You must be signed in to change notification settings - Fork 103
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 #464
Conversation
@shubhagarwal1 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
WalkthroughThe changes introduce a new React component, Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Hey @RamakrushnaBiswal i have been working on this, this got delayed as I was inactive i have compelted this and also solved all the merge conflicts, kindly review my PR |
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
frontend/src/components/Contributors.jsx (1)
46-56
: Enhance error handling UI with retry functionality.While the error UI is well-styled, it could be more helpful to users by providing a retry option and more detailed error information.
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"> + <div className="bg-red-50 dark:bg-red-900/20 text-red-600 dark:text-red-400 p-6 rounded-lg"> Error loading contributors: {error} + <button + onClick={() => { + setError(null); + setLoading(true); + fetchContributors(); + }} + className="mt-4 px-4 py-2 bg-red-600 text-white rounded hover:bg-red-700 transition-colors" + > + Try Again + </button> </div> </div> </div> ); }frontend/src/components/Shared/Navbar.jsx (3)
87-91
: Consider improving template literal readability.The conditional styling logic is correct and includes proper dark mode support. However, the template literal could be more readable.
Consider this format for better readability:
- className={`w-full fixed top-0 z-50 transition duration-300 ${isScrolled - ? 'bg-background-light dark:bg-background-dark shadow-lg text-black dark:text-white' - : 'bg-transparent text-black dark:text-white' - }`} + className={` + w-full fixed top-0 z-50 transition duration-300 + ${isScrolled + ? 'bg-background-light dark:bg-background-dark shadow-lg text-black dark:text-white' + : 'bg-transparent text-black dark:text-white' + } + `}
171-182
: Add accessibility attributes to menu button SVG.While the SVG styling is correct, the menu icon button could benefit from improved accessibility.
Add aria-label to improve screen reader support:
<svg className="h-6 w-6 stroke-black dark:stroke-white" fill="none" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" + aria-label="Open menu" + role="img" >
194-195
: Improve template literal formatting for mobile menu container.The styling logic and dark mode implementation are correct, but the template literal formatting could be improved.
Consider this format for better readability:
- className={`md:hidden ${isScrolled ? 'bg-amber-100 shadow-lg' : 'bg-[#E0F0B1] shadow-lg' - } dark:bg-black `} + className={` + md:hidden + ${isScrolled ? 'bg-amber-100 shadow-lg' : 'bg-[#E0F0B1] shadow-lg'} + dark:bg-black + `}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
frontend/src/components/Contributors.jsx
(1 hunks)frontend/src/components/Shared/Navbar.jsx
(7 hunks)frontend/src/router/index.jsx
(3 hunks)
🔇 Additional comments (6)
frontend/src/router/index.jsx (3)
29-30
: LGTM! Import statements are properly structured.
The new imports follow the project's conventions and are correctly placed with other component imports.
47-52
: LGTM! Improved formatting enhances readability.
The formatting changes to the protected admin route improve code readability while maintaining the same functionality.
61-63
: Consider route organization and access control.
The new routes are properly configured, but consider:
- Route organization: Group similar routes together (e.g., informational pages like About, Help, Contributors)
- Access control: Verify if the HelpAndSupport page should be publicly accessible
Let's verify the access control requirements:
✅ Verification successful
Route organization and access control are appropriately configured
Looking at the router configuration:
- The new routes (/help and /contributors) follow the same pattern as other public informational routes like /about and /menu
- Protected routes in the application (like /admin) explicitly use the ProtectedRoute component
- The placement at the end is logical as they are part of the informational/support pages group
The current implementation aligns with the application's routing patterns where informational and support pages are publicly accessible, while administrative functions are properly protected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if HelpAndSupport component has any admin-only or protected content
rg -l "isAdmin|isAuthenticated|ProtectedRoute" "frontend/src/components/Pages/HelpAndSupport"
Length of output: 178
Script:
#!/bin/bash
# Let's first find the correct path for HelpAndSupport component
fd -t f "HelpAndSupport"
# Then check for authentication patterns in the entire frontend directory
rg -l "isAdmin|isAuthenticated|ProtectedRoute" frontend/
# Look for any route protection patterns in router configuration
rg -A 5 -B 5 "ProtectedRoute|isAuthenticated" frontend/src/router/
Length of output: 2970
Script:
#!/bin/bash
# Let's check the content of HelpAndSupport component to understand its nature
rg -p "HelpAndSupport" frontend/src/components/Pages/ -A 10 -B 10
# Also check if there are any other public routes in the router for comparison
rg -A 1 "path=\"/[^\"]+\"" frontend/src/router/index.jsx
Length of output: 1282
frontend/src/components/Contributors.jsx (1)
1-7
: LGTM! Clean setup with proper state management.
The component setup follows React best practices with appropriate imports and well-structured state management.
frontend/src/components/Shared/Navbar.jsx (2)
26-26
: LGTM: Menu item addition aligns with PR objectives.
The new "CONTRIBUTORS" menu item is correctly integrated into the navigation structure with a consistent path format.
26-26
: Verify contributors route configuration.
Let's ensure the route for the contributors page is properly configured.
✅ Verification successful
Route configuration is properly set up
The contributors route is correctly configured:
- Route
/contributors
is defined infrontend/src/router/index.jsx
Contributors
component is properly implemented and exported infrontend/src/components/Contributors.jsx
- The component is correctly imported and used in the router configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the contributors route is configured in the router
# Expected: Should find the route configuration for /contributors
# Search for route configuration
rg -l "path.*['\"]\/contributors['\"]" "src/router/"
Length of output: 161
Script:
#!/bin/bash
# Let's try a simpler pattern to search for contributors route
rg -l "/contributors" src/router/
# Also search for any route configuration files
fd "routes?.tsx?" "src/router/"
fd "routes?.jsx?" "src/router/"
# Let's also check the main router configuration
fd "index" "src/router/"
Length of output: 469
Script:
#!/bin/bash
# Let's first find the router configuration files in the frontend directory
fd "routes?.tsx?" "frontend/"
fd "routes?.jsx?" "frontend/"
fd "index" "frontend/src" -e tsx -e jsx
# Also search for contributors route pattern in frontend source
rg -l "/contributors" "frontend/src/"
Length of output: 315
Script:
#!/bin/bash
# Let's check the route configuration in index.jsx
rg -A 5 -B 5 "/contributors" "frontend/src/router/index.jsx"
# Also verify if Contributors component exists and is properly exported
cat "frontend/src/components/Contributors.jsx"
Length of output: 6151
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> | ||
) : ( | ||
/* 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> | ||
)} | ||
</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> | ||
); |
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
Optimize rendering performance and enhance accessibility.
The UI implementation is clean, but there are opportunities for improvement:
- List rendering optimization
- Image loading optimization
- Keyboard navigation
- Screen reader support
Apply these improvements:
{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"
+ role="article"
+ aria-labelledby={`contributor-${contributor.id}`}
>
<img
src={contributor.avatar_url}
+ loading="lazy"
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
+ id={`contributor-${contributor.id}`}
href={contributor.html_url}
target="_blank"
rel="noopener noreferrer"
className="text-blue-600 dark:text-blue-400 hover:underline font-medium"
+ aria-label={`View ${contributor.name || contributor.login}'s GitHub profile`}
>
{contributor.name || contributor.login}
</a>
- <p className="text-sm text-gray-500 dark:text-gray-400">
+ <p className="text-sm text-gray-500 dark:text-gray-400" aria-label="GitHub username">
@{contributor.login}
</p>
</div>
- <span className="text-green-600 dark:text-green-400 font-semibold">
+ <span className="text-green-600 dark:text-green-400 font-semibold" aria-label="Number of contributions">
{contributor.contributions} contributions
</span>
</div>
Consider adding these performance optimizations:
- Implement virtualization for large lists using
react-window
orreact-virtualized
- Add pagination or infinite scroll for better performance with large datasets
- Memoize contributor cards using
React.memo
Committable suggestion skipped: line range outside the PR's diff.
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 | ||
}; | ||
}) | ||
); | ||
|
||
setContributors(contributorsWithDetails); | ||
setError(null); | ||
} catch (err) { | ||
setError(err.message); | ||
} finally { | ||
setLoading(false); | ||
} | ||
}; | ||
|
||
fetchContributors(); | ||
}, []); |
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.
Critical: Improve API fetching strategy and error handling.
The current implementation has several issues that need to be addressed:
- Multiple API calls (one per contributor) could quickly hit GitHub's rate limits
- No error handling for individual user data fetches
- Hardcoded repository URL
- Missing cleanup for potential race conditions
Here's a suggested improvement:
useEffect(() => {
+ const controller = new AbortController();
+ const REPO_OWNER = 'RamakrushnaBiswal';
+ const REPO_NAME = 'PlayCafe';
+
const fetchContributors = async () => {
try {
setLoading(true);
- const response = await fetch('https://api.github.com/repos/RamakrushnaBiswal/PlayCafe/contributors');
+ const response = await fetch(
+ `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/contributors`,
+ { signal: controller.signal }
+ );
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) => {
+ // Batch user details requests in groups of 10
+ const contributorsWithDetails = [];
+ for (let i = 0; i < data.length; i += 10) {
+ const batch = data.slice(i, i + 10);
+ const batchResults = await Promise.allSettled(
+ batch.map(async (contributor) => {
const userResponse = await fetch(contributor.url);
+ if (!userResponse.ok) {
+ throw new Error(`Failed to fetch details for ${contributor.login}`);
+ }
const userData = await userResponse.json();
return {
...contributor,
name: userData.name || userData.login,
bio: userData.bio,
location: userData.location,
company: userData.company
};
- })
- );
+ }));
+
+ // Handle individual failures gracefully
+ batchResults.forEach((result, index) => {
+ if (result.status === 'fulfilled') {
+ contributorsWithDetails.push(result.value);
+ } else {
+ contributorsWithDetails.push({
+ ...batch[index],
+ name: batch[index].login,
+ error: 'Failed to load additional details'
+ });
+ }
+ });
+
+ // Add delay between batches to respect rate limits
+ if (i + 10 < data.length) {
+ await new Promise(resolve => setTimeout(resolve, 1000));
+ }
+ }
setContributors(contributorsWithDetails);
setError(null);
} catch (err) {
+ if (err.name === 'AbortError') return;
setError(err.message);
} finally {
setLoading(false);
}
};
fetchContributors();
+ return () => controller.abort();
}, []);
Additionally, consider:
- Using environment variables for the repository details
- Implementing proper caching to avoid repeated fetches
- Adding retry logic for failed requests
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
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 | |
}; | |
}) | |
); | |
setContributors(contributorsWithDetails); | |
setError(null); | |
} catch (err) { | |
setError(err.message); | |
} finally { | |
setLoading(false); | |
} | |
}; | |
fetchContributors(); | |
}, []); | |
useEffect(() => { | |
const controller = new AbortController(); | |
const REPO_OWNER = 'RamakrushnaBiswal'; | |
const REPO_NAME = 'PlayCafe'; | |
const fetchContributors = async () => { | |
try { | |
setLoading(true); | |
const response = await fetch( | |
`https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/contributors`, | |
{ signal: controller.signal } | |
); | |
if (!response.ok) { | |
throw new Error('Failed to fetch contributors'); | |
} | |
const data = await response.json(); | |
// Batch user details requests in groups of 10 | |
const contributorsWithDetails = []; | |
for (let i = 0; i < data.length; i += 10) { | |
const batch = data.slice(i, i + 10); | |
const batchResults = await Promise.allSettled( | |
batch.map(async (contributor) => { | |
const userResponse = await fetch(contributor.url); | |
if (!userResponse.ok) { | |
throw new Error(`Failed to fetch details for ${contributor.login}`); | |
} | |
const userData = await userResponse.json(); | |
return { | |
...contributor, | |
name: userData.name || userData.login, | |
bio: userData.bio, | |
location: userData.location, | |
company: userData.company | |
}; | |
})); | |
// Handle individual failures gracefully | |
batchResults.forEach((result, index) => { | |
if (result.status === 'fulfilled') { | |
contributorsWithDetails.push(result.value); | |
} else { | |
contributorsWithDetails.push({ | |
...batch[index], | |
name: batch[index].login, | |
error: 'Failed to load additional details' | |
}); | |
} | |
}); | |
// Add delay between batches to respect rate limits | |
if (i + 10 < data.length) { | |
await new Promise(resolve => setTimeout(resolve, 1000)); | |
} | |
} | |
setContributors(contributorsWithDetails); | |
setError(null); | |
} catch (err) { | |
if (err.name === 'AbortError') return; | |
setError(err.message); | |
} finally { | |
setLoading(false); | |
} | |
}; | |
fetchContributors(); | |
return () => controller.abort(); | |
}, []); |
f34b7ec
into
RamakrushnaBiswal:main
fix: #341
hey @RamakrushnaBiswal please review my PR
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Contributors
component that fetches and displays a list of contributors from a specified GitHub repository.Contributors
component, accessible via/contributors
.HelpAndSupport
route, accessible via/help
.Bug Fixes
Contributors
component.