Feat/revamp member profile page#4797
Conversation
… components; remove Closed from Milestones
Summary by CodeRabbit
WalkthroughThis PR restructures the member profile page layout by introducing a new ChangesMember Profile Layout Restructure with Supporting Component Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Milestones.tsx (1)
41-42: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider adding a defensive null check for
openIssuesCount.The component uses null checks for other potentially nullable fields (
item.createdAton line 38,item?.repositoryNameon line 44). For consistency and defensive programming, consider adding a fallback foropenIssuesCountin case it's undefined or null:🛡️ Suggested defensive check
<div className="mr-4 flex items-center"> <FaCircleExclamation className="mr-2 h-4 w-4" /> - <span>{item.openIssuesCount} open</span> + <span>{item.openIssuesCount ?? 0} open</span> </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Milestones.tsx` around lines 41 - 42, The JSX rendering in Milestones.tsx uses item.openIssuesCount without a null check; update the span that displays "{item.openIssuesCount} open" to defensively handle undefined/null (e.g., use a fallback like item.openIssuesCount ?? 0 or conditional rendering) so it always renders a sensible value; locate the usage of item.openIssuesCount near the FaCircleExclamation element and replace it with the null-safe expression to preserve UI and avoid rendering "undefined".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/members/`[memberKey]/page.tsx:
- Around line 26-28: UserSummary currently casts away a nullable prop
(UserSummaryProps.user) before passing to MemberDetailSidebar which can crash;
either change the UserSummaryProps type to require user: User (remove null) so
UserSummary and its export are safe, or keep the nullable type and add a runtime
guard in the UserSummary function to handle null (e.g., return null/placeholder)
before calling MemberDetailSidebar; update the UserSummary signature and any
callers (notably the existing guarded call can remain) to match the tightened
type if you choose the type-change approach.
---
Outside diff comments:
In `@frontend/src/components/Milestones.tsx`:
- Around line 41-42: The JSX rendering in Milestones.tsx uses
item.openIssuesCount without a null check; update the span that displays
"{item.openIssuesCount} open" to defensively handle undefined/null (e.g., use a
fallback like item.openIssuesCount ?? 0 or conditional rendering) so it always
renders a sensible value; locate the usage of item.openIssuesCount near the
FaCircleExclamation element and replace it with the null-safe expression to
preserve UI and avoid rendering "undefined".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0b4f4f6-3bd7-451d-8c1f-49bba393a212
📒 Files selected for processing (11)
frontend/src/app/members/[memberKey]/page.tsxfrontend/src/components/ContributionHeatmap.tsxfrontend/src/components/InfoBlock.tsxfrontend/src/components/ItemCardList.tsxfrontend/src/components/Milestones.tsxfrontend/src/components/RecentReleases.tsxfrontend/src/components/Release.tsxfrontend/src/components/RepositoryCard.tsxfrontend/src/components/cards/MemberDetailSidebar.tsxfrontend/src/components/cards/RepositoriesModules.tsxfrontend/src/components/skeletons/MemberDetailsPageSkeleton.tsx
kasya
left a comment
There was a problem hiding this comment.
I left some requests below, but please address Code Rabbit comment before going over these. Thanks!
…tributionHeatmap responsiveness and scaling
…ti-column design with conditional styling
… UI layout consistency
…an up unused imports
b2b2c40
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/__tests__/unit/pages/UserDetails.test.tsx (1)
398-404:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix fixture shape to match the page’s GraphQL contract.
This test sets
recentIssuesunderuser, butUserDetailsPagereadsgraphQLData.recentIssues(top-level). The current assertion can pass without exercising the intended empty-issues path.Suggested patch
test('handles no recent issues gracefully', async () => { const noIssuesData = { + ...mockUserDetailsData, + recentIssues: [], user: { ...mockUserDetailsData.user, recentIssues: {} }, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/__tests__/unit/pages/UserDetails.test.tsx` around lines 398 - 404, The test fixture in the handles no recent issues gracefully test incorrectly nests recentIssues under the user object, but UserDetailsPage reads graphQLData.recentIssues at the top level. Restructure the noIssuesData object so that recentIssues is a top-level property (at the same level as user) set to an empty object, matching the GraphQL contract that the page component expects. This ensures the test actually exercises the empty-issues code path rather than just passing without validating the intended behavior.frontend/__tests__/a11y/components/ContributionHeatmap.a11y.test.tsx (1)
88-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the same chart-ready wait in the second a11y test to avoid timing flakes.
Line 88 through Line 93 runs
axe(container)without waiting for chart mount, while the previous test now waits explicitly. Make both tests consistent so the a11y check runs against fully rendered markup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/__tests__/a11y/components/ContributionHeatmap.a11y.test.tsx` around lines 88 - 93, The second a11y test (the one checking accessibility violations when title is provided) is running the axe accessibility check without waiting for the chart component to fully mount, which differs from the previous test and can cause timing flakes. Add an explicit wait for the chart to be ready before calling axe(container) in this test to match the behavior of the first test and ensure the a11y check runs against fully rendered markup.frontend/src/components/ContributionHeatmap.tsx (1)
340-360:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender only one chart instance to avoid duplicate heavy work.
Line 340 through Line 360 currently mounts two
Chartcomponents and hides one via CSS. Both still render and execute, which doubles chart work on every update and can degrade interaction on the member page.Suggested direction
- <div className="mx-auto w-max px-2 md:hidden"> - <Chart - height={getChartHeight()} - options={options} - series={heatmapSeries} - type="heatmap" - width={chartWidth} - /> - </div> - <div - className="hidden w-full px-2 md:block" - style={{ aspectRatio: `${chartWidth} / ${getChartHeight()}` }} - > - <Chart - height="100%" - options={options} - series={heatmapSeries} - type="heatmap" - width="100%" - /> - </div> + <div className="mx-auto w-max min-w-full px-2"> + <Chart + height={getChartHeight()} + options={options} + series={heatmapSeries} + type="heatmap" + width={chartWidth} + /> + </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ContributionHeatmap.tsx` around lines 340 - 360, The ContributionHeatmap component is rendering two Chart instances simultaneously and hiding one via CSS, which causes duplicate rendering work. Instead of using two separate Chart components with CSS display toggles (md:hidden and md:block), use a responsive hook like useMediaQuery or a custom screen size detection hook to conditionally render only a single Chart component based on the current screen size. This approach will ensure only one Chart instance is mounted and executed at any given time, eliminating the duplicate heavy work on every update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/__tests__/a11y/components/ContributionHeatmap.a11y.test.tsx`:
- Around line 88-93: The second a11y test (the one checking accessibility
violations when title is provided) is running the axe accessibility check
without waiting for the chart component to fully mount, which differs from the
previous test and can cause timing flakes. Add an explicit wait for the chart to
be ready before calling axe(container) in this test to match the behavior of the
first test and ensure the a11y check runs against fully rendered markup.
In `@frontend/__tests__/unit/pages/UserDetails.test.tsx`:
- Around line 398-404: The test fixture in the handles no recent issues
gracefully test incorrectly nests recentIssues under the user object, but
UserDetailsPage reads graphQLData.recentIssues at the top level. Restructure the
noIssuesData object so that recentIssues is a top-level property (at the same
level as user) set to an empty object, matching the GraphQL contract that the
page component expects. This ensures the test actually exercises the
empty-issues code path rather than just passing without validating the intended
behavior.
In `@frontend/src/components/ContributionHeatmap.tsx`:
- Around line 340-360: The ContributionHeatmap component is rendering two Chart
instances simultaneously and hiding one via CSS, which causes duplicate
rendering work. Instead of using two separate Chart components with CSS display
toggles (md:hidden and md:block), use a responsive hook like useMediaQuery or a
custom screen size detection hook to conditionally render only a single Chart
component based on the current screen size. This approach will ensure only one
Chart instance is mounted and executed at any given time, eliminating the
duplicate heavy work on every update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c293484-83b4-46a5-acbc-603d63bd8226
📒 Files selected for processing (15)
frontend/__tests__/a11y/components/ContributionHeatmap.a11y.test.tsxfrontend/__tests__/a11y/components/RepositoryCard.a11y.test.tsxfrontend/__tests__/unit/components/ContributionHeatmap.test.tsxfrontend/__tests__/unit/components/InfoBlock.test.tsxfrontend/__tests__/unit/components/Milestones.test.tsxfrontend/__tests__/unit/components/RecentRelease.test.tsxfrontend/__tests__/unit/components/RepositoryCard.test.tsxfrontend/__tests__/unit/pages/Home.test.tsxfrontend/__tests__/unit/pages/OrganizationDetails.test.tsxfrontend/__tests__/unit/pages/ProjectDetails.test.tsxfrontend/__tests__/unit/pages/RepositoryDetails.test.tsxfrontend/__tests__/unit/pages/UserDetails.test.tsxfrontend/src/app/members/[memberKey]/page.tsxfrontend/src/components/ContributionHeatmap.tsxfrontend/src/components/cards/MemberDetailSidebar.tsx
💤 Files with no reviewable changes (6)
- frontend/tests/unit/pages/Home.test.tsx
- frontend/tests/unit/pages/OrganizationDetails.test.tsx
- frontend/tests/unit/pages/RepositoryDetails.test.tsx
- frontend/tests/unit/components/RecentRelease.test.tsx
- frontend/tests/unit/pages/ProjectDetails.test.tsx
- frontend/tests/unit/components/Milestones.test.tsx
|
There was a problem hiding this comment.
2 issues found across 15 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
@kasya Below are the ScreenRecording of the Updated Behaviour of the Heatmap and the Member Details Comment-1 HeatmapC-1-Heatmap.mp4Comment-2 Member DetailC-2-Member_Sidebar.mp4 |



Proposed change
Resolves #4339
This PR updates the member profile page layout to follow a structure similar to GitHub’s profile pages, providing a clearer hierarchy for users' contributions, repositories, and activities.
Key changes include:
MemberDetailSidebaras a clearly defined left sidebar for user details and stats.MemberDetailsPageSkeletonto match the new UI's two-column design (sidebar on the left, content and heatmap on the right) ensuring a smooth transition during loading.Screenshots
Checklist
make check-testlocally: all warnings addressed, tests passed