feat: Improve empty state for LatestMergedPRs#453
Conversation
Replace the null return when there are no merged PRs with a proper empty state that matches the ossfolio design system. - Dashed border container with canvas-soft background (consistent with other empty states like ProfileReposSection) - SVG icon (circle-plus) to visually indicate the absence of data - Clear title and helpful subtitle explaining when PRs will appear - Uses the same CSS variable palette as the rest of the profile Closes PRODHOSH#317
Welcome to OSSfolio, @AliMahmoudDev! 🎉Thank you for opening this pull request and contributing to the open-source community! 🚀 To ensure a smooth review process, please make sure you have:
We will review your PR as soon as possible. Happy coding! 💻✨ |
github-actions
Bot
added
frontend
📝 WalkthroughWalkthrough
ChangesLatest merged PRs
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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 `@src/components/profile/LatestMergedPRs.tsx`:
- Around line 8-46: Refactor LatestMergedPRs to render one shared section and h2
wrapper outside the empty/populated conditional. Use an isEmpty check to select
only the inner empty-state content or merged PR list, preserving the existing
heading, layout styles, and both state-specific renderings.
- Around line 23-37: Add aria-hidden="true" to the decorative SVG element in
LatestMergedPRs so screen readers skip it while the adjacent text conveys the
empty-state message.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4d7086b2-92b7-463d-aa47-7ac4aa69ef10
📒 Files selected for processing (1)
src/components/profile/LatestMergedPRs.tsx
| if (!mergedPRs || mergedPRs.length === 0) { | ||
| return null; | ||
| return ( | ||
| <section style={{ marginTop: '32px' }}> | ||
| <h2 style={{ fontSize: '16px', fontWeight: 600, color: 'var(--color-ink)', margin: 0, marginBottom: '12px' }}> | ||
| Latest Merged Pull Requests | ||
| </h2> | ||
| <div | ||
| style={{ | ||
| padding: '40px 20px', | ||
| border: '1px dashed var(--color-hairline-strong)', | ||
| borderRadius: '12px', | ||
| textAlign: 'center', | ||
| backgroundColor: 'var(--color-canvas-soft)', | ||
| }} | ||
| > | ||
| <svg | ||
| style={{ margin: '0 auto 12px', color: 'var(--color-ink-mute-2)' }} | ||
| width="32" | ||
| height="32" | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| strokeWidth="2" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| > | ||
| <circle cx="12" cy="12" r="10" /> | ||
| <path d="M8 12h8" /> | ||
| <path d="M12 8v8" /> | ||
| </svg> | ||
| <p style={{ fontSize: '14px', fontWeight: 600, color: 'var(--color-ink)', margin: 0 }}> | ||
| No merged pull requests yet | ||
| </p> | ||
| <p style={{ fontSize: '13px', color: 'var(--color-ink-mute)', margin: '4px 0 0 0' }}> | ||
| Merged PRs will appear here once they land in the contributor's repositories. | ||
| </p> | ||
| </div> | ||
| </section> | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consolidate identical section and heading wrappers.
The <section> and <h2> elements are duplicated in both the empty state and the populated list state. Consider lifting these elements outside the conditional to simplify the component and ensure the layout and styling remain consistent.
export function LatestMergedPRs({ mergedPRs }: LatestMergedPRsProps) {
const isEmpty = !mergedPRs || mergedPRs.length === 0;
return (
<section style={{ marginTop: '32px' }}>
<h2 style={{ fontSize: '16px', fontWeight: 600, color: 'var(--color-ink)', margin: 0, marginBottom: '12px' }}>
Latest Merged Pull Requests
</h2>
{isEmpty ? (
<div
style={{
padding: '40px 20px',
border: '1px dashed var(--color-hairline-strong)',
borderRadius: '12px',
textAlign: 'center',
backgroundColor: 'var(--color-canvas-soft)',
}}
>
{/* SVG and empty state text */}
</div>
) : (
<ul style={{ listStyle: 'none', padding: 0, margin: 0 }}>
{/* list items */}
</ul>
)}
</section>
);
}🤖 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 `@src/components/profile/LatestMergedPRs.tsx` around lines 8 - 46, Refactor
LatestMergedPRs to render one shared section and h2 wrapper outside the
empty/populated conditional. Use an isEmpty check to select only the inner
empty-state content or merged PR list, preserving the existing heading, layout
styles, and both state-specific renderings.
| <svg | ||
| style={{ margin: '0 auto 12px', color: 'var(--color-ink-mute-2)' }} | ||
| width="32" | ||
| height="32" | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| strokeWidth="2" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| > | ||
| <circle cx="12" cy="12" r="10" /> | ||
| <path d="M8 12h8" /> | ||
| <path d="M12 8v8" /> | ||
| </svg> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Hide the decorative SVG from screen readers.
Adding aria-hidden="true" ensures that assistive technologies skip this purely visual element, as the adjacent text already conveys the empty state message.
🛠️ Proposed fix to hide the decorative SVG
<svg
+ aria-hidden="true"
style={{ margin: '0 auto 12px', color: 'var(--color-ink-mute-2)' }}
width="32"
height="32"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
strokeWidth="2"
strokeLinecap="round"
strokeLinejoin="round"
>📝 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.
| <svg | |
| style={{ margin: '0 auto 12px', color: 'var(--color-ink-mute-2)' }} | |
| width="32" | |
| height="32" | |
| viewBox="0 0 24 24" | |
| fill="none" | |
| stroke="currentColor" | |
| strokeWidth="2" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| > | |
| <circle cx="12" cy="12" r="10" /> | |
| <path d="M8 12h8" /> | |
| <path d="M12 8v8" /> | |
| </svg> | |
| <svg | |
| aria-hidden="true" | |
| style={{ margin: '0 auto 12px', color: 'var(--color-ink-mute-2)' }} | |
| width="32" | |
| height="32" | |
| viewBox="0 0 24 24" | |
| fill="none" | |
| stroke="currentColor" | |
| strokeWidth="2" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| > | |
| <circle cx="12" cy="12" r="10" /> | |
| <path d="M8 12h8" /> | |
| <path d="M12 8v8" /> | |
| </svg> |
🤖 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 `@src/components/profile/LatestMergedPRs.tsx` around lines 23 - 37, Add
aria-hidden="true" to the decorative SVG element in LatestMergedPRs so screen
readers skip it while the adjacent text conveys the empty-state message.
2 participants
Replaces the
return nullwhen there are no merged PRs with a proper empty state that matches the ossfolio design system.Before: The section completely disappeared when no PRs were available.
After: A styled empty state with:
var(--color-canvas-soft)background (consistent with ProfileReposSection and other empty states)Closes #317
Summary by CodeRabbit