Skip to content

[feat] 둘러보기 UX 개선사항 반영 #108

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.

Sign up for GitHub

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

Merged
maylh merged 4 commits into from
Sep 25, 2025
Merged

[feat] 둘러보기 UX 개선사항 반영 #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d14df9d
feat: 플레이어 재생중일 때 CD 회전 css 추가 (#107)
maylh Sep 22, 2025
cbb2c32
feat: 특정 곡 클릭 시 해당 인덱스부터 재생 (#107)
maylh Sep 22, 2025
649b6af
feat: unmute 이후 상태 유지되도록 loadVideoById 호출 (#107)
maylh Sep 24, 2025
82899fd
fix: 트랙 key값 id로 변경 (#107)
maylh Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/shared/ui/Link.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import type { Track } from '@/entities/playlist'
interface LinkProps {
variant?: 'large' | 'small'
data: Track
onClick?: () => void
}

const Link = ({ variant = 'large', data }: LinkProps) => {
const Link = ({ variant = 'large', data, onClick }: LinkProps) => {
return (
<LinkBox $variant={variant}>
<LinkBox $variant={variant} onClick={onClick}>
<Thumbnail $thumbnail={data.youtubeThumbnail ?? undefined} $variant={variant}>
{!data.youtubeThumbnail && (
<Gallery width={variant === 'large' ? 24 : 20} height={variant === 'large' ? 24 : 20} />
Expand Down
15 changes: 14 additions & 1 deletion src/widgets/playlist/PlaylistInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useNavigate } from 'react-router-dom'

import styled from 'styled-components'

import { usePlaylist } from '@/app/providers/PlayerProvider'
import { Cancel } from '@/assets/icons'
import type { PlaylistDetailResponse } from '@/entities/playlist'
import { getGenreLabel } from '@/shared/lib'
Expand All @@ -17,6 +18,13 @@ interface PlaylistInfoProps {

const PlaylistInfo = ({ playlistData, isLoading, isError }: PlaylistInfoProps) => {
const navigate = useNavigate()
const { setPlaylist, currentPlaylist } = usePlaylist()

const handleClickTrack = (trackIndex: number) => {
if (!currentPlaylist) return
navigate(-1)
setPlaylist(currentPlaylist, trackIndex, 0)
}
Comment on lines +23 to +27
Copy link

@gemini-code-assist gemini-code-assist bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

플레이리스트 상세 페이지에서 특정 트랙을 클릭했을 때의 재생 로직에 치명적인 오류가 있습니다.

현재 코드는 usePlaylist에서 가져온 currentPlaylist를 사용하여 setPlaylist를 호출하고 있습니다. 이 경우, 사용자가 보고 있는 플레이리스트(playlistData)가 아닌 현재 재생 중인 플레이리스트의 트랙이 변경됩니다. 만약 아무것도 재생 중이지 않다면 currentPlaylist가 null이므로 아무 동작도 하지 않게 됩니다.

사용자가 보고 있는 playlistData를 기준으로 플레이리스트를 설정하도록 수정해야 합니다. playlistData의 타입(PlaylistDetailResponse)과 setPlaylist가 요구하는 타입(PlaylistInfo)이 다르므로, 필요한 형태로 객체를 변환하여 전달해야 합니다.

  const handleClickTrack = (trackIndex: number) => {
    if (!playlistData) return
    const playlistToPlay: PlaylistInfo = {
      ...playlistData,
      creator: {
        creatorId: playlistData.creatorId,
        creatorNickname: playlistData.creatorNickname,
      },
    }
    navigate(-1)
    setPlaylist(playlistToPlay, trackIndex, 0)
  }

Comment on lines +23 to +27
Copy link

@coderabbitai coderabbitai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

선택한 트랙 재생 시 원본 플레이리스트 소스 확인 필요 (현재 로직은 currentPlaylist만 사용)

상세 화면에서 클릭한 트랙의 인덱스로 재생하려면, currentPlaylist가 상세의 playlistData와 동일하지 않은 경우에도 해당 상세 플레이리스트를 소스로 설정해야 합니다. 현 로직은 currentPlaylist가 없으면 아무 작업도 하지 않고, 다른 플레이리스트가 활성화된 상태에서는 잘못된 인덱스로 점프할 수 있습니다.

권장: currentPlaylist?.playlistId !== playlistData.playlistId인 경우 playlistData를 Player가 요구하는 타입으로 변환하여 setPlaylist에 전달하세요. 또한 setPlaylist 이후 navigate(-1) 호출 순서를 권장합니다.

예시(개념적):

const handleClickTrack = (trackIndex: number) => {
  if (!playlistData) return
  const source =
    currentPlaylist?.playlistId === playlistData.playlistId
      ? currentPlaylist
      : transformDetailToPlaylistInfo(playlistData) // 변환 함수 필요
  setPlaylist(source, trackIndex, 0)
  navigate(-1)
}
🤖 Prompt for AI Agents 
In src/widgets/playlist/PlaylistInfo.tsx around lines 23 to 27, the click
handler currently only uses currentPlaylist and returns early if it's missing,
which can cause wrong-track jumps when the detailed view's playlistData differs;
change the logic to first ensure playlistData exists, then choose the source: if
currentPlaylist?.playlistId === playlistData.playlistId use currentPlaylist,
otherwise convert playlistData into the Player's PlaylistInfo shape (implement a
transformDetailToPlaylistInfo or similar) and pass that to setPlaylist; call
setPlaylist(source, trackIndex, 0) before navigate(-1) so the player is set to
the correct track, and remove the early return on missing currentPlaylist in
favor of checking playlistData.


if (isError || !playlistData) {
return (
Expand Down Expand Up @@ -50,7 +58,12 @@ const PlaylistInfo = ({ playlistData, isLoading, isError }: PlaylistInfoProps) =
<TrackInfo>
{playlistData.songs &&
playlistData.songs.map((track, index) => (
<Link key={index} data={track} variant="large" />
<Link
key={track.id}
data={track}
variant="large"
onClick={() => handleClickTrack(index)}
/>
))}
</TrackInfo>
</Content>
Expand Down
16 changes: 14 additions & 2 deletions src/widgets/playlist/PlaylistLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const PlaylistLayout = ({
)}
</Container>
<Wrapper>
<CdWrapper>
<CdWrapper $isPlaying={isPlaying}>
{isMobile && (
<VolumeButton playerRef={playerRef} isMuted={isMuted} setIsMuted={setIsMuted} />
)}
Expand Down Expand Up @@ -130,6 +130,18 @@ const Container = styled.div`
justify-content: space-between;
`

const CdWrapper = styled.div`
const CdWrapper = styled.div<{ $isPlaying: boolean }>`
position: relative;
animation: spin 40s linear infinite;
animation-play-state: ${(props) => (props.$isPlaying ? 'running' : 'paused')};
transform-origin: center;

@keyframes spin {
0% {
transform: rotate(0deg);
}
100% {
transform: rotate(360deg);
}
}
`
27 changes: 21 additions & 6 deletions src/widgets/playlist/YoutubePlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useRef, useEffect } from 'react'
import YouTube from 'react-youtube'
import type { YouTubeProps, YouTubeEvent, YouTubePlayer } from 'react-youtube' //
import type { YouTubeProps, YouTubeEvent, YouTubePlayer } from 'react-youtube'

import { useDevice } from '@/shared/lib/useDevice'

interface YoutubePlayerProps {
videoId: string
Expand All @@ -10,23 +10,38 @@ interface YoutubePlayerProps {
}

function YoutubePlayer({ videoId, onReady, onStateChange }: YoutubePlayerProps) {
const deviceType = useDevice()
const isMobile = deviceType === 'mobile'
const playerRef = useRef<YouTubePlayer | null>(null)
const isFirstLoad = useRef(true)

const playerOpts: YouTubeProps['opts'] = {
playerVars: {
autoplay: 1,
mute: isMobile ? 1 : 0,
mute: 0,
playsinline: 1, // 모바일 인라인 재생
},
}

useEffect(() => {
if (!isFirstLoad.current && playerRef.current && videoId) {
// 첫 로딩 제외하고 loadVideoById 호출
try {
playerRef.current.loadVideoById(videoId)
} catch (e) {
console.error('loadVideoById 실패', e)
}
}
isFirstLoad.current = false
}, [videoId])

Comment on lines +24 to +35
Copy link

@coderabbitai coderabbitai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

videoId 변경 시 이중 load 위험 (react-youtube가 이미 처리함)

react-youtube는 videoId prop 변경 시 내부적으로 loadVideoById를 호출합니다. 현재 useEffect에서 동일 호출을 반복해 깜빡임/상태 경합이 생길 수 있습니다. 이 효과를 제거하는 것을 권장합니다.

적용 diff:

-  useEffect(() => {
-    if (!isFirstLoad.current && playerRef.current && videoId) {
-      // 첫 로딩 제외하고 loadVideoById 호출
-      try {
-        playerRef.current.loadVideoById(videoId)
-      } catch (e) {
-        console.error('loadVideoById 실패', e)
-      }
-    }
-    isFirstLoad.current = false
-  }, [videoId])
+  // react-youtube가 videoId 변경을 내부적으로 처리하므로 별도의 load 호출은 불필요합니다.

추가로 isFirstLoad ref는 불필요해집니다(정리 권장).

📝 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.

Suggested change
useEffect(() => {
if (!isFirstLoad.current && playerRef.current && videoId) {
// 첫 로딩 제외하고 loadVideoById 호출
try {
playerRef.current.loadVideoById(videoId)
} catch (e) {
console.error('loadVideoById 실패', e)
}
}
isFirstLoad.current = false
}, [videoId])
// react-youtube가 videoId 변경을 내부적으로 처리하므로 별도의 load 호출은 불필요합니다.
🤖 Prompt for AI Agents 
In src/widgets/playlist/YoutubePlayer.tsx around lines 24 to 35, remove the
useEffect that calls playerRef.current.loadVideoById on videoId changes because
react-youtube already handles loading when its videoId prop changes; delete the
entire effect block and also remove the isFirstLoad ref and any references to it
(cleanup its declaration and the isFirstLoad.current assignment) so we don't
duplicate behavior or keep unused state.

return (
<div style={{ width: 0, height: 0, overflow: 'hidden', position: 'absolute' }}>
<YouTube
videoId={videoId}
opts={playerOpts}
onReady={onReady}
onReady={(e) => {
playerRef.current = e.target
onReady(e)
}}
onStateChange={onStateChange}
/>
</div>
Expand Down
Loading