feat: implement UI enhancements #132
Conversation
- Remove ThemeToggle from PageHeader (moved to sidebar) - Add collapsible sidebar with localStorage persistence * Sidebar can be collapsed to show only icons * State persists across sessions * Smooth transitions for width changes - Add ThemeToggle to sidebar footer - Create global AppHeader component with: * Breadcrumb navigation showing current path * Global search bar with Cmd+K shortcut display * Fixed positioning at top of authenticated pages - Display issue/PR numbers in #ID format in tables * Added to Quick Wins table * Added to Action Required table * Numbers extracted from GitHub URLs
- Create DetailPanel component for displaying issue details * Slide-over panel from right side * Shows issue description, comments, labels, and metadata * Smooth transitions and responsive design * No external markdown dependencies - Create UserHoverCard component with profile info * Shows user avatar, bio, company, location * Displays repos, followers, and following counts * Integration with GitHub API to fetch user profiles - Implement Open Source Score algorithm * Formula: (Commits × 2) + (PRs × 5) + Starred Repos * Calculates contributions from last 3 months * New getUserContributions method in github-api-client * Fetches user events and starred repositories - Add score visualization to UserHoverCard * Gradient progress bar showing score * Displays score with proper formatting * Tooltip explaining the calculation * Beautiful card design with blue/indigo gradient
…enhancements-01AWVJeHqCUTLDm4MCfh6hTH
…nts-01AWVJeHqCUTLDm4MCfh6hTH Claude/lint fixes UI enhancements 01 awv je hq cutl dm4 m cfh6h th
Accessibility improvements: - Add aria-label to all icon-only navigation links in Sidebar - Add aria-hidden="true" to decorative icons - Add sr-only spans for screen reader support - Fix mobile sidebar collapse (keep full width on mobile with lg:w-16) Type safety fixes: - Replace custom UserProfile interface with GitHubUserDetailed type - Ensure type consistency across components API improvements: - Add getRequestHeaders() private method for consistent header creation - Fix commits counting to use actual payload.commits.length - Use this.baseUrl and getRequestHeaders() instead of hardcoded URL Modified files: - src/components/layout/Sidebar.tsx - src/components/ui/user-hover-card.tsx - src/lib/api/github-api-client.ts
…nts-01AWVJeHqCUTLDm4MCfh6hTH fix: resolve code quality issues in UI components and API client
- Remove unused imports (AppHeader, GitBranch, githubAPIClient, Badge, GitMerge, CardTitle, Home, Star, Users, cn) - Remove unused variables (sidebarCollapsed, onClose, tasks) - Prefix unused parameters with underscore where needed - Fix useEffect dependency warning in RepoDeepDive with eslint-disable comment - Fix TypeScript 'any' type in ReactMarkdown code component by adding explicit type annotations - Change empty interface to type alias in command.tsx All changes maintain existing functionality while ensuring code quality standards.
The updateColumn function from the Kanban store expects Partial<KanbanColumn>, but EditableColumn expects individual title and color parameters. Added handleUpdateColumn wrapper to bridge the gap.
The GitHubAPIClient class has a setUserToken method, not setToken. Updated the method call to match the correct API.
OrgData interface has orgName property, not organizationName. Updated Sidebar.tsx to use the correct property name.
Create hover-card.tsx based on Radix UI HoverCard primitive. This component is used by UserHoverCard and follows the same pattern as other UI primitives (tooltip, dialog, etc.).
Add missing Radix UI hover-card package required by the hover-card UI component.
Replace custom UserProfile interface with GitHubUserDetailed from types to match the return type of githubAPIClient.getUserProfile(). This fixes type mismatch where name was string|null vs string|undefined.
Add missing getHeaders() private method that constructs headers with GitHub token authorization. This method is used by getUserContributions() for fetching user starred repositories.
…XN6P2vKAAAZmx22HZoj fix: resolve all ESLint errors across components
…eHeader, and Sidebar components
WalkthroughAdds Radix hover-card dependency; introduces HoverCard and UserHoverCard UI, a DetailPanel, AppHeader, sidebar collapse refactor, issue-number badges, a user-contributions API method and a user-scores store/types, plus several import and type cleanup changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UserHoverCard
participant APIClient
participant GH as GitHub
Note over UserHoverCard,APIClient: Hover -> fetch profile (+ optional contributions)
User->>UserHoverCard: hover username
activate UserHoverCard
UserHoverCard->>APIClient: getUserProfile(username)
APIClient->>GH: GET /users/{username}
GH-->>APIClient: profile
APIClient-->>UserHoverCard: profile
alt showScore enabled
UserHoverCard->>APIClient: getUserContributions(username)
APIClient->>GH: GET /users/{username}/events
GH-->>APIClient: events
APIClient->>GH: GET /users/{username}/starred (or /user/starred)
GH-->>APIClient: stars
APIClient-->>UserHoverCard: {commits, prs, stars}
UserHoverCard->>UserHoverCard: calculateOpenSourceScore()
end
UserHoverCard-->>User: render hover card (profile + optional score)
deactivate UserHoverCard
sequenceDiagram
participant User
participant AppHeader
participant Router as usePathname
participant SearchStore
participant SearchModal
User->>AppHeader: route change / render
AppHeader->>Router: get path segments
Router-->>AppHeader: segments
AppHeader->>AppHeader: build breadcrumbs
AppHeader-->>User: render header + search button
User->>AppHeader: click search
AppHeader->>SearchStore: open modal
SearchStore-->>SearchModal: modal visible
SearchModal-->>User: show search UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 7
♻️ Duplicate comments (1)
src/app/action-required/page.tsx (1)
76-80: Extract duplicated helper to shared utility module.This function is identical to the one in
src/components/quick-wins/columns.tsx(lines 13-16). See the review comment in that file for consolidation guidance.
🧹 Nitpick comments (14)
src/components/search/RepoDeepDive.tsx (1)
73-82: Review the eslint-disable for exhaustive-deps.The effect checks
readme,fileTree.length, andissues.lengthbut doesn't include them in the dependency array. While the current pattern (load data only when tab switches AND data doesn't exist) works, this suppression could mask issues if the loading logic changes.Consider using separate effects for each tab's data loading, or add comments explaining why these dependencies are intentionally omitted to prevent future confusion.
Example refactor to make intent clearer:
useEffect(() => { if (activeTab === "readme" && !readme) { loadReadme() } else if (activeTab === "files" && fileTree.length === 0) { loadFileTree() } else if (activeTab === "issues" && issues.length === 0) { loadIssues() } - // eslint-disable-next-line react-hooks/exhaustive-deps + // Only load data when activeTab changes; readme/fileTree/issues are intentionally + // not in deps to avoid re-running when data is set (which would cause infinite loops) + // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeTab])src/components/layout/AppHeader.tsx (1)
10-17: Consider handling dynamic route segments.The
routeLabelsmap works well for static routes, but dynamic segments (e.g.,/user/[id]or/repo/[owner]/[name]) will fall back to displaying the raw parameter value. This may result in inconsistent or unclear breadcrumb labels.Consider extending the breadcrumb logic to handle dynamic segments:
const formatBreadcrumbLabel = (segment: string): string => { // Check static labels first if (routeLabels[segment]) return routeLabels[segment]; // Handle UUIDs or IDs if (/^[0-9a-f-]{36}$/.test(segment)) return "Details"; // Capitalize first letter for readable fallback return segment.charAt(0).toUpperCase() + segment.slice(1); };src/lib/api/github-api-client.ts (1)
879-934: Consider using fetchWithCache for consistency.The
getUserContributionsmethod makes a directfetchcall (line 914) instead of using thefetchWithCachemethod. This bypasses the caching layer and rate limit handling that other methods benefit from.While the link header parsing for star count requires accessing response headers directly, consider wrapping the events endpoint call in
fetchWithCache:- const eventsEndpoint = `/users/${username}/events/public?per_page=100`; - const events = await this.fetchWithCache<Array<{ + const events = await this.fetchWithCache<Array<{ type: string; created_at: string; payload?: { commits?: unknown[]; size?: number; }; - }>>( - eventsEndpoint, - true - ); + }>>(`/users/${username}/events/public?per_page=100`, true);The direct fetch for starred count is acceptable since you need the Link header.
src/components/ui/user-hover-card.tsx (4)
23-29: Consider extracting score calculation constants.The weighting factors (commits × 2, PRs × 5, stars × 1) lack documentation explaining the rationale. Consider extracting these as named constants with comments describing why these specific weights were chosen.
+// Score weighting factors based on contribution impact +const SCORE_WEIGHTS = { + COMMIT: 2, // Individual commits show consistent activity + PR: 5, // Pull requests demonstrate collaborative contribution + STAR: 1, // Stars reflect project popularity/reach +} as const; + function calculateOpenSourceScore(profile: GitHubUserDetailed | null, contributions?: { commits: number; prs: number; stars: number }): number { - const commitsScore = (contributions?.commits || 0) * 2; - const prsScore = (contributions?.prs || 0) * 5; + const commitsScore = (contributions?.commits || 0) * SCORE_WEIGHTS.COMMIT; + const prsScore = (contributions?.prs || 0) * SCORE_WEIGHTS.PR; - const starsScore = contributions?.stars || 0; + const starsScore = (contributions?.stars || 0) * SCORE_WEIGHTS.STAR; return commitsScore + prsScore + starsScore; }
36-58: Enhance error handling for better UX.The error handling logs to console but doesn't set an error state, so users see only the loading spinner indefinitely or a "Failed to load" message without details. Consider adding error state and displaying a user-friendly error message.
export function UserHoverCard({ username, children, showScore = false }: UserHoverCardProps) { const [profile, setProfile] = useState<GitHubUserDetailed | null>(null); const [loading, setLoading] = useState(false); const [score, setScore] = useState<number | null>(null); + const [error, setError] = useState<string | null>(null); useEffect(() => { const fetchProfile = async () => { if (!username) return; setLoading(true); + setError(null); try { const userProfile = await githubAPIClient.getUserProfile(username); setProfile(userProfile); if (showScore && userProfile) { const contributions = await githubAPIClient.getUserContributions(username); const calculatedScore = calculateOpenSourceScore(userProfile, contributions); setScore(calculatedScore); } } catch (error) { console.error("Failed to fetch user profile:", error); + setError("Unable to load profile"); } finally { setLoading(false); } }; fetchProfile(); }, [username, showScore]);Then update the error display to show the specific error message.
36-58: Consider adding request cancellation.If the
usernameprop changes rapidly (e.g., during typing or quick navigation), the previous fetch continues running, potentially causing a race condition where an older response overwrites a newer one. Consider usingAbortControllerto cancel in-flight requests.useEffect(() => { + const abortController = new AbortController(); + const fetchProfile = async () => { if (!username) return; setLoading(true); try { - const userProfile = await githubAPIClient.getUserProfile(username); + const userProfile = await githubAPIClient.getUserProfile(username, { signal: abortController.signal }); setProfile(userProfile); if (showScore && userProfile) { - const contributions = await githubAPIClient.getUserContributions(username); + const contributions = await githubAPIClient.getUserContributions(username, { signal: abortController.signal }); const calculatedScore = calculateOpenSourceScore(userProfile, contributions); setScore(calculatedScore); } } catch (error) { + if (error.name === 'AbortError') return; console.error("Failed to fetch user profile:", error); } finally { setLoading(false); } }; fetchProfile(); + + return () => abortController.abort(); }, [username, showScore]);Note: This requires the GitHub API client methods to accept and respect abort signals.
157-162: Extract hardcoded score normalization factor.The value
1000used for normalizing the progress bar width is a magic number. Consider extracting it as a named constant to improve maintainability and make the normalization logic clearer.+const MAX_SCORE_FOR_PROGRESS = 1000; + export function UserHoverCard({ username, children, showScore = false }: UserHoverCardProps) { // ... rest of component <div className="relative w-full h-2 bg-gray-200 dark:bg-gray-700 rounded-full overflow-hidden mt-2"> <div className="absolute top-0 left-0 h-full bg-gradient-to-r from-blue-500 to-indigo-500 rounded-full transition-all duration-500" - style={{ width: `${Math.min((score / 1000) * 100, 100)}%` }} + style={{ width: `${Math.min((score / MAX_SCORE_FOR_PROGRESS) * 100, 100)}%` }} /> </div>src/components/common/DetailPanel.tsx (2)
63-69: Animation workaround is functional but could be improved.The 10ms setTimeout is a common workaround to trigger CSS transitions, but it's somewhat fragile and relies on browser timing. Consider using React Transition Group or Framer Motion for more reliable animations, or the modern CSS
@starting-stylerule for enter animations.
199-222: Extract hardcoded comment limit.The value
5for limiting displayed comments is hardcoded in two places (slice and the "more comments" calculation). Consider extracting it as a constant for maintainability.+const MAX_DISPLAYED_COMMENTS = 5; + export function DetailPanel({ isOpen, onClose, issue, comments, loading }: DetailPanelProps) { // ... rest of component <h4 className="font-semibold mb-3">Recent Comments ({comments.length})</h4> <div className="space-y-3"> - {comments.slice(0, 5).map((comment) => ( + {comments.slice(0, MAX_DISPLAYED_COMMENTS).map((comment) => ( <Card key={comment.id}> {/* ... */} </Card> ))} - {comments.length > 5 && ( + {comments.length > MAX_DISPLAYED_COMMENTS && ( <p className="text-sm text-muted-foreground text-center"> - And {comments.length - 5} more comments... + And {comments.length - MAX_DISPLAYED_COMMENTS} more comments... </p> )}src/components/layout/Sidebar.tsx (5)
29-30: Sidebar collapse state wiring and toggle placement look solid; minor store-usage refinement possibleThe introduction of
sidebarCollapsedpersisted inusePreferencesStoreand the new footer toggle button correctly implement a collapsible, remembered sidebar. The toggle UX (icon + label/sr-only label,aria-labelreflecting action, desktop-only visibility viahidden lg:flex) is well thought out.One improvement to consider: both here and elsewhere in this component,
usePreferencesStore()is called without a selector, which subscribes to the full store and can cause unnecessary re-renders as preferences grow. For this new state, you could instead use selectors, e.g.usePreferencesStore((s) => s.sidebarCollapsed)andusePreferencesStore((s) => s.setSidebarCollapsed), and similarly forpinnedRepos, to keep this component’s subscriptions tighter.Also applies to: 44-44, 474-493, 496-496
171-176: Responsive aside layout and transition setup are correctThe updated
<aside>classes (fixedpositioning, transform +transition-all, and width switching betweenlg:w-16 w-64vsw-64) give you the expected behavior: full-width overlay on mobile and narrow/expanded variants only on large screens. Theflex flex-col h-screenbaseline also keeps the header/nav/footer blocks predictable.Be aware that animating width on the whole sidebar (
transition-all) can cause layout reflow in complex layouts; if you ever see jank in heavy views, consider restricting transitions totransformand managing adjacent content with CSS grid or a container that can absorb the width snap.
181-200: Header behavior under collapse is consistent; consider optional affordance tweakThe header correctly swaps between full branding (
GitHubMon+ subtitle + close button on mobile) and a compact “G” mark when collapsed. This nicely matches the collapsed width and keeps the top chrome minimal.If you later want to make the collapse behavior discoverable from the header itself (especially on very tall screens), you could optionally mirror the collapse/expand icon here in addition to the footer control, but that’s purely UX polish—not required for correctness.
209-220: Collapsed/expanded nav variants work; Quick Wins link could explicitly set a default tabThe collapsed vs expanded variants for Dashboard, Action Required, Quick Wins, and Favorites are implemented cleanly: icon-only in collapsed mode with
aria-label+sr-onlylabels, and full text + badges + accordions when expanded. Active-state styling viapathnameand theisActionRequiredTab/isQuickWinsTabflags is consistent.One nuance: the collapsed Quick Wins link uses
href="/quick-wins"(notabquery param), while the expanded submenu links use?tab=good-issuesand?tab=easy-fixes. Unless the/quick-winspage itself defaults the tab to “good-issues”, this can lead to a route where no tab appears active in this sidebar and, depending on the page implementation, potentially a different default view than intended.To make behavior explicit and consistent with the expanded submenu, you might want to change the collapsed Quick Wins link to point to the same default tab:
- <Link - href="/quick-wins" + <Link + href="/quick-wins?tab=good-issues"Everything else in these nav changes looks good and accessibility-friendly.
Also applies to: 223-271, 331-332, 337-383, 424-425, 429-442, 443-445
509-509: Footer org label fallback is sensibleUsing
orgData.orgName || "GitHub User"is a reasonable fallback and keeps the footer stable even when organization metadata is missing. If you later add multi-org support, consider whether this should prefer a different display name (e.g., a shorter org slug), but for now this is a clear improvement over leaving it blank.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
package.json(1 hunks)src/app/action-required/page.tsx(2 hunks)src/app/login/page.tsx(1 hunks)src/components/command/CommandPalette.tsx(0 hunks)src/components/common/DetailPanel.tsx(1 hunks)src/components/kanban/ColumnManagementModal.tsx(3 hunks)src/components/kanban/KanbanBoard.tsx(1 hunks)src/components/layout/AppHeader.tsx(1 hunks)src/components/layout/Breadcrumb.tsx(1 hunks)src/components/layout/Layout.tsx(2 hunks)src/components/layout/PageHeader.tsx(0 hunks)src/components/layout/Sidebar.tsx(6 hunks)src/components/quick-wins/columns.tsx(2 hunks)src/components/search/RepoDeepDive.tsx(3 hunks)src/components/ui/command.tsx(1 hunks)src/components/ui/hover-card.tsx(1 hunks)src/components/ui/user-hover-card.tsx(1 hunks)src/lib/api/github-api-client.ts(3 hunks)
💤 Files with no reviewable changes (2)
- src/components/layout/PageHeader.tsx
- src/components/command/CommandPalette.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/ui/user-hover-card.tsx (2)
src/types/github.ts (1)
GitHubUserDetailed(73-106)src/lib/api/github-api-client.ts (1)
githubAPIClient(1404-1404)
src/components/kanban/KanbanBoard.tsx (1)
src/lib/api/github-api-client.ts (1)
githubAPIClient(1404-1404)
src/components/layout/Sidebar.tsx (1)
src/stores/index.ts (2)
useSidebarState(179-182)useStoreHydration(43-55)
src/components/ui/hover-card.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (13)
src/components/ui/command.tsx (1)
26-26: LGTM – Clean simplification.Changing from an interface extension to a type alias is appropriate here since no additional properties are being added. This makes the code more concise and idiomatic.
src/components/search/RepoDeepDive.tsx (1)
288-292: Improved type safety with explicit parameter types.The explicit typing of the
codefunction parameters (inline,className,children) is a good improvement over the previous broader signature. This provides better type safety and IntelliSense support.src/components/kanban/ColumnManagementModal.tsx (2)
179-180: LGTM – Good cleanup of unused store property.Removing the unused
tasksproperty from the destructuring reduces unnecessary coupling to the store.
209-211: No issues found; the adapter implementation is correct.Verification confirms the store's
updateColumnsignature matches the component's wrapper usage. The method expectsupdateColumn(id: string, updates: Partial<KanbanColumn>), and the component correctly calls it with{ title, color }, which are valid KanbanColumn fields. The adapter pattern is properly implemented with no inconsistencies in other call sites.src/components/quick-wins/columns.tsx (1)
30-64: LGTM! Issue number badge implementation looks good.The conditional rendering and styling of the issue number badge is well-implemented. The monospace font and flex-shrink-0 ensure consistent display alongside the title.
src/app/login/page.tsx (1)
7-7: LGTM! Unused import removed.Good housekeeping -
CardTitlewas imported but never used in this file.src/app/action-required/page.tsx (1)
444-448: LGTM! Issue number badge rendering is consistent.The implementation matches the pattern used in Quick Wins and provides a good user experience.
src/components/layout/AppHeader.tsx (1)
62-75: LGTM! Search button with keyboard shortcut is user-friendly.The combination of visual search prompt and keyboard shortcut badge (⌘ K) provides excellent discoverability for the search feature.
src/components/ui/hover-card.tsx (1)
1-29: LGTM! Radix UI wrapper follows established patterns.The implementation correctly wraps
@radix-ui/react-hover-cardwith proper ref forwarding, default props, and consistent styling. The animation classes align with Radix's data attributes for smooth transitions.package.json (1)
19-19: LGTM! Dependency added for new HoverCard component.The
@radix-ui/react-hover-carddependency is properly versioned and consistent with other Radix UI packages in the project.src/components/kanban/KanbanBoard.tsx (1)
394-394: LGTM! Method call updated to match renamed API.The change from
setTokentosetUserTokencorrectly reflects the API update ingithub-api-client.ts. The authentication flow and error handling remain properly implemented.src/components/layout/Breadcrumb.tsx (1)
50-50: LGTM: Cleanup of unused parameter.Removing the unused
indexparameter from the map callback is a good cleanup. The key correctly usescrumb.pathwhich is unique for each breadcrumb.src/components/ui/user-hover-card.tsx (1)
163-165: No action required—timeframe is correctly implemented.The
getUserContributions()function insrc/lib/api/github-api-client.ts(lines 881-882) explicitly calculates the 3-month timeframe withthreeMonthsAgo.setMonth(threeMonthsAgo.getMonth() - 3). The tooltip text accurately reflects this implementation.
| {issue.labels.map((label, idx) => ( | ||
| <Badge | ||
| key={idx} | ||
| variant="outline" | ||
| style={{ | ||
| borderColor: `#${label.color}`, | ||
| backgroundColor: `#${label.color}20`, | ||
| color: `#${label.color}`, | ||
| }} | ||
| > | ||
| {label.name} | ||
| </Badge> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use unique identifier as key instead of array index.
Using the array index idx as the key can cause React rendering issues if the label order changes. If labels have unique identifiers, use those; otherwise, use label.name as the key (assuming names are unique within an issue).
- {issue.labels.map((label, idx) => (
+ {issue.labels.map((label) => (
<Badge
- key={idx}
+ key={label.name}
variant="outline"📝 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.
| {issue.labels.map((label, idx) => ( | |
| <Badge | |
| key={idx} | |
| variant="outline" | |
| style={{ | |
| borderColor: `#${label.color}`, | |
| backgroundColor: `#${label.color}20`, | |
| color: `#${label.color}`, | |
| }} | |
| > | |
| {label.name} | |
| </Badge> | |
| ))} | |
| {issue.labels.map((label) => ( | |
| <Badge | |
| key={label.name} | |
| variant="outline" | |
| style={{ | |
| borderColor: `#${label.color}`, | |
| backgroundColor: `#${label.color}20`, | |
| color: `#${label.color}`, | |
| }} | |
| > | |
| {label.name} | |
| </Badge> | |
| ))} |
🤖 Prompt for AI Agents
In src/components/common/DetailPanel.tsx around lines 146 to 158, the Badge list
uses the array index (idx) as the React key which can cause rendering issues;
change the key to a stable unique identifier instead (e.g., use label.id if
present, otherwise fallback to label.name) so use key={label.id ?? label.name}
or similar to ensure stable keys across reorders.
Critical: Potential XSS vulnerability with unsanitized color values.
The label.color value is directly injected into inline styles without validation. If a malicious actor can control label colors (e.g., through API manipulation), they could inject CSS that executes JavaScript via url('javascript:...') or other CSS-based attacks.
Apply this diff to sanitize the color value:
+function sanitizeHexColor(color: string): string {
+ // Ensure color is a valid 3 or 6 character hex code
+ const hexMatch = color.match(/^[0-9A-Fa-f]{6}$|^[0-9A-Fa-f]{3}$/);
+ return hexMatch ? color : '808080'; // fallback to gray if invalid
+}
+
export function DetailPanel({ isOpen, onClose, issue, comments, loading }: DetailPanelProps) {
// ... rest of component
{issue.labels.map((label, idx) => (
+ const safeColor = sanitizeHexColor(label.color);
<Badge
key={idx}
variant="outline"
style={{
- borderColor: `#${label.color}`,
+ borderColor: `#${safeColor}`,
- backgroundColor: `#${label.color}20`,
+ backgroundColor: `#${safeColor}20`,
- color: `#${label.color}`,
+ color: `#${safeColor}`,
}}
>
{label.name}
</Badge>
))}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/common/DetailPanel.tsx around lines 146 to 158, label.color is
injected directly into inline styles which can lead to CSS/XSS injection;
validate and sanitize the color before use by allowing only hex colors (3 or 6
hex digits) via a regex, normalize to lowercase and prepend '#' if missing, fall
back to a safe default color when validation fails, and construct derived values
(e.g. backgroundColor with appended alpha) from the sanitized hex only; replace
direct usage of label.color in the JSX with the sanitized value computed
beforehand.
| {issue.assignees.map((assignee, idx) => ( | ||
| <div key={idx} className="flex items-center gap-2"> | ||
| <Avatar className="w-6 h-6"> | ||
| <AvatarImage src={assignee.avatar_url} alt={assignee.login} /> | ||
| <AvatarFallback>{assignee.login.charAt(0).toUpperCase()}</AvatarFallback> | ||
| </Avatar> | ||
| <span className="text-sm">{assignee.login}</span> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use unique identifier as key instead of array index.
Similar to the labels issue, using the array index idx as the key for assignees can cause rendering issues. Use a unique identifier like assignee.login (assuming it's unique) as the key instead.
- {issue.assignees.map((assignee, idx) => (
- <div key={idx} className="flex items-center gap-2">
+ {issue.assignees.map((assignee) => (
+ <div key={assignee.login} className="flex items-center gap-2">
<Avatar className="w-6 h-6">
<AvatarImage src={assignee.avatar_url} alt={assignee.login} />
<AvatarFallback>{assignee.login.charAt(0).toUpperCase()}</AvatarFallback>
</Avatar>
<span className="text-sm">{assignee.login}</span>
</div>
))}📝 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.
| {issue.assignees.map((assignee, idx) => ( | |
| <div key={idx} className="flex items-center gap-2"> | |
| <Avatar className="w-6 h-6"> | |
| <AvatarImage src={assignee.avatar_url} alt={assignee.login} /> | |
| <AvatarFallback>{assignee.login.charAt(0).toUpperCase()}</AvatarFallback> | |
| </Avatar> | |
| <span className="text-sm">{assignee.login}</span> | |
| </div> | |
| ))} | |
| {issue.assignees.map((assignee) => ( | |
| <div key={assignee.login} className="flex items-center gap-2"> | |
| <Avatar className="w-6 h-6"> | |
| <AvatarImage src={assignee.avatar_url} alt={assignee.login} /> | |
| <AvatarFallback>{assignee.login.charAt(0).toUpperCase()}</AvatarFallback> | |
| </Avatar> | |
| <span className="text-sm">{assignee.login}</span> | |
| </div> | |
| ))} |
🤖 Prompt for AI Agents
In src/components/common/DetailPanel.tsx around lines 167 to 175, the assignees
list is using the array index (idx) as the React key which can cause rendering
bugs; replace the index with a stable unique identifier such as assignee.login
(or another unique id on the assignee object) by using that property as the key
for each mapped element so React can correctly track list items during
re-renders.
| import { useSidebarState } from "@/stores"; | ||
| import { SidebarToggle } from "./SidebarToggle"; | ||
| import { Breadcrumb } from "./Breadcrumb"; | ||
| // import { Breadcrumb } from "./Breadcrumb"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify AppHeader is rendering breadcrumbs elsewhere.
The Breadcrumb component has been commented out, and according to the PR objectives, breadcrumb navigation is now handled by the AppHeader component. Please verify that AppHeader is properly imported and rendered in the application layout to ensure breadcrumb functionality isn't lost.
Also applies to: 27-27
🏁 Script executed:
#!/bin/bash
# Verify AppHeader is imported and used in the codebase
rg -n "import.*AppHeader" --type=tsx --type=ts
rg -n "<AppHeader" --type=tsx --type=tsLength of output: 55
I'll retry the verification with corrected shell scripts:
🏁 Script executed:
#!/bin/bash
# Search for AppHeader imports (without problematic --type flag)
echo "=== Searching for AppHeader imports ==="
rg -n "import.*AppHeader" src/
echo "=== Searching for AppHeader component usage ==="
rg -n "<AppHeader" src/
echo "=== Checking Layout.tsx file ==="
cat src/components/layout/Layout.tsx
echo "=== Searching for Breadcrumb references (to verify commented out) ==="
rg -n "Breadcrumb" src/components/layout/Length of output: 408
🏁 Script executed:
#!/bin/bash
# Search for AppHeader imports
echo "=== Searching for AppHeader imports ==="
rg -n "import.*AppHeader"
echo "=== Searching for AppHeader component usage ==="
rg -n "<AppHeader"
echo "=== Checking Layout.tsx file ==="
cat src/components/layout/Layout.tsx
echo "=== Searching for Breadcrumb references ==="
rg -n "Breadcrumb"Length of output: 1362
🏁 Script executed:
#!/bin/bash
# Final verification: check if AppHeader component exists anywhere
echo "=== Searching for AppHeader component definition ==="
rg -n "export.*AppHeader|function AppHeader|const AppHeader"
echo "=== Checking for any breadcrumb rendering in app ==="
rg -n "Breadcrumb" -A 2 -B 2Length of output: 1361
🏁 Script executed:
#!/bin/bash
# Check AppHeader content
echo "=== Checking AppHeader.tsx content ==="
cat src/components/layout/AppHeader.tsx
echo "=== Verify AppHeader is imported/used anywhere ==="
rg -n "AppHeader" --type-add="tsx:*.tsx" -g "*.tsx" -g "*.ts"
echo "=== Check root layout or app component ==="
fd -e "tsx" -e "ts" | head -20 | xargs -I {} sh -c 'echo "=== {} ===" && head -30 {}'Length of output: 21939
AppHeader exists but is not being used—breadcrumb functionality is missing from the application.
Verification confirms AppHeader is properly implemented with breadcrumb rendering logic (src/components/layout/AppHeader.tsx:19-59), but it is never imported or rendered anywhere in the codebase. The Breadcrumb component has been removed from Layout.tsx (lines 7, 27), but AppHeader has not been integrated as its replacement.
To restore breadcrumb navigation: Import AppHeader in Layout.tsx and render it in the page layout (likely as a sticky header above the main content area).
🤖 Prompt for AI Agents
In src/components/layout/Layout.tsx around line 7, the Layout removed Breadcrumb
but never imports or renders the existing AppHeader
(src/components/layout/AppHeader.tsx:19-59), so breadcrumb navigation is
missing; fix by importing AppHeader at the top of Layout.tsx and render it as
the top/sticky header (above the main content area) where Breadcrumb previously
sat, ensuring any required props (e.g., location, title, or routes) are passed
through or derived from existing Layout props and preserving layout styling
(sticky/header z-index and spacing).
| function extractIssueNumber(url: string): string | null { | ||
| const match = url.match(/\/(issues|pull)\/(\d+)/); | ||
| return match ? match[2] : null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated helper to shared utility module.
The extractIssueNumber function is duplicated in src/app/action-required/page.tsx (lines 76-80) with identical implementation. This violates the DRY principle.
Consider extracting this to a shared utility module (e.g., src/lib/utils/github.ts):
// src/lib/utils/github.ts
export function extractIssueNumber(url?: string): string | null {
if (!url) return null;
const match = url.match(/\/(issues|pull)\/(\d+)/);
return match ? match[2] : null;
}Then import it in both files:
+import { extractIssueNumber } from "@/lib/utils/github";
-function extractIssueNumber(url: string): string | null {
- const match = url.match(/\/(issues|pull)\/(\d+)/);
- return match ? match[2] : null;
-}🤖 Prompt for AI Agents
In src/components/quick-wins/columns.tsx around lines 13 to 16, the local
extractIssueNumber helper is duplicated elsewhere; extract it to a shared module
src/lib/utils/github.ts as an exported function (accepting an optional string
and returning string | null, returning null for falsy input and matching
/\/(issues|pull)\/(\d+)/ to return group 2), then remove the local
implementation and import the new utility in this file and in
src/app/action-required/page.tsx, updating any calls to the function to match
the optional-parameter signature.
| } | ||
|
|
||
| export function RepoDeepDive({ repo, onClose }: RepoDeepDiveProps) { | ||
| export function RepoDeepDive({ repo }: RepoDeepDiveProps) { |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that the onClose prop is no longer passed by parent components.
The onClose prop has been removed from the function signature but remains in the RepoDeepDiveProps interface (line 32). While this is not breaking since it's optional, verify that no parent components are passing this prop.
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of RepoDeepDive component and check if onClose is passed
# Search for RepoDeepDive component usage
rg -n -C3 --type=tsx --type=ts '<RepoDeepDive' -g '!src/components/search/RepoDeepDive.tsx'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of RepoDeepDive component
# Search for RepoDeepDive component usage (JSX/TSX files)
rg -n -C3 '<RepoDeepDive' --glob='*.tsx' --glob='*.ts' -g '!src/components/search/RepoDeepDive.tsx'
echo "---"
echo "Searching for RepoDeepDive imports:"
echo "---"
# Also search for imports
rg -n 'import.*RepoDeepDive' --glob='*.tsx' --glob='*.ts' -g '!src/components/search/RepoDeepDive.tsx'Length of output: 701
Remove the unused onClose prop from RepoDeepDiveProps interface or restore it to the function signature.
The onClose prop has been removed from the function destructuring (line 59) but a parent component (SplitViewSearch.tsx:212) is still passing onClose={() => setShowDeepDive(false)}. The prop remains in the interface definition (RepoDeepDiveProps line 32) but is ignored.
Choose one approach:
- Remove
onCloseentirely: Delete from interface and removeonClose={...}fromsrc/components/search/SplitViewSearch.tsx:212 - Restore usage: Re-add
onCloseto the function destructuring if the callback is still needed
This incomplete cleanup creates a prop mismatch between parent and child components.
🤖 Prompt for AI Agents
In src/components/search/RepoDeepDive.tsx around line 59, the RepoDeepDiveProps
interface still defines an onClose prop but the component removed it from the
function destructuring, causing a mismatch with the parent (SplitViewSearch.tsx)
which still passes onClose; restore the prop by adding onClose back into the
RepoDeepDive function signature/destructuring and wire it to the component's
close action (e.g., call onClose() where the deep-dive should be dismissed or
when the close button is clicked) so the parent callback is honored;
alternatively, if the close callback is no longer needed, remove onClose from
the RepoDeepDiveProps interface and delete the onClose={...} from
src/components/search/SplitViewSearch.tsx at line ~212 to keep props consistent.
| private getHeaders(): HeadersInit { | ||
| const headers: HeadersInit = { | ||
| Accept: "application/vnd.github.v3+json", | ||
| "User-Agent": "GitHubMon/1.0", | ||
| }; | ||
|
|
||
| if (this.githubToken) { | ||
| headers["Authorization"] = `Bearer ${this.githubToken}`; | ||
| } | ||
|
|
||
| return headers; | ||
| } |
There was a problem hiding this comment.
Remove duplicate method - code duplication.
getHeaders() (lines 204-215) and getRequestHeaders() (lines 241-250) are identical implementations. This duplication should be consolidated into a single method.
Apply this diff to remove the duplicate:
private getHeaders(): HeadersInit {
const headers: HeadersInit = {
Accept: "application/vnd.github.v3+json",
"User-Agent": "GitHubMon/1.0",
};
if (this.githubToken) {
headers["Authorization"] = `Bearer ${this.githubToken}`;
}
return headers;
}
- private getRequestHeaders(): HeadersInit {
- const headers: HeadersInit = {
- Accept: "application/vnd.github.v3+json",
- "User-Agent": "GitHubMon/1.0",
- };
- if (this.githubToken) {
- headers["Authorization"] = `Bearer ${this.githubToken}`;
- }
- return headers;
- }Then update line 915 to use getHeaders():
- headers: this.getRequestHeaders(),
+ headers: this.getHeaders(),Also applies to: 241-250
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 204-215 and 241-250, there are
two identical methods getHeaders() and getRequestHeaders(); remove the duplicate
getRequestHeaders() implementation and consolidate to a single getHeaders()
method (keep the one at 204-215), then update any call sites (notably the
reference at line ~915) to call getHeaders() instead of getRequestHeaders();
ensure imports/types remain valid and run tests/lint to confirm no remaining
references to getRequestHeaders().
- Add UserContributions, UserScore, and UserScoreCacheEntry types - Create userScores Zustand store with 30-minute caching - Implement score calculation: (commits * 2) + (PRs * 5) + stars - Add user level classification (beginner to master) - Integrate UserHoverCard in QuickWins table author column - Display score with progress bar and contribution tooltip - Leverage existing getUserContributions API method - Add hydration support for userScores store The hover card shows user profile info, company, location, and an open source score based on last 3 months contributions.
…-01REMSaoZ3VK6uVAFhJsa6vM feat: implement user hover card with open source scoring system
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/quick-wins/columns.tsx (1)
14-17: Duplicate code: extract to shared utility.This concern was already flagged in previous review comments. The
extractIssueNumberhelper is duplicated insrc/app/action-required/page.tsx.
🧹 Nitpick comments (1)
src/stores/userScores.ts (1)
19-29: Consider extracting magic numbers as named constants.The score calculation formula and level thresholds are well-implemented but use hardcoded values. Consider extracting these for better maintainability and documentation:
+const SCORE_WEIGHTS = { + COMMITS: 2, + PRS: 5, + STARS: 1, +} as const; + +const LEVEL_THRESHOLDS = { + MASTER: 1000, + EXPERT: 500, + ADVANCED: 250, + INTERMEDIATE: 100, +} as const; + function calculateScore(commits: number, prs: number, stars: number): number { - return commits * 2 + prs * 5 + stars; + return commits * SCORE_WEIGHTS.COMMITS + prs * SCORE_WEIGHTS.PRS + stars * SCORE_WEIGHTS.STARS; } function calculateLevel(score: number): "beginner" | "intermediate" | "advanced" | "expert" | "master" { - if (score >= 1000) return "master"; - if (score >= 500) return "expert"; - if (score >= 250) return "advanced"; - if (score >= 100) return "intermediate"; + if (score >= LEVEL_THRESHOLDS.MASTER) return "master"; + if (score >= LEVEL_THRESHOLDS.EXPERT) return "expert"; + if (score >= LEVEL_THRESHOLDS.ADVANCED) return "advanced"; + if (score >= LEVEL_THRESHOLDS.INTERMEDIATE) return "intermediate"; return "beginner"; }This would make the scoring algorithm more explicit and easier to adjust in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/quick-wins/columns.tsx(3 hunks)src/stores/index.ts(3 hunks)src/stores/userScores.ts(1 hunks)src/types/github.ts(1 hunks)src/types/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/quick-wins/columns.tsx (2)
src/types/quickWins.ts (1)
GitHubIssue(1-24)src/components/ui/user-hover-card.tsx (1)
UserHoverCard(31-181)
src/types/github.ts (1)
src/types/index.ts (3)
UserContributions(64-64)UserScore(65-65)UserScoreCacheEntry(66-66)
src/stores/index.ts (1)
src/stores/userScores.ts (1)
useUserScoresStore(31-138)
src/stores/userScores.ts (3)
src/types/github.ts (2)
UserScoreCacheEntry(364-368)UserScore(356-362)src/stores/index.ts (1)
useUserScoresStore(19-19)src/lib/api/github-api-client.ts (1)
githubAPIClient(1404-1404)
🔇 Additional comments (9)
src/components/quick-wins/columns.tsx (2)
33-41: LGTM! Issue number badge improves usability.The issue number display is well-implemented with proper null handling and appropriate styling. The
#{issueNumber}format matches GitHub conventions and enhances issue identification.
243-255: LGTM! UserHoverCard integration enhances author information.The integration correctly wraps the author display with hover functionality, passes the required props, and maintains the existing visual layout. The
showScore={true}prop enables the open source score feature for context on contributors.src/types/index.ts (1)
64-66: LGTM! Clean type exports.The new type exports follow the existing pattern and provide the necessary type surface for the user scores functionality.
src/stores/index.ts (3)
9-9: LGTM! Store import added.The import follows the established pattern for other stores in this file.
19-19: LGTM! Store export added.The export is correctly placed alongside other store exports.
38-38: LGTM! Hydration properly integrated.The user scores store is correctly added to the parallel hydration flow using
persist.rehydrate(), consistent with other persisted stores.src/types/github.ts (1)
350-368: LGTM! Well-structured type definitions.The new interfaces are clean and properly typed:
UserContributionscaptures the three contribution metricsUserScoreincludes a discriminated union forlevelproviding type safetyUserScoreCacheEntryappropriately wraps the score with cache metadataThe use of
numberforfetchedAt(timestamp) andstringforcalculatedAt(ISO format) aligns with the store implementation.src/stores/userScores.ts (2)
31-138: LGTM! Well-implemented Zustand store with persistence.The store implementation is solid:
- Proper cache invalidation with 30-minute TTL
- Custom storage adapter correctly handles Map serialization for localStorage
- Error handling with graceful fallback to null
- Hydration properly integrated with
onRehydrateStorage- State updates follow Zustand best practices (immutable Map updates)
The architecture cleanly separates concerns with dedicated methods for getting cached scores vs. fetching fresh data.
50-88: No action needed—getUserContributionsmethod is properly implemented and returns the expected shape.The method exists in
src/lib/api/github-api-client.ts(line 879) with the correct return typePromise<{ commits: number; prs: number; stars: number }>, which matches exactly how it's being destructured in thefetchScoremethod.
2 participants
Summary by CodeRabbit
New Features
Chores