[feat/interview-chart] - 인터뷰 목록 + 차트 컴포넌트 #13
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces parameterized track list fetching, error handling in YouTube and interview hooks, and integrates a dynamic InterviewList into chart components while refining layouts and styling.
- Refactors
useTrackListto accept an optionalplaylistId - Wraps external fetches in
try/catchwith fallback empty arrays - Adds
InterviewListcomponent toChartTop, updates chart layouts, and adjusts styling
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/hooks/useTrackList.ts | Made playlistId configurable with a default value |
| src/features/tracks/hooks/getYoutube.ts | Added error handling (try/catch) but left unreachable returns |
| src/features/tracks/hooks/getTrackIdInterview.ts | Wrapped fetch in try/catch with empty-array fallback |
| src/features/homepage/components/InterviewList.tsx | Converted to accept artistName, slice, and className props |
| src/features/chart/components/ChartTop.tsx | Integrated InterviewList, tweaked text and styling |
| src/features/chart/components/ChartComponent.tsx | Defined props interface, updated layout, replaced placeholder title |
| src/app/homepage/page.tsx | Adjusted margins, passed props to InterviewList |
| src/app/charts/page.tsx | Used useTrackList for multiple regions, cleaned up layout |
Comments suppressed due to low confidence (1)
src/features/tracks/hooks/getYoutube.ts:8
- You’ve added a
try/catchpath here—consider adding unit tests to cover error scenarios, ensuring the fallback[]behavior.
try {
|
|
||
| if (!res.ok) { | ||
| throw new Error("유튜브 검색에 실패했습니다"); | ||
| return []; |
There was a problem hiding this comment.
This return [] is unreachable because it follows a throw. Remove either the throw or the return to clarify the error path.
| return []; |
|
|
||
| if (videos.length === 0) { | ||
| throw new Error("비디오를 찾을 수 없습니다"); | ||
| return []; |
There was a problem hiding this comment.
Similarly, this return [] after a throw is unreachable and should be removed for clarity.
| return []; |
| } | ||
| return ( | ||
| <div className="relative border-2 border-black p-10 mt-10 max-w-7xl mx-auto bg-white"> | ||
| <div className="relative border-3 border-black p-10 mt-10 max-w-7xl mx-auto bg-white"> |
There was a problem hiding this comment.
Tailwind CSS does not include a border-3 utility by default; consider using border-2, border-4, or adding a custom border width.
| <div className="relative border-3 border-black p-10 mt-10 max-w-7xl mx-auto bg-white"> | |
| <div className="relative border-2 border-black p-10 mt-10 max-w-7xl mx-auto bg-white"> |
| <div> | ||
| <div className="text-lg">차트 컴포넌트</div> | ||
| <div | ||
| className={`relative border-3 border-black p-5 mt-10 max-w-7xl bg-white w-full ${className}`} |
There was a problem hiding this comment.
The border-3 class isn’t part of default Tailwind utilities; switch to an existing utility or define a custom one.
| className={`relative border-3 border-black p-5 mt-10 max-w-7xl bg-white w-full ${className}`} | |
| className={`relative border-2 border-black p-5 mt-10 max-w-7xl bg-white w-full ${className}`} |
| {tracksList.map((item, index) => ( | ||
| <div | ||
| key={index} |
There was a problem hiding this comment.
Using the array index as a React key can lead to rendering issues; prefer a stable, unique identifier if available.
| {tracksList.map((item, index) => ( | |
| <div | |
| key={index} | |
| {tracksList.map((item) => ( | |
| <div | |
| key={item.track.id} |
| const usaTracksList = await useTrackList({ | ||
| playlistId: "0TyhU3nPbWY8BNObcPXt4u", | ||
| }); | ||
| console.log("koraTracksList", koraTracksList); |
There was a problem hiding this comment.
Remove or guard this console.log to avoid leaking debug output in production.
| console.log("koraTracksList", koraTracksList); | |
| if (process.env.NODE_ENV === "development") { | |
| console.log("koraTracksList", koraTracksList); | |
| } |
| <div className="w-[1043px] mx-auto"> | ||
| <div className="flex flex-col items-center justify-start h-full"> | ||
| <main className="flex mt-[168px] gap-4 h-[617px]"> | ||
| <main className="flex mt-[186px] gap-4 h-[617px]"> |
There was a problem hiding this comment.
[nitpick] Consider replacing the magic value mt-[186px] with a semantic utility or theme-defined spacing for consistency.
| <main className="flex mt-[186px] gap-4 h-[617px]"> | |
| <main className="flex mt-custom-186 gap-4 h-[617px]"> |
No description provided.