Add search bar and summary stats to Pull Requests page#8
Open
dennisimoo wants to merge 1 commit intomainfrom
Open
Add search bar and summary stats to Pull Requests page#8dennisimoo wants to merge 1 commit intomainfrom
dennisimoo wants to merge 1 commit intomainfrom
Conversation
Owner
Author
|
Hey @dennisimoo, thanks for your contribution! GitPay has reviewed your pull request. You can claim your SOL reward using the link below — just connect your Solana wallet and you're good to go. Powered by GitPay · Paid out on Solana |
📝 WalkthroughWalkthroughAdds search functionality and a real-time statistics toolbar to the PRs page. The new search feature matches contributor usernames, repositories, and PR titles, while a dedicated Stats row displays total PRs, pending count, claimed count, and average score. Filter buttons are consolidated into a toolbar alongside the search input. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/(app)/prs/page.tsx (1)
165-179: Consider memoizing derived stats.The pending and claimed counts are recalculated on every render. For large claim lists, this could be optimized using
useMemo:♻️ Optional optimization
+ const stats = useMemo(() => ({ + total: claims.length, + pending: claims.filter(c => !c.claimed).length, + claimed: claims.filter(c => c.claimed).length, + avgScore: claims.length ? Math.round(claims.reduce((s, c) => s + c.score, 0) / claims.length) : 0, + }), [claims]);Then reference
stats.total,stats.pending, etc. in the JSX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/prs/page.tsx around lines 165 - 179, The pending/claimed/total/avgScore calculations are recomputed each render from claims; wrap the derived stats in a useMemo hook (e.g., const stats = useMemo(() => { total: claims.length, pending: claims.filter(...).length, claimed: ..., avgScore: ... }, [claims])) and then replace uses of claims.length, claims.filter(...).length, and avgScore in the JSX with stats.total, stats.pending, stats.claimed, and stats.avgScore respectively; ensure you import useMemo from React and keep the same keys used in the mapped array for rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(app)/prs/page.tsx:
- Around line 197-205: The search input currently uses only a placeholder for
context (the <input> with value={search} and onChange={(e) =>
setSearch(e.target.value)}), which is not reliably announced by screen readers;
add an accessible label by adding an aria-label attribute (e.g.,
aria-label="Search PRs by contributor, repo, or title") to the same <input>
element so assistive technologies get explicit context while preserving the
existing placeholder and behavior.
- Around line 209-213: The empty-state message currently only checks filtered
and search; update the JSX where filtered and search are used to also consider
the active filter state (e.g., selectedFilter / statusFilter / filters) so the
message reflects whether there are no results because of a search, an active
filter, or truly no PRs; change the conditional that renders the <div> to
branch: if filtered.length === 0 and search show "No results matched your
search", else if filtered.length === 0 and a filter is active show "No pull
requests match the selected filter" (ideally including the filter name),
otherwise show "No pull requests yet". Use the existing symbols filtered and
search and the app's filter state variable to locate and implement the logic in
page.tsx.
---
Nitpick comments:
In `@src/app/`(app)/prs/page.tsx:
- Around line 165-179: The pending/claimed/total/avgScore calculations are
recomputed each render from claims; wrap the derived stats in a useMemo hook
(e.g., const stats = useMemo(() => { total: claims.length, pending:
claims.filter(...).length, claimed: ..., avgScore: ... }, [claims])) and then
replace uses of claims.length, claims.filter(...).length, and avgScore in the
JSX with stats.total, stats.pending, stats.claimed, and stats.avgScore
respectively; ensure you import useMemo from React and keep the same keys used
in the mapped array for rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c93ef42b-de91-4cab-b993-f1fb4aab589d
📒 Files selected for processing (1)
src/app/(app)/prs/page.tsx
Comment on lines
+197
to
+205
| <input | ||
| value={search} | ||
| onChange={(e) => setSearch(e.target.value)} | ||
| placeholder="Search by contributor, repo, or title…" | ||
| style={{ | ||
| flex: 1, padding: "6px 12px", border: "1px solid #e4e4e7", borderRadius: "7px", | ||
| fontSize: "12px", fontFamily: "inherit", color: "#000", outline: "none", background: "#fff", | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Add aria-label for accessibility.
The search input relies solely on placeholder for context, which is not announced by all screen readers. Adding an aria-label improves accessibility.
♿ Proposed fix
<input
value={search}
onChange={(e) => setSearch(e.target.value)}
placeholder="Search by contributor, repo, or title…"
+ aria-label="Search pull requests"
style={{📝 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.
Suggested change
| <input | |
| value={search} | |
| onChange={(e) => setSearch(e.target.value)} | |
| placeholder="Search by contributor, repo, or title…" | |
| style={{ | |
| flex: 1, padding: "6px 12px", border: "1px solid #e4e4e7", borderRadius: "7px", | |
| fontSize: "12px", fontFamily: "inherit", color: "#000", outline: "none", background: "#fff", | |
| }} | |
| /> | |
| <input | |
| value={search} | |
| onChange={(e) => setSearch(e.target.value)} | |
| placeholder="Search by contributor, repo, or title…" | |
| aria-label="Search pull requests" | |
| style={{ | |
| flex: 1, padding: "6px 12px", border: "1px solid `#e4e4e7`", borderRadius: "7px", | |
| fontSize: "12px", fontFamily: "inherit", color: "#000", outline: "none", background: "#fff", | |
| }} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/prs/page.tsx around lines 197 - 205, The search input
currently uses only a placeholder for context (the <input> with value={search}
and onChange={(e) => setSearch(e.target.value)}), which is not reliably
announced by screen readers; add an accessible label by adding an aria-label
attribute (e.g., aria-label="Search PRs by contributor, repo, or title") to the
same <input> element so assistive technologies get explicit context while
preserving the existing placeholder and behavior.
Comment on lines
209
to
213
| {filtered.length === 0 ? ( | ||
| <div style={{ border: "1px dashed #e4e4e7", borderRadius: "12px", padding: "48px 24px", textAlign: "center" }}> | ||
| <GitMerge size={28} strokeWidth={1.5} color="#d4d4d8" style={{ display: "inline-block", marginBottom: "10px" }} /> | ||
| <p style={{ fontSize: "14px", color: "#71717a" }}>No pull requests yet</p> | ||
| <p style={{ fontSize: "14px", color: "#71717a" }}>{search ? "No results matched your search" : "No pull requests yet"}</p> | ||
| </div> |
There was a problem hiding this comment.
Consider filter state in the empty message.
The message distinguishes between search and no-data, but doesn't account for filter state. If a user filters to "claimed" but all PRs are pending, they see "No pull requests yet" which is misleading since PRs exist.
💡 Suggested improvement
- <p style={{ fontSize: "14px", color: "#71717a" }}>{search ? "No results matched your search" : "No pull requests yet"}</p>
+ <p style={{ fontSize: "14px", color: "#71717a" }}>
+ {search || filter !== "all"
+ ? "No results matched your filters"
+ : "No pull requests yet"}
+ </p>📝 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.
Suggested change
| {filtered.length === 0 ? ( | |
| <div style={{ border: "1px dashed #e4e4e7", borderRadius: "12px", padding: "48px 24px", textAlign: "center" }}> | |
| <GitMerge size={28} strokeWidth={1.5} color="#d4d4d8" style={{ display: "inline-block", marginBottom: "10px" }} /> | |
| <p style={{ fontSize: "14px", color: "#71717a" }}>No pull requests yet</p> | |
| <p style={{ fontSize: "14px", color: "#71717a" }}>{search ? "No results matched your search" : "No pull requests yet"}</p> | |
| </div> | |
| {filtered.length === 0 ? ( | |
| <div style={{ border: "1px dashed `#e4e4e7`", borderRadius: "12px", padding: "48px 24px", textAlign: "center" }}> | |
| <GitMerge size={28} strokeWidth={1.5} color="#d4d4d8" style={{ display: "inline-block", marginBottom: "10px" }} /> | |
| <p style={{ fontSize: "14px", color: "#71717a" }}> | |
| {search || filter !== "all" | |
| ? "No results matched your filters" | |
| : "No pull requests yet"} | |
| </p> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/prs/page.tsx around lines 209 - 213, The empty-state message
currently only checks filtered and search; update the JSX where filtered and
search are used to also consider the active filter state (e.g., selectedFilter /
statusFilter / filters) so the message reflects whether there are no results
because of a search, an active filter, or truly no PRs; change the conditional
that renders the <div> to branch: if filtered.length === 0 and search show "No
results matched your search", else if filtered.length === 0 and a filter is
active show "No pull requests match the selected filter" (ideally including the
filter name), otherwise show "No pull requests yet". Use the existing symbols
filtered and search and the app's filter state variable to locate and implement
the logic in page.tsx.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit