🧹 Refactor ProfileCard to use smaller internal components#57
🧹 Refactor ProfileCard to use smaller internal components#57
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively refactors the ProfileCard component by breaking it down into smaller, more manageable internal components. This significantly improves readability and maintainability. I've added a couple of suggestions to further enhance the component's structure by improving how props are passed and how SVGs are handled, which could make the new components even more modular and easier to read.
| const ProfileMeta = ({ profile, joinDate }: { profile: UserProfile; joinDate: string }) => ( | ||
| <div className="flex flex-wrap justify-center gap-x-6 gap-y-2 text-sm text-muted sm:justify-start"> | ||
| {profile.company && ( | ||
| <span className="flex items-center gap-2 hover:text-foreground transition-colors"> | ||
| <svg className="h-4 w-4 shrink-0" fill="currentColor" viewBox="0 0 16 16"><path fillRule="evenodd" d="M1.5 14.25c0 .138.112.25.25.25H4v-1.25a.75.75 0 01.75-.75h2.5a.75.75 0 01.75.75v1.25h2.25a.25.25 0 00.25-.25V1.75a.25.25 0 00-.25-.25h-8.5a.25.25 0 00-.25.25v12.5zM1.75 0A1.75 1.75 0 000 1.75v12.5C0 15.216.784 16 1.75 16h12.5A1.75 1.75 0 0016 14.25v-8.5A1.75 1.75 0 0014.25 4H12V1.75A1.75 1.75 0 0010.25 0h-8.5zM12 5.5h2.25a.25.25 0 01.25.25v8.5a.25.25 0 01-.25.25H12V5.5zm-3 7.75v1.25h-2v-1.25H9zM3 3h2v1.5H3V3zm0 2.5h2V7H3V5.5zm0 2.5h2v1.5H3V8zm3-5h2v1.5H6V3zm0 2.5h2V7H6V5.5zm0 2.5h2v1.5H6V8z"/></svg> | ||
| {profile.company} | ||
| </span> | ||
| )} | ||
| {profile.location && ( | ||
| <span className="flex items-center gap-2 hover:text-foreground transition-colors"> | ||
| <svg className="h-4 w-4 shrink-0" fill="currentColor" viewBox="0 0 16 16"><path fillRule="evenodd" d="M11.536 3.464a5 5 0 010 7.072L8 14.07l-3.536-3.535a5 5 0 117.072-7.072v.001zm1.06 8.132a6.5 6.5 0 10-9.192 0l3.535 3.536a1.5 1.5 0 002.122 0l3.535-3.536zM8 9a2 2 0 100-4 2 2 0 000 4z"/></svg> | ||
| {profile.location} | ||
| </span> | ||
| )} | ||
| {profile.blog && ( | ||
| <a | ||
| href={profile.blog.startsWith("http") ? profile.blog : `https://${profile.blog}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="flex items-center gap-2 hover:text-accent transition-colors" | ||
| > | ||
| <svg className="h-4 w-4 shrink-0" fill="currentColor" viewBox="0 0 16 16"><path fillRule="evenodd" d="M7.775 3.275a.75.75 0 001.06 1.06l1.25-1.25a2 2 0 112.83 2.83l-2.5 2.5a2 2 0 01-2.83 0 .75.75 0 00-1.06 1.06 3.5 3.5 0 004.95 0l2.5-2.5a3.5 3.5 0 00-4.95-4.95l-1.25 1.25zm-4.69 9.64a2 2 0 010-2.83l2.5-2.5a2 2 0 012.83 0 .75.75 0 001.06-1.06 3.5 3.5 0 00-4.95 0l-2.5 2.5a3.5 3.5 0 004.95 4.95l1.25-1.25a.75.75 0 00-1.06-1.06l-1.25 1.25a2 2 0 01-2.83 0z"/></svg> | ||
| {profile.blog.replace(/^https?:\/\//, "")} | ||
| </a> | ||
| )} | ||
| <span className="flex items-center gap-2 hover:text-foreground transition-colors"> | ||
| <svg className="h-4 w-4 shrink-0" fill="currentColor" viewBox="0 0 16 16"><path fillRule="evenodd" d="M4.75 0a.75.75 0 01.75.75V2h5V.75a.75.75 0 011.5 0V2h1.25c.966 0 1.75.784 1.75 1.75v10.5A1.75 1.75 0 0113.25 16H2.75A1.75 1.75 0 011 14.25V3.75C1 2.784 1.784 2 2.75 2H4V.75A.75.75 0 014.75 0zm0 3.5h8.5a.25.25 0 01.25.25V6h-11V3.75a.25.25 0 01.25-.25h2zm-2.25 4v6.75c0 .138.112.25.25.25h10.5a.25.25 0 00.25-.25V7.5h-11z"/></svg> | ||
| Joined {joinDate} | ||
| </span> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
The ProfileMeta component contains large, inline SVG definitions which make the code verbose and harder to read. For better readability and maintainability, consider extracting these SVGs into their own dedicated components.
For example, you could create a CompanyIcon component:
// In a new file, e.g., src/components/Icons.tsx
export const CompanyIcon = (props: React.SVGProps<SVGSVGElement>) => (
<svg {...props} viewBox="0 0 16 16">
<path fillRule="evenodd" d="M1.5 14.25c0 .138.112.25.25.25H4v-1.25a.75.75 0 01.75-.75h2.5a.75.75 0 01.75.75v1.25h2.25a.25.25 0 00.25-.25V1.75a.25.25 0 00-.25-.25h-8.5a.25.25 0 00-.25.25v12.5zM1.75 0A1.75 1.75 0 000 1.75v12.5C0 15.216.784 16 1.75 16h12.5A1.75 1.75 0 0016 14.25v-8.5A1.75 1.75 0 0014.25 4H12V1.75A1.75 1.75 0 0010.25 0h-8.5zM12 5.5h2.25a.25.25 0 01.25.25v8.5a.25.25 0 01-.25.25H12V5.5zm-3 7.75v1.25h-2v-1.25H9zM3 3h2v1.5H3V3zm0 2.5h2V7H3V5.5zm0 2.5h2v1.5H3V8zm3-5h2v1.5H6V3zm0 2.5h2V7H6V5.5zm0 2.5h2v1.5H6V8z"/>
</svg>
);
// Then use it in ProfileMeta.tsx
<CompanyIcon className="h-4 w-4 shrink-0" fill="currentColor" />This approach cleans up the component's render method and makes the icons reusable.
| const ProfileStats = ({ profile }: { profile: UserProfile }) => ( | ||
| <div className="flex justify-center gap-8 pt-2 sm:justify-start"> | ||
| <div className="text-center sm:text-left"> | ||
| <div className="text-2xl font-bold text-foreground"> | ||
| {profile.followers.toLocaleString()} | ||
| </div> | ||
| <div className="text-xs uppercase tracking-wide text-muted">Followers</div> | ||
| </div> | ||
| <div className="text-center sm:text-left"> | ||
| <div className="text-2xl font-bold text-foreground"> | ||
| {profile.following.toLocaleString()} | ||
| </div> | ||
| <div className="text-xs uppercase tracking-wide text-muted">Following</div> | ||
| </div> | ||
| <div className="text-center sm:text-left"> | ||
| <div className="text-2xl font-bold text-foreground"> | ||
| {profile.public_repos.toLocaleString()} | ||
| </div> | ||
| <div className="text-xs uppercase tracking-wide text-muted">Repos</div> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
The ProfileStats component receives the entire profile object as a prop but only uses a few of its properties. To make the component more reusable and its data dependencies clearer, consider passing only the required properties as individual props. This makes the component's interface explicit and prevents passing unnecessary data.
You would then need to update the call site in ProfileCard like this:
<ProfileStats followers={profile.followers} following={profile.following} public_repos={profile.public_repos} />
const ProfileStats = ({
followers,
following,
public_repos,
}: {
followers: number;
following: number;
public_repos: number;
}) => (
<div className="flex justify-center gap-8 pt-2 sm:justify-start">
<div className="text-center sm:text-left">
<div className="text-2xl font-bold text-foreground">
{followers.toLocaleString()}
</div>
<div className="text-xs uppercase tracking-wide text-muted">Followers</div>
</div>
<div className="text-center sm:text-left">
<div className="text-2xl font-bold text-foreground">
{following.toLocaleString()}
</div>
<div className="text-xs uppercase tracking-wide text-muted">Following</div>
</div>
<div className="text-center sm:text-left">
<div className="text-2xl font-bold text-foreground">
{public_repos.toLocaleString()}
</div>
<div className="text-xs uppercase tracking-wide text-muted">Repos</div>
</div>
</div>
);
|
This PR is being closed as superseded by #69. The related UI and component refactors were consolidated there so the combined frontend cleanup can be reviewed and validated together. |
🎯 What: The
ProfileCardcomponent was quite long and contained many different parts of the UI, such as the Avatar, Meta information, Statistics, Organizations, and Pinned Repositories. These have been extracted into smaller internal functional components.💡 Why: This improves the maintainability and readability of the
ProfileCard.tsxfile by separating concerns and reducing the complexity of the mainProfileCardcomponent rendering function.✅ Verification: Verified by checking types (
npx tsc --noEmit), linting (npm run lint), passing all tests (npm run test), and verifying the build process (npm run build).✨ Result: The
ProfileCardcomponent is much shorter and easier to digest at a glance, with each part encapsulated in its own smaller component within the same file.PR created automatically by Jules for task 5444042468145380957 started by @is0692vs