-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Website: Add a toggle feature to sort open-source contributors #1006
base: main
Are you sure you want to change the base?
Conversation
…to sort the list of contributors by the number of commits or the lastest commits
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces enhancements to the open-source contributors section of the website. The changes include updating the Changes
Sequence DiagramsequenceDiagram
participant Fetch as Commit Data Fetcher
participant Process as Contributor Processor
participant Client as Contributors Client
Fetch->>Process: Retrieve GitHub Commits
Process->>Process: Sort Contributors
Process->>Client: Pass Sorted Contributors
Client->>Client: Render with Toggle Group
Possibly related PRs
Suggested labels
Suggested reviewers
Finishing Touches
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 (
|
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: 0
🧹 Nitpick comments (2)
website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx (1)
44-55
: Consider optimizing commit date processingThe current implementation iterates through all commits. Consider using reduce for better performance:
-const latestCommitDates = new Map<number, Date>(); -totalCommitsData.forEach((commit: GitHubCommit) => { - if (commit.author?.id && commit.commit?.author?.date) { - const contributorId = commit.author.id; - const commitDate = new Date(commit.commit.author.date); - const existingDate = latestCommitDates.get(contributorId); - if (!existingDate || commitDate > existingDate) { - latestCommitDates.set(contributorId, commitDate); - } - } -}); +const latestCommitDates = totalCommitsData.reduce((acc, commit) => { + if (commit.author?.id && commit.commit?.author?.date) { + const commitDate = new Date(commit.commit.author.date); + const existingDate = acc.get(commit.author.id); + if (!existingDate || commitDate > existingDate) { + acc.set(commit.author.id, commitDate); + } + } + return acc; +}, new Map<number, Date>());website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (1)
73-78
: Add accessibility attributes to toggle groupEnhance accessibility by adding ARIA labels:
-<section className="flex mb-10"> +<section className="flex mb-10" role="region" aria-label="Sort contributors"> <ToggleGroup type="single" value={selectedToggle} onValueChange={handleToggleChange}> - <ToggleGroupItem value="commit count">Commit Count</ToggleGroupItem> - <ToggleGroupItem value="latest commit">Latest Commit</ToggleGroupItem> + <ToggleGroupItem value="commit count" aria-label="Sort by commit count">Commit Count</ToggleGroupItem> + <ToggleGroupItem value="latest commit" aria-label="Sort by latest commit">Latest Commit</ToggleGroupItem> </ToggleGroup> </section>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ui/src/index.ts
(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx
(4 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-commits.ts
(2 hunks)website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test website
- GitHub Check: Security checks (typescript)
- GitHub Check: Prettify
🔇 Additional comments (7)
ui/src/index.ts (1)
30-30
: LGTM! Export follows established patterns.The new toggle-group export maintains consistency with existing exports and supports the PR's toggle feature implementation.
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-commits.ts (3)
Line range hint
6-11
: LGTM: Type safety improvementThe interface update ensures type safety by making the author field non-nullable.
59-65
: LGTM: Return value enhancementThe addition of totalCommitsData to the return value supports the new sorting feature.
50-57
: Add error handling and rate limit considerationsThe pagination implementation could:
- Hit GitHub API rate limits with large repositories
- Fail silently if API requests fail
Add error handling and consider implementing rate limiting:
for (let page = 1; page <= Math.ceil(totalCommits / 100); page++) { + try { const pagedUrl = `https://api.github.com/repos/${owner}/${repo}/commits?per_page=100&page=${page}`; const pagedRes = await fetchData(owner, repo, pagedUrl); + if (!pagedRes.ok) { + throw new Error(`Failed to fetch page ${page}: ${pagedRes.statusText}`); + } const pagedData: GitHubCommit[] = await pagedRes.json(); totalCommitsData = totalCommitsData.concat(pagedData); + // Add delay to respect rate limits + await new Promise(resolve => setTimeout(resolve, 1000)); + } catch (error) { + console.error(`Error fetching commits page ${page}:`, error); + break; + } }website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx (1)
70-76
: LGTM: Clean sorting implementationThe sorting implementation is well-structured:
- Creates new arrays to avoid mutations
- Clear sorting logic for both criteria
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (2)
13-18
: LGTM: Well-defined interfaceThe Contributor interface properly defines all required fields.
51-52
: LGTM: Clean state managementThe state management implementation is efficient and properly initialized.
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 comments (1)
website/src/app/[lang]/[region]/(website)/open-source/(components)/get-commits.ts (1)
Line range hint
6-11
: Handle potential null author values from GitHub APIThe GitHub API can return null for the author field when commits are made with unregistered email addresses. The interface should allow for null values.
- author: { + author: { id: number; login: string; avatar_url: string; - }; + } | null;
🧹 Nitpick comments (1)
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (1)
Line range hint
87-94
: Consider performance optimizations for large listsFor better performance with large contributor lists:
- Implement virtualization using a library like
react-window
- Memoize contributor components
+import { FixedSizeGrid } from 'react-window'; + +const MemoizedContributor = React.memo(Contributor); + <section className="flex flex-wrap gap-4"> - {displayedContributors.map((contributor: Contributor) => ( - <Contributor + {displayedContributors.map((contributor: Contributor) => ( + <MemoizedContributor key={contributor.id} name={contributor.name} commits={contributor.commits}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ui/src/index.ts
(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx
(4 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/get-commits.ts
(2 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx
(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx
(1 hunks)website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- website/src/app/[lang]/[region]/(website)/open-source/(components)/issue.tsx
- website/src/app/[lang]/[region]/(website)/open-source/(components)/issues-client.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/src/index.ts
🔇 Additional comments (3)
website/src/app/[lang]/[region]/(website)/open-source/(sections)/contributors.tsx (2)
44-66
: Well-implemented data processing logicEfficient use of Map for tracking commit dates and proper filtering of contributors.
70-76
: Clean and efficient sorting implementationGood use of spread operator to avoid mutations and clear sorting logic.
website/src/app/[lang]/[region]/(website)/open-source/(components)/contributors-client.tsx (1)
80-85
: Well-implemented toggle functionalityClean integration with UI library and proper state management.
Add a toggle feature to the 'contributors' section to allow sorting the list of contributors by the number of commits or latest commits.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor