-
Notifications
You must be signed in to change notification settings - Fork 0
🧹 Extract UserPage into smaller components #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| export default function BackgroundDecoration() { | ||
| return ( | ||
| <div className="absolute inset-0 overflow-hidden pointer-events-none fixed"> | ||
| <div className="absolute -top-[10%] -right-[10%] w-[60%] h-[60%] rounded-full bg-accent opacity-5 blur-[120px] animate-pulse-slow" /> | ||
| <div | ||
| className="absolute top-[40%] -left-[10%] w-[50%] h-[50%] rounded-full bg-success opacity-5 blur-[120px] animate-pulse-slow" | ||
| style={{ animationDelay: "2s" }} | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||
| type Props = { | ||||||
| errors: { section: string; message: string }[]; | ||||||
| }; | ||||||
|
|
||||||
| export default function ErrorMessages({ errors }: Props) { | ||||||
| if (!errors || errors.length === 0) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/[username]/components/ErrorMessages.tsx
Line: 6
Comment:
**冗長な null チェック**
`errors` の型は `{ section: string; message: string }[]`(非 nullable)と宣言されており、呼び出し元も `UserSummary` の型通りに配列を渡すため、`!errors` のチェックは不要です。型情報に合わせてシンプルにできます。
```suggestion
if (errors.length === 0) {
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| return ( | ||||||
| <div className="mb-6 space-y-2 animate-slide-up"> | ||||||
| {errors.map((err) => ( | ||||||
| <div | ||||||
| key={err.section} | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/[username]/components/ErrorMessages.tsx
Line: 14
Comment:
**重複キーによるレンダリング問題の可能性**
`err.section` をキーとして使用しているため、同じセクション名を持つエラーが複数存在する場合に React がキーの重複を警告し、リストの更新が正しく行われないことがあります。元のコードにも同じ問題がありましたが、この抽出を機に配列インデックスなど一意な値をキーとして使うことを検討してください。
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| className="rounded-md border border-danger/30 bg-danger/10 px-4 py-3 text-sm text-danger" | ||||||
| > | ||||||
| <strong>{err.section}:</strong> {err.message} | ||||||
| </div> | ||||||
| ))} | ||||||
| </div> | ||||||
| ); | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import SkillsCard from "@/components/SkillsCard"; | ||
| import ContributionsCard from "@/components/ContributionsCard"; | ||
| import ReposCard from "@/components/ReposCard"; | ||
| import ActivityCard from "@/components/ActivityCard"; | ||
| import InterestsCard from "@/components/InterestsCard"; | ||
| import AnimatedWrapper from "@/components/AnimatedWrapper"; | ||
| import { UserSummary } from "@/lib/types"; | ||
|
|
||
| type Props = { | ||
| summary: UserSummary; | ||
| }; | ||
|
|
||
| export default function UserSummaryGrid({ summary }: Props) { | ||
| return ( | ||
| <div className="mt-8 grid gap-6 lg:grid-cols-2"> | ||
| {/* Skills */} | ||
| {summary.repositories && ( | ||
| <AnimatedWrapper delay="0.2s"> | ||
| <SkillsCard repositories={summary.repositories} /> | ||
| </AnimatedWrapper> | ||
| )} | ||
|
|
||
| {/* Contributions */} | ||
| {summary.contributions && ( | ||
| <AnimatedWrapper delay="0.3s"> | ||
| <ContributionsCard contributions={summary.contributions} /> | ||
| </AnimatedWrapper> | ||
| )} | ||
|
|
||
| {/* Repos */} | ||
| {summary.repositories && ( | ||
| <AnimatedWrapper delay="0.4s"> | ||
| <ReposCard repositories={summary.repositories} /> | ||
| </AnimatedWrapper> | ||
| )} | ||
|
|
||
| {/* Interests */} | ||
| {summary.interests && ( | ||
| <AnimatedWrapper delay="0.5s"> | ||
| <InterestsCard interests={summary.interests} /> | ||
| </AnimatedWrapper> | ||
| )} | ||
|
|
||
| {/* Activity */} | ||
| {summary.activity && ( | ||
| <AnimatedWrapper delay="0.6s"> | ||
| <ActivityCard activity={summary.activity} /> | ||
| </AnimatedWrapper> | ||
| )} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The classes
absoluteandfixedare mutually exclusive and redundant. Since this is a background decoration, you should choose one based on the desired behavior:fixedto keep it static in the viewport while scrolling, orabsoluteto stay relative to the page container. Having both is confusing and relies on CSS declaration order (wherefixedtypically overridesabsolutein Tailwind).