Conversation
WalkthroughYouTube 플레이어 제어를 PlayerProvider로 통합하고, 플레이리스트/진행바 시간 계산 방식을 누적 시간 모델로 전환했습니다. Discover/MyCD 페이지는 Provider의 playerRef와 제어 콜백을 사용하도록 리팩터링되었고, 상세 페이지 UI는 공용 PlaylistInfo 컴포넌트로 통합되었습니다. 정적 JSON 목업 데이터가 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant D as Discover/MyCD Page
participant PP as PlayerProvider (Context)
participant YT as YouTube Player
U->>D: 트랙 선택/재생 토글
D->>PP: setPlaylist(...) / play() / pause()
PP->>PP: 상태 업데이트(isPlaying, currentIndex)
PP-->>YT: playerRef.playVideo()/pauseVideo()/seekTo()
YT-->>PP: onStateChange(ENDED)
PP->>PP: nextTrack()
PP-->>YT: seekTo(0) & playVideo()
note over PP,YT: handlePlayerStateChange로 종료 시 다음 트랙 진행
sequenceDiagram
autonumber
participant PB as ProgressBar
participant PP as PlayerProvider
participant YT as YouTube Player
loop 매 1초 (isPlaying일 때)
PB->>YT: playerRef.getCurrentTime()
YT-->>PB: 현재 트랙 경과 시간
PB->>PP: updateCurrentTime(현재 트랙 시간)
PB->>PB: accTime = getAccTime(trackLengths, currentIndex, currentTime)
PB->>PB: 진행률 = accTime / duration
end
note over PB: 누적 시간 기반 진행 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
Summary of ChangesHello @maylh, 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! 이 PR은 '둘러보기' 및 '나의 CD' 기능 전반에 걸쳐 플레이리스트 및 플레이어 관련 코드의 리팩토링을 목표로 합니다. 주요 변경 사항은 플레이어 제어 로직을 Highlights
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
|
|
🎵 Storybook Link 🎵 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 둘러보기와 나의 CD 페이지의 코드 구조를 개선하는 중요한 리팩토링을 담고 있습니다. 중복되던 플레이어 제어 로직을 PlayerProvider로 통합하고, UI를 PlaylistInfo 위젯으로 분리하여 코드의 재사용성과 유지보수성을 크게 향상시킨 점이 인상적입니다. 전반적으로 훌륭한 리팩토링입니다.
다만, 리팩토링이 discover 페이지에는 잘 적용되었지만 mycd 페이지에는 일부 이전 로직이 남아있는 것으로 보입니다. 예를 들어 onSelectTrack 핸들러나 isCurrentlyPlaying 계산 로직이 discover 페이지와 다르게 구현되어 있습니다. 전체적인 일관성을 위해 이 부분도 함께 검토해보시면 좋을 것 같습니다.
추가적인 성능 최적화 및 코드 개선을 위해 아래에 몇 가지 의견을 남깁니다.
| const setPlaylist = (playlist: PlaylistInfo, trackIndex?: number, time?: number) => { | ||
| setCurrentPlaylist(playlist) | ||
| if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex) | ||
| if (time !== undefined) setCurrentTime(time) | ||
| setIsPlaying(true) | ||
|
|
||
| if (playerRef.current) { | ||
| if (time !== undefined) playerRef.current.seekTo(time, true) | ||
| playerRef.current.playVideo() | ||
| } | ||
| } |
There was a problem hiding this comment.
setPlaylist 함수를 useCallback으로 감싸서 불필요한 리렌더링을 방지하는 것이 좋습니다. 현재 구현에서는 PlaylistProvider가 리렌더링될 때마다 새로운 setPlaylist 함수가 생성되어, 이를 사용하는 자식 컴포넌트들도 함께 리렌더링될 수 있습니다. setState 함수와 ref는 항상 동일한 참조를 유지하므로 의존성 배열은 비워두어도 괜찮습니다. 1
| const setPlaylist = (playlist: PlaylistInfo, trackIndex?: number, time?: number) => { | |
| setCurrentPlaylist(playlist) | |
| if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex) | |
| if (time !== undefined) setCurrentTime(time) | |
| setIsPlaying(true) | |
| if (playerRef.current) { | |
| if (time !== undefined) playerRef.current.seekTo(time, true) | |
| playerRef.current.playVideo() | |
| } | |
| } | |
| const setPlaylist = useCallback((playlist: PlaylistInfo, trackIndex?: number, time?: number) => { | |
| setCurrentPlaylist(playlist) | |
| if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex) | |
| if (time !== undefined) setCurrentTime(time) | |
| setIsPlaying(true) | |
| if (playerRef.current) { | |
| if (time !== undefined) playerRef.current.seekTo(time, true) | |
| playerRef.current.playVideo() | |
| } | |
| }, []) |
Style Guide References
Footnotes
| const updateCurrentTime = (time: number) => { | ||
| setCurrentTime(time) | ||
| } | ||
| const updateCurrentTime = (time: number) => setCurrentTime(time) |
There was a problem hiding this comment.
updateCurrentTime 함수도 useCallback으로 감싸서 매 렌더링마다 함수가 새로 생성되는 것을 방지하는 것이 좋습니다. 이는 setPlaylist와 마찬가지로 성능 최적화에 도움이 됩니다. 1
| const updateCurrentTime = (time: number) => setCurrentTime(time) | |
| const updateCurrentTime = useCallback((time: number) => setCurrentTime(time), []) |
Style Guide References
Footnotes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/widgets/playlist/ProgressBar.tsx (1)
53-76: 바 끝지점 클릭 시 트랙 계산 오류 (마지막 트랙 선택 불가)seekTime === duration인 경우 루프를 빠져나와 trackIndex가 0으로 남습니다. 경계값 포함/마지막 트랙 대입 처리가 필요합니다.
적용 예시:
const handleBarClick = (e: React.MouseEvent<HTMLDivElement, MouseEvent>) => { @@ - for (let i = 0; i < trackLengths.length; i++) { - currentTotal += trackLengths[i] - if (seekTime < currentTotal) { - trackIndex = i - // 해당 곡 내에서의 시간을 계산 - localTime = seekTime - (currentTotal - trackLengths[i]) - break - } - } + for (let i = 0; i < trackLengths.length; i++) { + const len = trackLengths[i] + currentTotal += len + const hit = seekTime <= currentTotal || i === trackLengths.length - 1 + if (hit) { + trackIndex = i + localTime = Math.min(Math.max(seekTime - (currentTotal - len), 0), len) + break + } + }src/pages/mycd/index.tsx (1)
94-98: 중복 seek/play 제거 — setPlaylist가 이미 seek 및 재생을 수행합니다setPlaylist(playlist, trackIndex, time)는 time이 주어지면 playerRef.current.seekTo(time, true)와 playerRef.current.playVideo()를 호출합니다. 따라서 src/pages/mycd/index.tsx의 onSelectTrack(약 94–98행)에서 아래 두 줄을 삭제하거나 통합하세요:
- if (time !== undefined) playerRef.current?.seekTo(time, true)
- if (!isPlaying) play()
src/pages/discover/index.tsx (2)
103-113: 초기 플레이리스트 세팅이 지연/누락될 수 있음 (isReady 조건 과도).현재 isReady가 playlistAsInfo와 shuffleData 모두 준비되어야 true입니다. URL 파라미터가 없고 셔플 데이터만 준비된 경우 초기화가 영원히 실행되지 않을 수 있습니다. 둘 중 하나만 준비돼도 초기화 가능하도록 완화하세요.
- const isReady = !!playlistAsInfo && shuffleData !== undefined + const isReady = shuffleData !== undefined || !!playlistAsInfo
54-55: usePlaylistDetail 훅: playlistId가 없을 때 NaN로 API 호출되는 문제 — 훅 수정 필요검증 결과: src/entities/playlist/model/usePlaylists.ts의 usePlaylistDetail(playlistId: number)이 options를 받지 않고 바로 useQuery를 호출하므로 Number(playlistId)가 NaN일 때 불필요한 API 요청/에러가 발생합니다.
- 권장(간단): 훅 내부에서 enabled 검증 추가(예: enabled: Number.isFinite(playlistId) 또는 Number.isInteger(playlistId) && playlistId >= 0)로 요청 차단 — 수정 파일: src/entities/playlist/model/usePlaylists.ts.
- 대안: 훅 시그니처를 (playlistId: number, options?: UseQueryOptions)로 변경해 options를 useQuery에 전달하고 호출부에서 playlistIdNum + enabled로 가드(호출부 변경 필요: src/pages/discover/index.tsx:54, src/pages/discover/playlist/index.tsx:8, src/pages/mycd/playlist/index.tsx:9).
- 현재 훅은 options.enabled를 지원하지 않으므로 훅 내부 검증 또는 시그니처 변경 중 하나를 적용해야 합니다.
🧹 Nitpick comments (15)
vercel.json (1)
2-4: productionBranch가 실제 배포 브랜치와 일치하는지 확인 필요Vercel Production 배포 트리거 브랜치가 실제 운영 브랜치가 'main'인지 확인해주세요. 운영이 'develop' 또는 다른 브랜치라면 예기치 않은 배포 중단이 발생할 수 있습니다.
src/shared/ui/Link.tsx (1)
49-52: 썸네일 고정(flex-shrink: 0)과 제목 100% 폭 적용은 적절합니다 + 텍스트 수축 허용 권장Flex 컨텍스트에서 텍스트 줄바꿈/클램프가 확실히 동작하도록 TextBox에 min-width: 0 추가를 권장합니다.
const TextBox = styled.div` display: flex; align-items: center; + min-width: 0; `Also applies to: 67-70
src/app/providers/PlayerProvider.tsx (3)
40-43: setPlaylist 내 즉시 재생 호출의 준비 상태 확인playerRef.current 존재만으로는 재생 준비(READY) 보장이 없습니다. isReady 게이트를 이미 상위에서 사용 중이라면 OK입니다. 아니라면 READY 이벤트 이후로 지연 실행하거나, 실패 시 isPlaying 롤백이 필요합니다.
- if (playerRef.current) { - if (time !== undefined) playerRef.current.seekTo(time, true) - playerRef.current.playVideo() - } + if (playerRef.current) { + if (time !== undefined) playerRef.current.seekTo(time, true) + try { + playerRef.current.playVideo() + } finally { + // 실패 시 UI 불일치 방지 + // (실패 콜백이 없다면 onStateChange에서 동기화하는 편이 더 견고) + } + }
74-79: 플레이어 상태 동기화 범위 확장 권장ENDED만 처리하면 UI와 실제 플레이어 상태가 어긋날 수 있습니다. PLAYING/PAUSED도 동기화하세요. window.YT 대신 전역 YT 사용으로도 충분합니다.
- const handlePlayerStateChange = useCallback( - (event: YT.OnStateChangeEvent) => { - if (event.data === window.YT.PlayerState.ENDED) nextTrack() - }, - [nextTrack] - ) + const handlePlayerStateChange = useCallback( + (event: YT.OnStateChangeEvent) => { + switch (event.data) { + case YT.PlayerState.ENDED: + nextTrack() + break + case YT.PlayerState.PLAYING: + setIsPlaying(true) + break + case YT.PlayerState.PAUSED: + setIsPlaying(false) + break + default: + break + } + }, + [nextTrack] + )
81-96: Context value 메모이제이션/함수 안정화로 리렌더 최소화value 객체를 useMemo로 감싸고 setPlaylist/updateCurrentTime도 useCallback으로 안정화하면 소비자 리렌더를 줄일 수 있습니다.
-import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react' +import { createContext, useState, useContext, useRef, useCallback, useMemo, type ReactNode } from 'react' - const setPlaylist = (playlist: PlaylistInfo, trackIndex?: number, time?: number) => { + const setPlaylist = useCallback((playlist: PlaylistInfo, trackIndex?: number, time?: number) => { setCurrentPlaylist(playlist) if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex) if (time !== undefined) setCurrentTime(time) setIsPlaying(true) if (playerRef.current) { if (time !== undefined) playerRef.current.seekTo(time, true) playerRef.current.playVideo() } - } + }, []) - const updateCurrentTime = (time: number) => setCurrentTime(time) + const updateCurrentTime = useCallback((time: number) => setCurrentTime(time), []) - const value = { + const value = useMemo(() => ({ currentPlaylist, currentTrackIndex, currentTime, isPlaying, setPlaylist, play, pause, nextTrack, prevTrack, updateCurrentTime, playerRef, handlePlayerStateChange, - } + }), [ + currentPlaylist, + currentTrackIndex, + currentTime, + isPlaying, + setPlaylist, + play, + pause, + nextTrack, + prevTrack, + updateCurrentTime, + handlePlayerStateChange, + ])src/widgets/playlist/PlaylistLayout.tsx (2)
17-31: 불필요한 props 제거 (currentTime)현재 컴포넌트 내에서 currentTime을 사용하지 않습니다. 인터페이스/props에서 제거해 API 표면을 줄이세요.
적용 예시:
interface PlaylistSlideProps { data: PlaylistInfo currentPlaylist: PlaylistInfo | null currentTrackIndex: number - currentTime: number isPlaying: boolean onPlayPause: () => void onNext: () => void onPrev: () => void onSelectTrack: (trackIndex: number, time?: number) => void playerRef: React.RefObject<YT.Player | null> isMuted: boolean | null setIsMuted: React.Dispatch<React.SetStateAction<boolean | null>> type?: 'My' | 'Discover' }
64-101: 중복 계산 값 추출 및 경미한 정리stickers 계산이 두 곳에서 반복됩니다. 한 번만 계산해 사용하세요. 또한 Number 캐스팅이 불필요합니다.
적용 예시:
return ( <> <Header center={ <> - <span>{data.playlistName}</span> - <span>{getTrackOrderLabel(Number(currentTrackIndex))}</span> + <span>{data.playlistName}</span> + <span>{getTrackOrderLabel(currentTrackIndex)}</span> </> } /> <Container> @@ <Wrapper> + {/* + 중복 방지: stickers 한 번만 계산 + */} + {/* + 컴포넌트 상단으로 올려도 됨 + */} + {(() => { + /* no-op IIFE for diff context */ + })()} + {/* 실제 코드에선 아래 한 줄만 상단에 선언하세요: + const stickers = data?.cdItems ?? data?.onlyCdResponse?.cdItems ?? [] + */} <CdWrapper> {isMobile && ( <VolumeButton playerRef={playerRef} isMuted={isMuted} setIsMuted={setIsMuted} /> )} <Cd variant="xxl" bgColor="none" - stickers={data?.cdItems ?? data?.onlyCdResponse?.cdItems ?? []} + stickers={data?.cdItems ?? data?.onlyCdResponse?.cdItems ?? []} /> </CdWrapper> <ActionBar playlistId={data.playlistId} isFollowing={!!isFollowing} userName={data.creator.creatorNickname} showFollow={type !== 'My'} creatorId={data.creator.creatorId} - stickers={data?.cdItems ?? data?.onlyCdResponse?.cdItems ?? []} + stickers={data?.cdItems ?? data?.onlyCdResponse?.cdItems ?? []} /> </Wrapper>실제 반영 시에는 상단에
const stickers = data?.cdItems ?? data?.onlyCdResponse?.cdItems ?? []선언 후 Cd/ActionBar 모두stickers={stickers}로 변경하세요.src/pages/mycd/index.tsx (2)
90-92: 재생 토글 기준 일치화 (상태/플레이어 싱크 미스매치 완화)표시 상태는 isCurrentlyPlaying을 쓰는데, 토글은 isPlaying으로 판단합니다. 동일 기준으로 통일하세요.
적용 예시:
- onPlayPause={() => (isPlaying ? pause() : play())} + onPlayPause={() => (isCurrentlyPlaying ? pause() : play())}
54-59: 플레이어 상태 콜백: Provider의 공용 핸들러 사용 고려Provider가
handlePlayerStateChange를 노출한다면 재사용하여 페이지별 편차를 줄이세요.적용 예시:
- const handlePlayerStateChange = useCallback( - (event: YT.OnStateChangeEvent) => { - if (event.data === window.YT.PlayerState.ENDED) nextTrack() - }, - [nextTrack] - ) + // const { handlePlayerStateChange } = usePlaylist()src/pages/mycd/playlist/index.tsx (1)
4-4: import 일관화 (바렐 사용 권장)동일 컴포넌트를 다른 페이지는 바렐에서 named import, 이 파일은 파일 경로 default import를 사용 중입니다. 일관성을 위해 바렐에서 가져오세요.
적용 예시:
-import PlaylistInfo from '@/widgets/playlist/PlaylistInfo' +import { PlaylistInfo } from '@/widgets/playlist'src/widgets/playlist/ProgressBar.tsx (1)
25-34: 타임 폴링 조건 보완 제안일시정지 상태에서 외부 시킹(마커/바 클릭) 후 즉시 UI 반영이 늦을 수 있습니다. isPlaying이 false여도 playerRef 이벤트/상태 변경에 따라 한번 업데이트 하는 로직을 고려해 주세요.
src/widgets/playlist/PlaylistInfo.tsx (2)
9-9: 글로벌 Error/Link 식별자 음영(섀도잉) 해소 및 명확한 네이밍.Biome 경고(lint/suspicious/noShadowRestrictedNames)처럼 Error 식별자를 음영합니다. 또한 Link는 react-router-dom의 Link와 혼동될 소지가 큽니다. 별칭을 권장합니다.
-import { Error, Header, Link, Loading, SvgButton } from '@/shared/ui' +import { Error as UiError, Header, Link as UiLink, Loading, SvgButton } from '@/shared/ui'- <Error /> + <UiError />- <Link key={index} data={track} variant="large" /> + <UiLink key={index} data={track} variant="large" />Also applies to: 24-24, 53-53
51-54: 리스트 key로 index 사용 지양.재정렬/삽입 시 불필요한 재마운트가 발생할 수 있습니다. 트랙의 고유 id(예: songId, id, youtubeUrl 등)로 교체하세요.
- playlistData.songs.map((track, index) => ( - <UiLink key={index} data={track} variant="large" /> + playlistData.songs.map((track) => ( + <UiLink key={track.id ?? track.songId ?? track.youtubeUrl} data={track} variant="large" /> ))}트랙 객체에 어떤 고유 식별자가 있는지 확인 부탁드립니다.
src/pages/discover/index.tsx (2)
123-129: 주석과 구현 불일치(5초 vs 10초).주석은 “10초마다”인데 실제 interval은 5000ms입니다. 혼선을 막기 위해 주석을 맞추거나 주기를 10초로 변경하세요.
- // 재생 중일 때 10초마다 refetch + // 재생 중일 때 5초마다 refetch const viewCountTimer = setInterval(() => { if (isPlaying) { refetchViewCounts() } - }, 5000) + }, 5000)
163-164: onPlayPause 핸들러 안정화(불필요한 재생성 방지).자식 리렌더를 줄이려면 useCallback으로 래핑하세요.
- isPlaying={isPlaying} - onPlayPause={() => (isPlaying ? pause() : play())} + isPlaying={isPlaying} + onPlayPause={useCallback(() => (isPlaying ? pause() : play()), [isPlaying, pause, play])}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/app/providers/PlayerProvider.tsx(5 hunks)src/pages/discover/commentData.json(0 hunks)src/pages/discover/index.tsx(5 hunks)src/pages/discover/playlist/index.tsx(1 hunks)src/pages/mycd/index.tsx(3 hunks)src/pages/mycd/myPlaylist.json(0 hunks)src/pages/mycd/playlist/index.tsx(1 hunks)src/shared/lib/getAccTime.ts(1 hunks)src/shared/lib/index.ts(1 hunks)src/shared/ui/Link.tsx(2 hunks)src/widgets/playlist/PlaylistInfo.tsx(1 hunks)src/widgets/playlist/PlaylistLayout.tsx(2 hunks)src/widgets/playlist/ProgressBar.tsx(4 hunks)src/widgets/playlist/index.ts(1 hunks)vercel.json(1 hunks)
💤 Files with no reviewable changes (2)
- src/pages/discover/commentData.json
- src/pages/mycd/myPlaylist.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
**/*.{ts,tsx,js,jsx}: ## 1. 일반적인 코딩 컨벤션포맷팅
.prettierrc설정에 따라 포맷팅 확인- 들여쓰기: 2칸 스페이스
- 최대 줄 길이: 100자
- 세미콜론 사용 안함
- 따옴표: 작은따옴표 사용
- 괄호 안 공백: 있음
- 화살표 함수 괄호: 항상 사용
- 줄바꿈: LF 사용
네이밍 컨벤션
- 컴포넌트: PascalCase (예: UserProfile)
- 유틸리티/훅/변수: camelCase (예: getUserData, useUserInfo)
- 상수: UPPER_SNAKE_CASE (예: API_BASE_URL)
- 이미지 파일: kebab-case (예: user-profile-icon.png)
주석 사용
- 복잡한 로직에만 주석 추가
- 불필요한 주석 지양 (코드로 설명 가능한 것)
- TODO/FIXME 형식:
// TODO: 설명 - 작성자가독성
- 매직 넘버 지양, 의미있는 상수 사용
- 함수는 하나의 책임만 가지도록 작성 (최대 20줄 권장)
- 중첩 깊이 최소화 (3단계 이하 권장)
2. React 모범 사례
컴포넌트 작성
- 최신 React hooks 사용 권장
- 컴포넌트는 단일 책임 원칙 준수
- Presentational/Container 컴포넌트 분리
- 성능 최적화: memo, useCallback, useMemo 적절히 사용
- 대용량 리스트는 가상화 라이브러리 사용 고려
상태 관리
- Zustand와 Tanstack Query를 일관되게 사용
- 상태 구조는 정규화된 형태로 관리
- 에러 처리: Error Boundary와 try-catch 또는 onError 콜백 활용
3. 스타일링
Styled Components
- Styled Components 일관되게 사용
- 스타일드 컴포넌트명은 의미있게 작성
- 동적 스타일링은 props나 CSS 변수 활용
- 테마 시스템 활용하여 글로벌 스타일 관리
- 재사용 가능한 스타일은 mixin이나 확장으로 관리
- CSS 포맷팅 가독성 유지
- 사용하지 않는 스타일이나 중복 스타일 제거
4. Vite 및 빌드 최적화
- 모듈 import 최적화 (tree-shaking 고려)
- 환경변수는 .env 파일로 관리
- vite.config.ts에서 빌드 성능 튜닝 (sourcemap 설정, 플러그인 최적화 등)
5. 아키텍처 및 개발 환경
폴더 구조
- Feature-Sliced Design (FSD) 구조 준수
- 레이어별 참조 규칙 엄격히 적용
타입스크립트
- strict 모드 사용
- 타입 명시적으로 작성 (any 사용 지양)
- path alias (@/_) 절대 경로 import 사용
Git 훅
- Husky 설정으로 pre-commit, commit-msg 린팅 확인
6. 기타 가이드라인
- 충분한 근거와 함께 리뷰 제공
- 정보 검증 후 답변
- 간결하고 명확한 응답
- 필요시 추가 컨텍스트 요청
- 검증되지 않은 주장 지양
- 가능한 경우 출처 명시
- 별도 언급 없으면 JavaScript 기준
- 한국어로 응답
- 대부분 브라우저에서 지원하는 ES6+ 기능 활용
- 코드 리뷰를 통한 유지보수성 향상에 적극 활용
Files:
src/widgets/playlist/index.tssrc/shared/lib/index.tssrc/shared/ui/Link.tsxsrc/shared/lib/getAccTime.tssrc/app/providers/PlayerProvider.tsxsrc/pages/mycd/index.tsxsrc/pages/discover/playlist/index.tsxsrc/pages/mycd/playlist/index.tsxsrc/widgets/playlist/PlaylistInfo.tsxsrc/widgets/playlist/ProgressBar.tsxsrc/pages/discover/index.tsxsrc/widgets/playlist/PlaylistLayout.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: maylh
PR: dnd-side-project/dnd-13th-8-frontend#55
File: src/widgets/playlist/BackgroundPlayer.tsx:55-83
Timestamp: 2025-08-20T05:59:48.729Z
Learning: BackgroundPlayer 컴포넌트에서 네비게이션 후 컨트롤 버튼이 작동하지 않는 버그는 주로 다음 원인들로 발생한다: 1) DOM ID 충돌 (고정값 'yt-player' 사용), 2) window.YT가 이미 존재할 때 새 플레이어를 생성하지 않음, 3) onStateChange 의존성으로 인한 useEffect 재실행과 전역 콜백 덮어쓰기, 4) 리마운트 시 ref 상태 불일치. 해결책으로는 고유 DOM ID 생성, initPlayer 함수 분리를 통한 즉시 초기화, 콜백 ref 사용으로 의존성 최소화, 플레이리스트 변경 시 상태 초기화 등이 필요하다.
📚 Learning: 2025-08-20T05:59:48.729Z
Learnt from: maylh
PR: dnd-side-project/dnd-13th-8-frontend#55
File: src/widgets/playlist/BackgroundPlayer.tsx:55-83
Timestamp: 2025-08-20T05:59:48.729Z
Learning: BackgroundPlayer 컴포넌트에서 네비게이션 후 컨트롤 버튼이 작동하지 않는 버그는 주로 다음 원인들로 발생한다: 1) DOM ID 충돌 (고정값 'yt-player' 사용), 2) window.YT가 이미 존재할 때 새 플레이어를 생성하지 않음, 3) onStateChange 의존성으로 인한 useEffect 재실행과 전역 콜백 덮어쓰기, 4) 리마운트 시 ref 상태 불일치. 해결책으로는 고유 DOM ID 생성, initPlayer 함수 분리를 통한 즉시 초기화, 콜백 ref 사용으로 의존성 최소화, 플레이리스트 변경 시 상태 초기화 등이 필요하다.
Applied to files:
src/pages/mycd/index.tsxsrc/pages/discover/index.tsx
🧬 Code graph analysis (8)
src/shared/lib/getAccTime.ts (1)
src/shared/lib/index.ts (1)
getAccTime(4-4)
src/app/providers/PlayerProvider.tsx (1)
src/entities/playlist/types/playlist.ts (1)
PlaylistInfo(82-94)
src/pages/mycd/index.tsx (3)
src/app/providers/PlayerProvider.tsx (1)
usePlaylist(101-105)src/features/auth/store/authStore.ts (1)
useAuthStore(7-52)src/entities/playlist/model/useMyPlaylist.ts (1)
useMyRepresentativePlaylist(72-77)
src/pages/discover/playlist/index.tsx (2)
src/entities/playlist/model/usePlaylists.ts (1)
usePlaylistDetail(45-50)src/entities/playlist/types/playlist.ts (1)
PlaylistInfo(82-94)
src/pages/mycd/playlist/index.tsx (2)
src/entities/playlist/model/usePlaylists.ts (1)
usePlaylistDetail(45-50)src/entities/playlist/types/playlist.ts (1)
PlaylistInfo(82-94)
src/widgets/playlist/PlaylistInfo.tsx (2)
src/entities/playlist/types/playlist.ts (2)
PlaylistDetailResponse(72-80)PlaylistInfo(82-94)src/shared/styles/mixins.ts (1)
flexColCenter(10-15)
src/widgets/playlist/ProgressBar.tsx (2)
src/app/providers/PlayerProvider.tsx (1)
usePlaylist(101-105)src/shared/lib/getAccTime.ts (1)
getAccTime(1-3)
src/pages/discover/index.tsx (1)
src/app/providers/PlayerProvider.tsx (1)
usePlaylist(101-105)
🪛 Biome (2.1.2)
src/widgets/playlist/PlaylistInfo.tsx
[error] 9-9: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (7)
src/shared/ui/Link.tsx (1)
60-65: Title width: 100% 변경 LGTM행 높이/라인 클램프와 조합되어 안정적인 줄바꿈을 보장합니다.
src/app/providers/PlayerProvider.tsx (2)
16-18: 컨텍스트에 playerRef/handlePlayerStateChange 추가 LGTM소비자에서 YT Player 초기화/바인딩을 구성하기에 필요한 최소 표면을 적절히 노출했습니다.
74-79: 검증 완료 — YT 플레이어 바인딩 리그레션 없음
PlayerProvider의 handlePlayerStateChange가 YoutubePlayer로 정상 전달되어 onStateChange로 바인딩되며, 저장소에서 'yt-player' 고정 ID나 new YT.Player 생성은 발견되지 않았습니다. (참조: src/app/providers/PlayerProvider.tsx, src/widgets/playlist/YoutubePlayer.tsx, src/pages/discover/index.tsx)src/shared/lib/index.ts (1)
4-4: getAccTime 공개 export LGTMProgressBar 등에서 공용 유틸 접근이 일관돼집니다.
src/widgets/playlist/index.ts (1)
9-9: PlaylistInfo 재노출 LGTM위젯 모듈 단일 진입점 정리가 좋아졌습니다.
src/pages/discover/index.tsx (2)
99-101: getVideoId 인자 유효성 확인 권장.currentTrackIndex 범위/URL 유효성 보장은 컨텍스트에 위임된 듯하나, getVideoId가 undefined/null을 안전하게 처리하는지 확인 바랍니다. 필요하면 사전 가드 추가를 권장합니다.
177-189: YouTube 초기화 안정성 체크(과거 회귀 방지).BackgroundPlayer 관련 과거 회귀 이슈(고정 DOM id, 전역 콜백 덮어쓰기 등)가 재발하지 않는지 YoutubePlayer 내부 구현(고유 DOM id, 즉시 init, 콜백 ref) 확인 부탁드립니다. 현재 페이지 단에서는 playerRef 할당/seek/play 순서가 합리적으로 보입니다.
| const nextTrack = useCallback(() => { | ||
| if (currentPlaylist && currentTrackIndex < currentPlaylist.songs.length - 1) { | ||
| setCurrentTrackIndex((prevIndex) => prevIndex + 1) | ||
| setCurrentTrackIndex((prev) => prev + 1) | ||
| setCurrentTime(0) | ||
| if (playerRef.current) playerRef.current.seekTo(0, true) | ||
| } | ||
| } | ||
| }, [currentPlaylist, currentTrackIndex]) | ||
|
|
There was a problem hiding this comment.
다음 트랙 이동 시 seekTo(0)는 세그먼트 기반 모델에서 오동작 가능
앨범 1개 영상에 트랙 세그먼트를 매핑(누적 시간 모델)한다면, 트랙 시작 누적 시점으로 seek 해야 합니다. 0초로 이동하면 항상 영상 처음으로 돌아갑니다. getAccTime 유틸을 활용해 누적 시작 시점으로 이동하도록 수정 제안합니다.
- const nextTrack = useCallback(() => {
- if (currentPlaylist && currentTrackIndex < currentPlaylist.songs.length - 1) {
- setCurrentTrackIndex((prev) => prev + 1)
- setCurrentTime(0)
- if (playerRef.current) playerRef.current.seekTo(0, true)
- }
- }, [currentPlaylist, currentTrackIndex])
+ const nextTrack = useCallback(() => {
+ if (!currentPlaylist) return
+ if (currentTrackIndex >= currentPlaylist.songs.length - 1) return
+ const nextIndex = currentTrackIndex + 1
+ setCurrentTrackIndex(nextIndex)
+ setCurrentTime(0)
+ // FIXME: 아래 trackLengths는 실제 트랙 길이(초) 배열로 매핑하세요.
+ // 예: const trackLengths = currentPlaylist.songs.map((s) => s.lengthSec)
+ // 혹은 데이터 구조에 맞는 속성으로 교체
+ const trackLengths: number[] =
+ currentPlaylist.songs.map((/* s */) => /* lengthSec */ 0) // TODO: implement
+ if (playerRef.current) {
+ const acc = getAccTime(trackLengths, nextIndex, 0)
+ playerRef.current.seekTo(acc, true)
+ }
+ }, [currentPlaylist, currentTrackIndex])추가: 상단 import
-import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react'
+import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react'
+import { getAccTime } from '@/shared/lib'📝 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.
| const nextTrack = useCallback(() => { | |
| if (currentPlaylist && currentTrackIndex < currentPlaylist.songs.length - 1) { | |
| setCurrentTrackIndex((prevIndex) => prevIndex + 1) | |
| setCurrentTrackIndex((prev) => prev + 1) | |
| setCurrentTime(0) | |
| if (playerRef.current) playerRef.current.seekTo(0, true) | |
| } | |
| } | |
| }, [currentPlaylist, currentTrackIndex]) | |
| import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react' | |
| import { getAccTime } from '@/shared/lib' | |
| const nextTrack = useCallback(() => { | |
| if (!currentPlaylist) return | |
| if (currentTrackIndex >= currentPlaylist.songs.length - 1) return | |
| const nextIndex = currentTrackIndex + 1 | |
| setCurrentTrackIndex(nextIndex) | |
| setCurrentTime(0) | |
| // FIXME: 아래 trackLengths는 실제 트랙 길이(초) 배열로 매핑하세요. | |
| // 예: const trackLengths = currentPlaylist.songs.map((s) => s.lengthSec) | |
| // 혹은 데이터 구조에 맞는 속성으로 교체 | |
| const trackLengths: number[] = | |
| currentPlaylist.songs.map((/* s */) => /* lengthSec */ 0) // TODO: implement | |
| if (playerRef.current) { | |
| const acc = getAccTime(trackLengths, nextIndex, 0) | |
| playerRef.current.seekTo(acc, true) | |
| } | |
| }, [currentPlaylist, currentTrackIndex]) |
🤖 Prompt for AI Agents
In src/app/providers/PlayerProvider.tsx around lines 56 to 63, nextTrack
currently seeks to 0 which breaks segment-based (accumulated time) playlists;
instead compute the track's accumulated start time with the getAccTime util and
seek there. Change the seek call to
playerRef.current.seekTo(getAccTime(currentPlaylist.songs, currentTrackIndex +
1), true) (use the new index after incrementing), keep setCurrentTime(0) for
track-relative position, and add an import for getAccTime at the top of the
file.
| const prevTrack = useCallback(() => { | ||
| if (currentTrackIndex > 0) { | ||
| setCurrentTrackIndex((prevIndex) => prevIndex - 1) | ||
| setCurrentTrackIndex((prev) => prev - 1) | ||
| setCurrentTime(0) | ||
| if (playerRef.current) playerRef.current.seekTo(0, true) | ||
| } | ||
| } | ||
| }, [currentTrackIndex]) | ||
|
|
There was a problem hiding this comment.
이전 트랙 이동도 누적 시작 시점으로 seek 필요
prevTrack도 동일하게 누적 시작 시점으로 이동해야 합니다.
- const prevTrack = useCallback(() => {
- if (currentTrackIndex > 0) {
- setCurrentTrackIndex((prev) => prev - 1)
- setCurrentTime(0)
- if (playerRef.current) playerRef.current.seekTo(0, true)
- }
- }, [currentTrackIndex])
+ const prevTrack = useCallback(() => {
+ if (!currentPlaylist) return
+ if (currentTrackIndex <= 0) return
+ const prevIndex = currentTrackIndex - 1
+ setCurrentTrackIndex(prevIndex)
+ setCurrentTime(0)
+ const trackLengths: number[] =
+ currentPlaylist.songs.map((/* s */) => /* lengthSec */ 0) // TODO: implement
+ if (playerRef.current) {
+ const acc = getAccTime(trackLengths, prevIndex, 0)
+ playerRef.current.seekTo(acc, true)
+ }
+ }, [currentPlaylist, currentTrackIndex])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/providers/PlayerProvider.tsx around lines 64–71, prevTrack currently
seeks to 0 when moving to the previous track; it must instead seek to the
cumulative start time for that track. Replace the fixed 0 seek with calculation
of the track's accumulated start (e.g., compute the sum of durations/start
offsets for all tracks before the new index or use the existing helper used by
nextTrack), setCurrentTime to that accumulatedStart, and call
playerRef.current.seekTo(accumulatedStart, true). Also ensure the useCallback
dependency list includes any values used to compute the accumulated start
(tracks or helper).
| const { id } = useParams<{ id: string }>() | ||
| const { data, isLoading, isError } = usePlaylistDetail(Number(id)) | ||
|
|
||
| const { data: playlistData, isLoading, isError } = usePlaylistDetail(Number(id)) | ||
|
|
||
| if (isError || !playlistData) { | ||
| return ( | ||
| <NoDataWrapper> | ||
| <Error /> | ||
| </NoDataWrapper> | ||
| ) | ||
| } | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <NoDataWrapper> | ||
| <Loading isLoading width="100%" height="100%" /> | ||
| </NoDataWrapper> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <Wrapper> | ||
| <Header | ||
| left={<span>플레이리스트</span>} | ||
| right={<SvgButton icon={Cancel} onClick={() => navigate(-1)} />} | ||
| /> | ||
| <Content> | ||
| <PlaylistHorizontal | ||
| genre={getGenreLabel(playlistData?.genre || '')} | ||
| title={playlistData?.playlistName || ''} | ||
| username={playlistData?.creatorNickname || ''} | ||
| stickers={playlistData?.onlyCdResponse?.cdItems || []} | ||
| /> | ||
| <TrackInfo> | ||
| {playlistData.songs && | ||
| playlistData.songs.map((track, index) => ( | ||
| <Link key={index} data={track} variant="large" /> | ||
| ))} | ||
| </TrackInfo> | ||
| </Content> | ||
| </Wrapper> | ||
| ) | ||
| return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} /> | ||
| } |
There was a problem hiding this comment.
NaN playlistId 호출 방지: 파라미터 가드 추가 필요
id가 없거나 비정상일 때 Number(id)는 NaN이 되어 잘못된 API 호출이 발생할 수 있습니다. 유효성 검사 후에만 쿼리하도록 가드해 주세요.
적용 예시:
const { id } = useParams<{ id: string }>()
- const { data, isLoading, isError } = usePlaylistDetail(Number(id))
-
- return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} />
+ const playlistId = Number(id)
+ if (!Number.isFinite(playlistId)) {
+ return <PlaylistInfo playlistData={undefined} isLoading={false} isError />
+ }
+ const { data, isLoading, isError } = usePlaylistDetail(playlistId)
+ return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} />Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/discover/playlist/index.tsx around lines 7 to 11, the component
converts params id to Number(id) unguarded which can produce NaN and trigger
invalid API calls; validate the id first (e.g., check id exists and
parseInt/Number is valid and not NaN) and only call usePlaylistDetail when the
id is a valid number (or pass an enabled flag to the hook conditional on
validity); render a fallback (loading/empty/error) or return early when id is
missing/invalid so the API is never invoked with NaN.
| const MyCdInfoPage = () => { | ||
| const location = useLocation() | ||
| const { playlistId } = location.state as { playlistId: number } | ||
| const { data, isLoading, isError } = usePlaylistDetail(Number(playlistId)) | ||
|
|
||
| const { data: playlistData, isLoading, isError } = usePlaylistDetail(Number(playlistId)) | ||
|
|
||
| if (isError || !playlistData) { | ||
| return ( | ||
| <NoDataWrapper> | ||
| <Error /> | ||
| </NoDataWrapper> | ||
| ) | ||
| } | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <NoDataWrapper> | ||
| <Loading isLoading width="100%" height="100%" /> | ||
| </NoDataWrapper> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <Wrapper> | ||
| <Header | ||
| left={<span>플레이리스트</span>} | ||
| right={<SvgButton icon={Cancel} onClick={() => navigate(-1)} />} | ||
| /> | ||
| <Content> | ||
| <PlaylistHorizontal | ||
| genre={playlistData?.genre || ''} | ||
| title={playlistData?.playlistName || ''} | ||
| username={playlistData?.creatorNickname || ''} | ||
| /> | ||
| <TrackInfo> | ||
| {playlistData.songs && | ||
| playlistData.songs.map((track, index) => ( | ||
| <Link key={index} data={track} variant="large" /> | ||
| ))} | ||
| </TrackInfo> | ||
| </Content> | ||
| </Wrapper> | ||
| ) | ||
| return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} /> | ||
| } |
There was a problem hiding this comment.
직접 진입 시 location.state 없음 처리
새로고침/직접 접근 시 location.state가 없으면 Number(undefined) → NaN으로 잘못된 쿼리가 나갑니다. 가드를 추가해주세요.
적용 예시:
const MyCdInfoPage = () => {
const location = useLocation()
- const { playlistId } = location.state as { playlistId: number }
- const { data, isLoading, isError } = usePlaylistDetail(Number(playlistId))
+ const state = location.state as { playlistId?: number } | null
+ const playlistId = state?.playlistId
+ if (!Number.isFinite(Number(playlistId))) {
+ return <PlaylistInfo playlistData={undefined} isLoading={false} isError />
+ }
+ const { data, isLoading, isError } = usePlaylistDetail(Number(playlistId))
return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} />
}📝 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.
| const MyCdInfoPage = () => { | |
| const location = useLocation() | |
| const { playlistId } = location.state as { playlistId: number } | |
| const { data, isLoading, isError } = usePlaylistDetail(Number(playlistId)) | |
| const { data: playlistData, isLoading, isError } = usePlaylistDetail(Number(playlistId)) | |
| if (isError || !playlistData) { | |
| return ( | |
| <NoDataWrapper> | |
| <Error /> | |
| </NoDataWrapper> | |
| ) | |
| } | |
| if (isLoading) { | |
| return ( | |
| <NoDataWrapper> | |
| <Loading isLoading width="100%" height="100%" /> | |
| </NoDataWrapper> | |
| ) | |
| } | |
| return ( | |
| <Wrapper> | |
| <Header | |
| left={<span>플레이리스트</span>} | |
| right={<SvgButton icon={Cancel} onClick={() => navigate(-1)} />} | |
| /> | |
| <Content> | |
| <PlaylistHorizontal | |
| genre={playlistData?.genre || ''} | |
| title={playlistData?.playlistName || ''} | |
| username={playlistData?.creatorNickname || ''} | |
| /> | |
| <TrackInfo> | |
| {playlistData.songs && | |
| playlistData.songs.map((track, index) => ( | |
| <Link key={index} data={track} variant="large" /> | |
| ))} | |
| </TrackInfo> | |
| </Content> | |
| </Wrapper> | |
| ) | |
| return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} /> | |
| } | |
| const MyCdInfoPage = () => { | |
| const location = useLocation() | |
| const state = location.state as { playlistId?: number } | null | |
| const playlistId = state?.playlistId | |
| if (!Number.isFinite(Number(playlistId))) { | |
| return <PlaylistInfo playlistData={undefined} isLoading={false} isError /> | |
| } | |
| const { data, isLoading, isError } = usePlaylistDetail(Number(playlistId)) | |
| return <PlaylistInfo playlistData={data} isLoading={isLoading} isError={isError} /> | |
| } |
🤖 Prompt for AI Agents
In src/pages/mycd/playlist/index.tsx around lines 6 to 12, location.state is
assumed to exist which leads to Number(undefined) → NaN on refresh/direct
access; add a guard to handle missing state by first checking if
location.state?.playlistId exists and using that, otherwise extract the
playlistId from the URL (e.g. useParams or parse location.pathname/search),
convert to Number, validate it is a finite number, and if invalid handle
gracefully (redirect to a safe page, show an error, or avoid calling
usePlaylistDetail). Ensure usePlaylistDetail is only called with a valid numeric
id.
| export const getAccTime = (trackLengths: number[], trackIndex: number, currentTime: number) => { | ||
| return trackLengths.slice(0, trackIndex).reduce((acc, len) => acc + len, 0) + currentTime | ||
| } |
There was a problem hiding this comment.
누적 시간 상한/하한 클램프 필요 (진행률 100% 초과 방지)
현재 트랙 경계/플레이어 상태 지연 등으로 currentTime이 트랙 길이를 초과하면 accTime이 전체 duration을 넘어 100% 이상이 됩니다. 클램프 추가를 권장합니다.
적용 예시:
-export const getAccTime = (trackLengths: number[], trackIndex: number, currentTime: number) => {
- return trackLengths.slice(0, trackIndex).reduce((acc, len) => acc + len, 0) + currentTime
-}
+export const getAccTime = (trackLengths: number[], trackIndex: number, currentTime: number) => {
+ const safeIndex = Math.max(0, Math.min(trackIndex, trackLengths.length))
+ const safeLengths = trackLengths.map((l) => Math.max(0, l || 0))
+ const playedBefore = safeLengths.slice(0, safeIndex).reduce((acc, len) => acc + len, 0)
+ const curLen = safeIndex < safeLengths.length ? safeLengths[safeIndex] : 0
+ const duration = safeLengths.reduce((acc, len) => acc + len, 0)
+ const local = Math.max(0, Math.min(currentTime || 0, curLen))
+ return Math.min(playedBefore + local, duration)
+}📝 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.
| export const getAccTime = (trackLengths: number[], trackIndex: number, currentTime: number) => { | |
| return trackLengths.slice(0, trackIndex).reduce((acc, len) => acc + len, 0) + currentTime | |
| } | |
| export const getAccTime = (trackLengths: number[], trackIndex: number, currentTime: number) => { | |
| const safeIndex = Math.max(0, Math.min(trackIndex, trackLengths.length)) | |
| const safeLengths = trackLengths.map((l) => Math.max(0, l || 0)) | |
| const playedBefore = safeLengths.slice(0, safeIndex).reduce((acc, len) => acc + len, 0) | |
| const curLen = safeIndex < safeLengths.length ? safeLengths[safeIndex] : 0 | |
| const duration = safeLengths.reduce((acc, len) => acc + len, 0) | |
| const local = Math.max(0, Math.min(currentTime || 0, curLen)) | |
| return Math.min(playedBefore + local, duration) | |
| } |
🤖 Prompt for AI Agents
In src/shared/lib/getAccTime.ts around lines 1 to 3, the function can produce
accTime beyond the total duration when currentTime exceeds the current track
length or is negative; clamp values to prevent progress >100% or <0. Ensure
trackIndex is within bounds, clamp currentTime to the range [0,
trackLengths[trackIndex] || 0], compute the prefix sum of trackLengths up to
trackIndex, then compute accTime = prefixSum + clampedCurrentTime and finally
clamp the result between 0 and totalDuration (sum of all trackLengths) before
returning; implement clamps using Math.min/Math.max.
| if (isError || !playlistData) { | ||
| return ( | ||
| <NoDataWrapper> | ||
| <Error /> | ||
| </NoDataWrapper> | ||
| ) | ||
| } | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <NoDataWrapper> | ||
| <Loading isLoading width="100%" height="100%" /> | ||
| </NoDataWrapper> | ||
| ) | ||
| } |
There was a problem hiding this comment.
로딩/에러 상태 분기 순서 수정 필요 (초기 로드 시 오탐 에러 표시).
isLoading=true 이면서 playlistData가 아직 없을 때도 에러가 먼저 렌더링됩니다. 로딩을 우선 처리하고, 그 다음 에러, 마지막에 데이터 부재를 처리하세요.
- if (isError || !playlistData) {
- return (
- <NoDataWrapper>
- <Error />
- </NoDataWrapper>
- )
- }
-
- if (isLoading) {
+ if (isLoading) {
return (
<NoDataWrapper>
<Loading isLoading width="100%" height="100%" />
</NoDataWrapper>
)
}
+
+ if (isError) {
+ return (
+ <NoDataWrapper>
+ <UiError />
+ </NoDataWrapper>
+ )
+ }
+
+ if (!playlistData) {
+ return null
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/widgets/playlist/PlaylistInfo.tsx around lines 21 to 35, the current
conditional renders Error when playlistData is missing even during initial
loading; reorder the checks so loading is handled first, then error, then
missing-data: first if (isLoading) return the Loading UI, next if (isError)
return the Error UI, and finally if (!playlistData) return a NoData UI or
equivalent; ensure the branches are mutually exclusive so loading state takes
precedence over error/no-data rendering.
| const accTime = getAccTime(trackLengths, currentIndex, currentTime) | ||
|
|
There was a problem hiding this comment.
accTime 클램프 적용 (진행률 100% 초과 방지)
accTime이 duration을 넘어가지 않도록 클램프하여 UI 오버플로우를 막으세요.
적용 예시:
- const accTime = getAccTime(trackLengths, currentIndex, currentTime)
+ const accTime = getAccTime(trackLengths, currentIndex, currentTime)
+ const clampedAccTime = Math.min(Math.max(accTime, 0), duration)그리고 아래 계산에 clampedAccTime 사용:
- const progressPercent = duration > 0 ? (accTime / duration) * 100 : 0
+ const progressPercent = duration > 0 ? (clampedAccTime / duration) * 100 : 0및 시간 표시:
- <span>{formatTime(accTime)}</span>
+ <span>{formatTime(clampedAccTime)}</span>📝 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.
| const accTime = getAccTime(trackLengths, currentIndex, currentTime) | |
| const accTime = getAccTime(trackLengths, currentIndex, currentTime) | |
| const clampedAccTime = Math.min(Math.max(accTime, 0), duration) |
🤖 Prompt for AI Agents
In src/widgets/playlist/ProgressBar.tsx around lines 36 to 37, accTime is
computed without clamping which can exceed the track duration and cause UI
overflow; clamp accTime to be within [0, duration] (e.g., clampedAccTime =
Math.min(Math.max(accTime, 0), duration)) and then replace uses of accTime in
progress calculations (percent, filled width) and in any displayed time strings
with clampedAccTime so the progress never shows over 100% or displays times
beyond duration.
2 participants
🛰️ 관련 이슈
✨ 주요 변경 사항
🔍 테스트 방법 / 체크리스트
🗯️ PR 포인트
PlaylistInfowidgets 생성 -> 둘러보기, 나의 cd에 각각 적용🚀 알게된 점
📖 참고 자료 (선택)
Summary by CodeRabbit
신기능
리팩터
스타일
잡무