Refactor/#244 기존 SongDetailPage 컴포넌트에 새로운 플로우와 시안을 적용한다. #261

Creative-Lee · 2023-08-14T06:11:04Z

📝작업 내용

기존 ToggleGroup 컴포넌트를 KillingpartTrackList 컴포넌트로 대체
- progress bar 작동하도록 timer 컴포넌트 구현
- 이와 관련한 시간 동기화 안되는 문제 발생.
  - 디스커션 or 대화로 전달 예정입니다.
DetailPage내 컴포넌트, 상태, effect 분리
기존 구간 반복 기능 개선
- 구간 반복 off시 파트 1회 재생후 멈춤
- 기존 구간반복 버튼 토글 시 구간의 맨 처음으로 보내고 있는데, 이 부분을 개별 동작으로 개선하려다가 실패했습니다
  - 개선 시도하다가 도움될 내용이 있어서 코난에게 따로 설명드릴게요.
기존 댓글 기능이 그대로 동작하도록 함.
- 우코 댓글모달 구현 끝나면 새로운 이슈생성해서 통합하겠습니다.

추가 작업

Player & Youtube 관련

PlayerContext 내 함수 정리
player state -> player ref로 변경
player ref 초기화 방식 변경
- new Player() 결과물 직접 할당 -> onReady 이벤트로 할당

etc

파트 재생중 유튜브 플레이어로 시간을 조작하면 재생중인 파트 토글이 풀리는 기능 구현

💬리뷰 참고사항

로직에 대한 의문이 있을 것 같아요.
무조건 물어봐 주세요!

디자인 변경 전

디자인 변경 후

#️⃣연관된 이슈

close #244

1. 기존 nowPlayingTrack 상태에 사용된 rank는 변동 가능한 값이므로 part의 id로 변경 2. 이에 따른 props 변경

api 응답 type 그대로 받도록 수정

1. 테스트가 끝난 컴포넌트의 상태를 공유하기 위해 변경하였음. 2. 파일명 변경

1. 기존 isPlaying은 플레이어의 play와 혼동이 생길 가능성이 많았음. 개선을 통해 플레이중인 트랙이라는 의미를 강조함 2. 이에따라 props가 변경되었음

1. 플레이어 초기화 이후 ref에 할당하도록 개선 2. 관련 이벤트 핸들러 생성

1. 타이머, 플레이어 프로바이더의 함수에서 pause 관련 함수 제거 2. 트랙 컴포넌트의 타이머가 영상 상태와 동기화 되도록 수정

실행중인 파트 기준이 rank -> partId 로 변경됨에 따라 로직을 삭제함

github-actions · 2023-08-14T06:13:06Z

Unit Test Results

  69 files   69 suites 9s ⏱️
276 tests 276 ✔️ 0 💤 0 ❌
279 runs 279 ✔️ 0 💤 0 ❌

Results for commit c3d437f.

♻️ This comment has been updated with latest results.

Creative-Lee

리뷰에 도움되도록 주석 달아놓았습니다~

Creative-Lee · 2023-08-14T06:12:53Z

frontend/src/features/songs/components/KillingPartInterface.tsx

+    }
+  }, [videoPlayer, playerState]);
+
+  useEffect(() => {


반복 재생 이펙트 이동 및 로직 개선

Creative-Lee · 2023-08-14T06:13:49Z

frontend/src/features/songs/components/KillingPartInterface.tsx

+const DEFAULT_PART_ID = -1;
+
+const KillingPartInterface = ({ killingParts, songId }: KillingPartInterfaceProps) => {
+  const [nowPlayingTrack, setNowPlayingTrack] = useState<KillingPart['id']>(DEFAULT_PART_ID);


이제 변동 가능한 rank 대신 고정값인 id로 관리합니다!

💬 rank는 왜 변동가능하고 id는 왜 고정인가요? 둘 다 똑같은 KillingPart의 정보 아닌가요?
rank로 관리하나 id로 관리하나 무슨 차이인지 잘 모르겠어요

역으로 생각하면 서버에서 내려주는 killingParts에는 rank가 순서대로 고정적으로 담겨있고,
나머지 정보가 변동된다고 해석할 수도 있을 것 같은데 뭐가 고정이고 뭐가 변동이라는 말인가요?

제 설명이 다소 애매하고 부족했네요.
생각했던 부분을 전부 나열하는게 오히려 좋을것 같아서 아래에 다 적어봅니다!

{killingPart && <CommentList songId={id} partId={killingParts[killingRank! - 1].id} />}

리팩터링 전에는 page컴포넌트가 KillingRank 상태를 가지고 있었고, rank로 killingPart를 찾고,
이걸 코멘트 리스트에 id를 넘기는데 쓰이고 있었어요.
이 부분에서 기존에 있던 KillingRank state대신, nowPlayingTrack state에 담는 값을 id로 바꾸면서
하나로 통일하면 좋겠다고 생각했어요~

또 이 과정에서 "part의 id는 순위가 변동된다고 변하지 않고, rank는 순위에 변동에 따라 바뀌니깐
rank가 어떤걸 정하는 내부적 기준이 되면 언젠가 의미적으로 햇갈릴 수 있겠다" 막연하게 생각하기도 했고요!

이제 변동 가능한 rank 대신 고정값인 id로 관리합니다!

이 문장 하나로는 택도 없었네요 ㅠ

역으로 생각하면 서버에서 내려주는 killingParts에는 rank가 순서대로 고정적으로 담겨있고,
나머지 정보가 변동된다고 해석할 수도 있을 것 같은데 뭐가 고정이고 뭐가 변동이라는 말인가요?

코난의 의문이 확실히 이해됩니다!
혹시 위 이유들을 읽어보시고 여전히 의문이 남으면 직접 이야기 해주세요!

리팩터링 전에는 page컴포넌트가 KillingRank 상태를 가지고 있었고, rank로 killingPart를 찾고,
이걸 코멘트 리스트에 id를 넘기는데 쓰이고 있었어요.
이 부분에서 기존에 있던 KillingRank state대신, nowPlayingTrack state에 담는 값을 id로 바꾸면서
하나로 통일하면 좋겠다고 생각했어요~

그렇군요 👍👍

Creative-Lee · 2023-08-14T06:21:25Z

frontend/src/features/songs/components/KillingPartTrack.tsx


-  // const stopPlaying = () => {
-  //   const isPlaying = videoPlayer?.getPlayerState() === 1;
+  useEffect(() => {


progress bar를 위한 타이머 effect
플레이어 상태를 따라갑니다.

Creative-Lee · 2023-08-14T06:29:40Z

frontend/src/features/songs/components/Thumbnail.tsx


-const Thumbnail = ({ ...props }: ThumbnailProps) => {
+const Thumbnail = ({ size = 'lg', ...props }: ThumbnailProps) => {


사이즈 수정 가능하도록 props 추가

Creative-Lee · 2023-08-14T06:37:26Z

frontend/src/features/youtube/components/Youtube.tsx

+        playerVars: { start, rel: 0 },
+        events: {
+          onReady: ({ target: player }) => (videoPlayer.current = player),
+          onStateChange: updatePlayerState,


playerState 이용하기 위한 이벤트

cruelladevil

고생하셨습니다 👍👍
코멘트는 전반적으로 어떤 의도로 작성하셨는지 궁금해서 남긴 질문들입니다!
그리고 아래 2가지 추가 질문에 대해서 알려주세용~

useCallback의 사용 기준을 모르겠어요.
videoPlayer가 ref로 바뀐 이유가 궁금해요.

cruelladevil · 2023-08-14T06:35:19Z

frontend/src/features/comments/components/CommentList.tsx

-  if (!comments) {
-    return null;
-  }
-


refactor: 코멘트 컴포넌트 props타입 변경

💬 커밋 메세지로 확인 안되어서 그런데 이건 왜 고치신건가요?

커밋을 잘 나누지 못했네요...
만약 comments 배열 자체가 없거나, comments 배열은 있는데 코멘트가 없어도
랜더링은 진행되어서 댓글 0개는 보여주는게 좋겠다 생각했어요!

cruelladevil · 2023-08-14T06:55:07Z

frontend/src/features/songs/components/KillingPartInterface.tsx

+const DEFAULT_PART_ID = -1;
+
+const KillingPartInterface = ({ killingParts, songId }: KillingPartInterfaceProps) => {
+  const [nowPlayingTrack, setNowPlayingTrack] = useState<KillingPart['id']>(DEFAULT_PART_ID);


💬 rank는 왜 변동가능하고 id는 왜 고정인가요? 둘 다 똑같은 KillingPart의 정보 아닌가요?
rank로 관리하나 id로 관리하나 무슨 차이인지 잘 모르겠어요

역으로 생각하면 서버에서 내려주는 killingParts에는 rank가 순서대로 고정적으로 담겨있고,
나머지 정보가 변동된다고 해석할 수도 있을 것 같은데 뭐가 고정이고 뭐가 변동이라는 말인가요?

cruelladevil · 2023-08-14T06:58:35Z

frontend/src/features/songs/components/KillingPartTrack.stories.tsx

-  };
-
-  const isPlaying = killingPart.rank === nowPlayingTrack;
+  const isNowPlayingTrack = killingPart.rank === nowPlayingTrack;


Suggested change

const isNowPlayingTrack = killingPart.rank === nowPlayingTrack;

const isNowPlayingTrack = killingPart.id === nowPlayingTrack;

이런일이! 수정하겠습니다!

cruelladevil · 2023-08-14T07:00:10Z

frontend/src/features/songs/components/KillingPartTrack.tsx

@@ -13,49 +15,74 @@ import type React from 'react';

 interface KillingPartTrackProps {
  killingPart: KillingPart;
-  isPlaying: boolean;
-  changePlayingTrack: React.ChangeEventHandler<HTMLInputElement>;
+  isNowPlayingTrack: boolean;


💬 네이밍에 now는 왜 붙는지 궁금해요

개인적으로는 현재 실행중인 트랙이라는 의미를 명확하게 나타내주는게 좋다고 생각했어요
개발 당시에 유튜브 플레이어 관련해서도 isPlaying과 비슷한 의미를 가지는 변수가 있어서 여기서 오는 혼동을 막기위한 이유도 있었어요!

cruelladevil · 2023-08-14T07:01:35Z

frontend/src/features/songs/components/KillingPartTrack.tsx

-  //   videoPlayer?.playVideo();
-  // };
+  const getPlayIcon = useCallback(() => {
+    if (!isNowPlayingTrack || playerState === 2) {


Suggested change

if (!isNowPlayingTrack || playerState === 2) {

if (!isNowPlayingTrack || playerState === YT.PlayerState.PAUSED) {

아 이렇게 접근 가능하군요!
하나 배워갑니다! 바로 반영할게요!

cruelladevil · 2023-08-14T07:04:27Z

frontend/src/features/songs/components/KillingPartTrack.tsx

-  //   videoPlayer?.seekTo(start, false);
-  //   videoPlayer?.playVideo();
-  // };
+  const getPlayIcon = useCallback(() => {


💬 왜 useCallback이어야 되는지 궁금해요

timer 관련 상태덕분에... 이 함수는 0.1초 마다 매번 새로 만들어지는데요!
타이머 상태로 인한 리랜더링에는 새로 생성되지 않도록 했습니다!

cruelladevil · 2023-08-14T07:06:56Z

frontend/src/features/songs/components/KillingPartTrack.tsx

+          onChange={toggleTrackPlayAndStop}
+          onClick={toggleTrackPlayAndStop}


💬 onChange, onClick 둘 다 있는 이유가 뭔가요??

라디오 버튼이라 동일한 버튼을 다시 클릭 했을때는 onChange가 터지지 않았어요
재 클릭으로 노래를 멈추게 하려면 클릭이벤트가 필요했습니다!

cruelladevil · 2023-08-14T07:17:56Z

frontend/src/features/songs/components/Thumbnail.tsx

+const SIZE = {
+  md,
+  lg,
+} as const;


💬 나눠져있는걸 합쳐도 될 것 같아요

Suggested change

const SIZE = {

md,

lg,

} as const;

const SIZE_VARIANTS = {

md: css`

width: 60px;

height: 60px;

`,

lg: css`

width: 70px;

height: 70px;

`,

};

확실히 굳이 나뉘어져 있을 필요가 없네요!
좋은 의견 감사합니다~!

cruelladevil · 2023-08-14T07:31:33Z

frontend/src/features/youtube/components/VideoPlayerProvider.tsx

+  const updatePlayerState: YT.PlayerEventHandler<YT.PlayerEvent> = useCallback(({ target }) => {
+    const state = target.getPlayerState();
+
+    setPlayerState(state);
+  }, []);


💬 onStateChange는 왜 여기서 정의되어 넘겨주고있고,
player는 왜 onReady에서 set하는건가요?

대화를 통해 의미파악 완료했습니다 ㅎ
프로바이더 한곳에서 관리하도록 변경했습니다!

cruelladevil · 2023-08-14T09:43:15Z

frontend/src/shared/components/Timer/TimerProvider.tsx

💬 커스텀훅이 아니라 컨텍스트인 이유가 있을까요?

이 부분은 타이머를 커먼으로 가져가느냐,
도메인 종속으로 가져가면서 유튜브 플레이어와 합체시키느냐
고민하다가 그대로 남은 부분입니다.

현 상태로는 코난 말씀대로 커스텀훅인 편이 훨씬 좋겠네요!
다만 아직 (제 머리속)정리가 덜 되어서, 추후에 기능이 정리되면 그때 변경해도 될까요?

변경된 관리방식적용

Creative-Lee

리뷰 감사합니다!
답변 및 리뷰 반영 했으니 확인해주세요 ㅎㅎ

본문 코멘트에 대한 답변도 남깁니다 ㅎ

useCallback의 사용 기준을 모르겠어요.
충분히 이해됩니다..하하...
제가 의식적으로 메모이제이션을 활용해보고자 노력중인데,
아직 손에 익지 않다보니 어떤 부분은 적용되고, 어떤 부분은 적용되지 않은 상태일거에요.
기준은 랜더링에 의해 새롭게 생성될 필요가 없는 값, 함수에는 모두 적용하자! 인데
빼먹은 부분도 많습니다.
성장하는 과정 어딘가 같아요.. 넓은 아량으로 이해해 주시면 감사하겠습니다!
videoPlayer가 ref로 바뀐 이유가 궁금해요.

플레이어를 상태에 담아 사용했을 때 이점이 없다고 생각했습니다.
랜더링에 영향을 미치지 않는 요소라고 생각했기 때문이에요!
우코와 종종 이야기 나왔던 부분이라 이번 기회로 수정했습니다.
💬 긴가 민가 한데, state로 관리했을 때 이점이 있을까요? 따로 이야기 나눠보시죠!

Creative-Lee · 2023-08-15T02:18:55Z

frontend/src/features/comments/components/CommentList.tsx

-  if (!comments) {
-    return null;
-  }
-


커밋을 잘 나누지 못했네요...
만약 comments 배열 자체가 없거나, comments 배열은 있는데 코멘트가 없어도
랜더링은 진행되어서 댓글 0개는 보여주는게 좋겠다 생각했어요!

Creative-Lee · 2023-08-15T02:42:32Z

frontend/src/features/songs/components/KillingPartInterface.tsx

+const DEFAULT_PART_ID = -1;
+
+const KillingPartInterface = ({ killingParts, songId }: KillingPartInterfaceProps) => {
+  const [nowPlayingTrack, setNowPlayingTrack] = useState<KillingPart['id']>(DEFAULT_PART_ID);


제 설명이 다소 애매하고 부족했네요.
생각했던 부분을 전부 나열하는게 오히려 좋을것 같아서 아래에 다 적어봅니다!

{killingPart && <CommentList songId={id} partId={killingParts[killingRank! - 1].id} />}

리팩터링 전에는 page컴포넌트가 KillingRank 상태를 가지고 있었고, rank로 killingPart를 찾고,
이걸 코멘트 리스트에 id를 넘기는데 쓰이고 있었어요.
이 부분에서 기존에 있던 KillingRank state대신, nowPlayingTrack state에 담는 값을 id로 바꾸면서
하나로 통일하면 좋겠다고 생각했어요~

또 이 과정에서 "part의 id는 순위가 변동된다고 변하지 않고, rank는 순위에 변동에 따라 바뀌니깐
rank가 어떤걸 정하는 내부적 기준이 되면 언젠가 의미적으로 햇갈릴 수 있겠다" 막연하게 생각하기도 했고요!

이제 변동 가능한 rank 대신 고정값인 id로 관리합니다!

이 문장 하나로는 택도 없었네요 ㅠ

역으로 생각하면 서버에서 내려주는 killingParts에는 rank가 순서대로 고정적으로 담겨있고,
나머지 정보가 변동된다고 해석할 수도 있을 것 같은데 뭐가 고정이고 뭐가 변동이라는 말인가요?

코난의 의문이 확실히 이해됩니다!
혹시 위 이유들을 읽어보시고 여전히 의문이 남으면 직접 이야기 해주세요!

Creative-Lee · 2023-08-15T02:43:03Z

frontend/src/features/songs/components/KillingPartTrack.stories.tsx

-  };
-
-  const isPlaying = killingPart.rank === nowPlayingTrack;
+  const isNowPlayingTrack = killingPart.rank === nowPlayingTrack;


이런일이! 수정하겠습니다!

Creative-Lee · 2023-08-15T03:04:21Z

frontend/src/features/songs/components/KillingPartTrack.tsx

@@ -13,49 +15,74 @@ import type React from 'react';

 interface KillingPartTrackProps {
  killingPart: KillingPart;
-  isPlaying: boolean;
-  changePlayingTrack: React.ChangeEventHandler<HTMLInputElement>;
+  isNowPlayingTrack: boolean;


개인적으로는 현재 실행중인 트랙이라는 의미를 명확하게 나타내주는게 좋다고 생각했어요
개발 당시에 유튜브 플레이어 관련해서도 isPlaying과 비슷한 의미를 가지는 변수가 있어서 여기서 오는 혼동을 막기위한 이유도 있었어요!

Creative-Lee · 2023-08-15T03:05:06Z

frontend/src/features/songs/components/KillingPartTrack.tsx

-  //   videoPlayer?.playVideo();
-  // };
+  const getPlayIcon = useCallback(() => {
+    if (!isNowPlayingTrack || playerState === 2) {


아 이렇게 접근 가능하군요!
하나 배워갑니다! 바로 반영할게요!

Creative-Lee · 2023-08-15T03:06:32Z

frontend/src/features/songs/components/KillingPartTrack.tsx

-  //   videoPlayer?.seekTo(start, false);
-  //   videoPlayer?.playVideo();
-  // };
+  const getPlayIcon = useCallback(() => {


timer 관련 상태덕분에... 이 함수는 0.1초 마다 매번 새로 만들어지는데요!
타이머 상태로 인한 리랜더링에는 새로 생성되지 않도록 했습니다!

Creative-Lee · 2023-08-15T03:13:07Z

frontend/src/features/songs/components/KillingPartTrack.tsx

+          onChange={toggleTrackPlayAndStop}
+          onClick={toggleTrackPlayAndStop}


라디오 버튼이라 동일한 버튼을 다시 클릭 했을때는 onChange가 터지지 않았어요
재 클릭으로 노래를 멈추게 하려면 클릭이벤트가 필요했습니다!

Creative-Lee · 2023-08-15T03:15:50Z

frontend/src/features/songs/components/Thumbnail.tsx

+const SIZE = {
+  md,
+  lg,
+} as const;


확실히 굳이 나뉘어져 있을 필요가 없네요!
좋은 의견 감사합니다~!

Creative-Lee · 2023-08-15T03:49:20Z

frontend/src/features/youtube/components/VideoPlayerProvider.tsx

+  const updatePlayerState: YT.PlayerEventHandler<YT.PlayerEvent> = useCallback(({ target }) => {
+    const state = target.getPlayerState();
+
+    setPlayerState(state);
+  }, []);


대화를 통해 의미파악 완료했습니다 ㅎ
프로바이더 한곳에서 관리하도록 변경했습니다!

Creative-Lee · 2023-08-15T04:43:48Z

frontend/src/shared/components/Timer/TimerProvider.tsx

이 부분은 타이머를 커먼으로 가져가느냐,
도메인 종속으로 가져가면서 유튜브 플레이어와 합체시키느냐
고민하다가 그대로 남은 부분입니다.

현 상태로는 코난 말씀대로 커스텀훅인 편이 훨씬 좋겠네요!
다만 아직 (제 머리속)정리가 덜 되어서, 추후에 기능이 정리되면 그때 변경해도 될까요?

ukkodeveloper

도밥 고생 많으셨어요! 킬링파트별로 관리하는 것이 어려웠을텐데 대단해요. 코드의 경우는 로직이 복잡하여 많은 부분 신경쓰진 못했지만, 최대한 제 의견이 있는 부분에 의견을 남겼습니다.

렌더된 화면에서 작동 측면에서 두 가지 드릴 말씀이 있습니다.

좋아요 크기

킬링파트 재생이 자동으로 정지되어도 댓글이 보이면 좋을 것 같아요. 재생이 멈추면 보던 댓글이 사라져서 아쉬워요.

추가로 작업 범위가 너무 넓어서, 한번에 코드 리뷰하는 게 조금 힘든 것 같아요! 다음엔 작업 범위를 조금 작게 가져가셔더 좋을 것 같습니다!

ukkodeveloper · 2023-08-15T05:36:00Z

frontend/src/features/songs/components/KillingPartTrack.stories.tsx

@@ -36,21 +39,15 @@ const killingPart: KillingPart = {
 };

 const KillingPartTrackWithHook = () => {
-  const [nowPlayingTrack, setNowPlayingTrack] = useState(-1);
+  const [nowPlayingTrack, setNowPlayingTrack] = useState<KillingPart['id']>(-1);


💬 ide에서는 똑같이 number로 읽더라고요. useState<KillingPart['id']>(-1);로 변경했을 때 어떤 점이 좋나요?!?

저번에도 비슷한 질문을 해주신것 같은데요!
개발자의 타입 추론 목적으로 접근해 보는건 어떨까요??

해당 코드가 항상 killingPart의 id 만을 받아야 하는 상태인것을 아는 상황이라면 number로 두어도 이해가 되겠지만,
코드를 처음 보는 개발자 입장에서는 이해가 어려울 수도 있겠죠!
number는 조금 넓은 타입이니까요.

위와 같은 이유로 저는 객체 타입의 하위 타입? 이라면 조금더 좁혀서 명시해 주고 있습니다!
답변이 되었을까요?

ukkodeveloper · 2023-08-15T05:45:34Z

frontend/src/features/youtube/components/VideoPlayerProvider.tsx


-  const updatePlayer = useCallback((player: YT.Player) => setVideoPlayer(player), []);
+  const pause = useCallback(() => videoPlayer.current?.pauseVideo(), []);
+  const play = useCallback(() => videoPlayer.current?.playVideo(), []);
+
+  const seekTo = useCallback(
+    (start: number) => {
+      videoPlayer.current?.seekTo(start, true);
+      play();
+    },
+    [play]
+  );
+
+  const initPlayer: YT.PlayerEventHandler<YT.PlayerEvent> = useCallback(({ target }) => {
+    videoPlayer.current = target;
+  }, []);
+
+  const updatePlayerState: YT.PlayerEventHandler<YT.PlayerEvent> = useCallback(({ target }) => {
+    const state = target.getPlayerState();
+
+    setPlayerState(state);
+  }, []);


이렇게 함수로 다 래핑한 이유가 궁금해요...(이유: 간단한 로직인데 새로운 함수로 선언되어 있는 부분이 많아서 변수명을 제 뇌 메모리에 쌓아두어야 하는데, 메모리 절대량이 적어요 ㅠㅠ)

프로젝트 내에
videoPlayer.current?.pauseVideo()
videoPlayer.current?.playVideo()
videoPlayer.current?.seekTo()
가 자주 사용되더라고요
자주 사용되는 함수를 미리 선언해두어서 길이를 줄여보았습니다!
seekTo는 PlayVideo와 같이 쓰여서 묶어두었고요

ukkodeveloper · 2023-08-15T05:50:25Z

frontend/src/features/songs/components/KillingPartTrack.tsx

+    if (
+      !isNowPlayingTrack ||
+      playerState === YT.PlayerState.PAUSED ||
+      playerState === YT.PlayerState.BUFFERING


킬링파트를 듣던 중, 인터넷이 불안정하여 재생 중간에 유튜브가 버퍼링되면 다시 reset되나요?

맞습니다... ㅠ
현재 상태로는 일시정지가 없기 때문에 버퍼링시에 멈춰버립니다
반대로 두면 버퍼링시에 멈추지 않고 나아갑니다

-수정-
~~버퍼링시에는 일시정지를 하도록 수정하였습니다!~~

해당 로직을 수정하면 프로그래스 바가 의도대로 동작하지 않아서 추가 수정중입니다!

수정 완료

ukkodeveloper · 2023-08-15T05:59:50Z

frontend/src/shared/components/Timer/TimerProvider.tsx

+  const count = useCallback(() => {
+    setCountedTime((prev) => prev + 0.1);
+  }, []);
+


함수로 감싼 이유가 궁금해요...2

해당 함수가 여러곳에서 사용되고 있었을 때 분리한 부분입니다!
하지만 지금은 딱 1번 사용되고 있으니, 분리되어있을 필요가 없네요! 수정하도록 하겠습니다!

ukkodeveloper · 2023-08-15T06:11:35Z

frontend/src/shared/components/Timer/TimerProvider.tsx

+  const [isStart, setIsStart] = useState(false);
+  const [autoRestart, setAutoRestart] = useState(false);
+
+  const initialTimeRef = useRef(time);


ref로 주신 이유가 있을까요?

cruelladevil · 2023-08-15T07:39:14Z

frontend/src/features/songs/components/KillingPartInterface.tsx

+      <FlexContainer>
+        <TitleWrapper aria-label="Top 3 킬링파트 듣기">
+          <ItalicTitle aria-hidden="true">Top 3</ItalicTitle>
+          <NormalTitle aria-hidden="true"> Killing part</NormalTitle>


Suggested change

<NormalTitle aria-hidden="true"> Killing part</NormalTitle>

<NormalTitle aria-hidden="true">Killing part</NormalTitle>

다른 스타일의 텍스트 사이에 띄어쓰기가 필요해서 넣었습니다!

아하! 띄어쓰기로 간격을 주지말고 TitleWrapper를 flex로 변경하고 간격을 주면 어떨까요??

<TitleWrapper aria-label="Top 3 킬링파트 듣기"> <ItalicTitle aria-hidden="true">Top 3</ItalicTitle> <Spacing direction="horizontal" size={10} /> <NormalTitle aria-hidden="true">Killing part</NormalTitle> </TitleWrapper>

노래 한곡 재생이 끝나도 댓글 창이 유지되도록 수정

1. 구간 반복 기능이 노래의 재생과 타이머에 영향을 미치지 않도록 개선하였음. 개선 전 반복 기능 토글 시 파트의 처음으로 돌아감 타이머가 초기화됨 개선 후 반복 기능 토글 시 토글 기능만 on off 되고 파트의 처음으로 가지 않음. 타이머가 초기화 되지 않음. 기존 개별 Track컴포넌트에 적용했던 TimerProvider를 killingPartInterface 컴포넌트에 적용함.

Creative-Lee · 2023-08-16T05:54:24Z

구간 반복 기능이 노래의 재생과 타이머에 영향을 미치지 않도록 개선하였음.

개선 전
반복 기능 토글 시 파트의 처음으로 돌아감
타이머가 초기화됨

개선 후
반복 기능 토글 시 토글 기능만 on off 되고 파트의 처음으로 가지 않음.
타이머가 초기화 되지 않음.

관련 기능 잘 돌아갑니다 ㅠㅠ

…into refactor/#244

Creative-Lee added 30 commits August 11, 2023 19:02

refactor: Youtube player 초기화관련 상태 제거 및 ref로 대체

9dae4e4

design: 버튼 길이 수정

4d269dc

refactor: msw fixture 데이터 수정

d008412

refactor: 썸네일 컴포넌트 사이즈 props 추가

797e175

chore: 중첩 label 수정

afa7c92

design: 반복 버튼 위치 변경

a40efc6

chore: 중첩 label 추가

c968fbd

refactor: 기존 킬링파트 재생기능 적용

99f3403

test: props 변경으로 인한 스토리 수정

8896774

refactor: msw fixture 데이터 타입에 맞게 수정

8b96c7d

refactor: 비디오 플레이 관련 함수 프로바이더로 분리

a2fb71e

refactor: 반복재생 기능 비활성화

477faa6

refactor: SongDetailPage의 책임분리

378c172

refactor: KillingPartInterface로 부터 전달되는 props 변경

b1c4868

1. 기존 nowPlayingTrack 상태에 사용된 rank는 변동 가능한 값이므로 part의 id로 변경 2. 이에 따른 props 변경

refactor: 플레이어 관련 변수 분리 및 함수 시그니처 변경

e2706fe

refactor: 불필요한 컨텍스트 제거

9aa0718

test: props 및 state 타입 변경으로 인한 스토리 수정

e5b9c39

refactor: 코멘트 컴포넌트 props타입 변경

ad186bc

api 응답 type 그대로 받도록 수정

feat: Timer 컴포넌트 구현

58f05df

refactor: Timer 컴포넌트 Context+provider 화

f7f67cb

1. 테스트가 끝난 컴포넌트의 상태를 공유하기 위해 변경하였음. 2. 파일명 변경

chore: provider화 된 Timer 컴포넌트 스토리 삭제

7475e9c

refactor: 플레이어 상태 별 변수 분리 및 토글 함수 추가

91717d6

refactor: 변수 의미 개선 및 props 변경

8d9f812

1. 기존 isPlaying은 플레이어의 play와 혼동이 생길 가능성이 많았음. 개선을 통해 플레이중인 트랙이라는 의미를 강조함 2. 이에따라 props가 변경되었음

test: props 변경사항 반영

361cb1e

refactor(TimerProvider): 함수명 개선 및 얼리 리턴 적용

9fd9485

chore: 일시정지 아이콘 추가

476d38a

refactor: 유튜브 컴포넌트 개선

4886a6f

1. 플레이어 초기화 이후 ref에 할당하도록 개선 2. 관련 이벤트 핸들러 생성

refactor: 트랙 컴포넌트 정책 변경으로 인한 관련 프로바이더 개선

dee7353

1. 타이머, 플레이어 프로바이더의 함수에서 pause 관련 함수 제거 2. 트랙 컴포넌트의 타이머가 영상 상태와 동기화 되도록 수정

feat: 유튜브 플레이어 조작 시 포커스 아웃 이펙트 구현

af60a62

refactor: 불필요한 변수 삭제

5fad8e4

실행중인 파트 기준이 rank -> partId 로 변경됨에 따라 로직을 삭제함

Creative-Lee added this to the S-HOOK 사용자 플로우 및 편의성 개선 milestone Aug 14, 2023

Creative-Lee requested review from cruelladevil and ukkodeveloper August 14, 2023 06:11

Creative-Lee self-assigned this Aug 14, 2023

test: 트랙 컴포넌트 스토리에 프로바이더 추가

7dfcf04

Creative-Lee commented Aug 14, 2023

View reviewed changes

Creative-Lee mentioned this pull request Aug 14, 2023

[FEAT] 좋아요 기능을 구현한다. #262

Closed

3 tasks

cruelladevil approved these changes Aug 14, 2023

View reviewed changes

Creative-Lee added 4 commits August 15, 2023 11:44

test: 킬링파트 트랙 스토리 수정

e8a3e38

변경된 관리방식적용

refactor: enum 타입 적용

5d60a20

refactor: 썸네일 사이즈 상수 통합

2488d6b

refactor: 플레이어 이벤트 관리 통일 및 타입 변경

ee88e78

Creative-Lee commented Aug 15, 2023

View reviewed changes

fix: 아이콘 함수 분기문의 잘못된 enum 타입 수정

307d4c1

ukkodeveloper reviewed Aug 15, 2023

View reviewed changes

cruelladevil reviewed Aug 15, 2023

View reviewed changes

Creative-Lee added 6 commits August 15, 2023 16:59

refactor: 불필요한 함수 래핑 분리

7c5960e

fix: 버퍼링 시 플레이어 멈추는 상황 해결

98be716

refactor: 댓글 상태 추가 및 사용성 개선

9d1c617

노래 한곡 재생이 끝나도 댓글 창이 유지되도록 수정

refactor: 킬링파트 트랙은 타이머에 관여하지 않도록 개선

84caa6c

test: 스토리북 내 미사용 변수 린트 예외처리

145c1d1

Creative-Lee requested a review from ukkodeveloper August 16, 2023 05:54

ukkodeveloper approved these changes Aug 17, 2023

View reviewed changes

Creative-Lee added 2 commits August 18, 2023 02:20

Merge branch 'main' of https://github.com/woowacourse-teams/2023-shook …

a09260b

…into refactor/#244

fix: main 머지 후 충돌 해결

c3d437f

Creative-Lee merged commit 623fcc7 into main Aug 17, 2023

Creative-Lee deleted the refactor/#244 branch August 17, 2023 17:27


		const Thumbnail = ({ ...props }: ThumbnailProps) => {
		const Thumbnail = ({ size = 'lg', ...props }: ThumbnailProps) => {

	const isNowPlayingTrack = killingPart.rank === nowPlayingTrack;
	const isNowPlayingTrack = killingPart.id === nowPlayingTrack;

	if (!isNowPlayingTrack \|\| playerState === 2) {
	if (!isNowPlayingTrack \|\| playerState === YT.PlayerState.PAUSED) {

		onChange={toggleTrackPlayAndStop}
		onClick={toggleTrackPlayAndStop}

-const SIZE = {
-  md,
-  lg,
-} as const;
+const SIZE_VARIANTS = {
+  md: css`
+    width: 60px;
+    height: 60px;
+  `,
+  lg: css`
+    width: 70px;
+    height: 70px;
+  `,
+};

	<NormalTitle aria-hidden="true"> Killing part</NormalTitle>
	<NormalTitle aria-hidden="true">Killing part</NormalTitle>

Refactor/#244 기존 SongDetailPage 컴포넌트에 새로운 플로우와 시안을 적용한다. #261

Refactor/#244 기존 SongDetailPage 컴포넌트에 새로운 플로우와 시안을 적용한다. #261

Conversation

Creative-Lee commented Aug 14, 2023 • edited Loading

📝작업 내용

추가 작업

Player & Youtube 관련

etc

💬리뷰 참고사항

디자인 변경 전

디자인 변경 후

#️⃣연관된 이슈

github-actions bot commented Aug 14, 2023 • edited Loading

Unit Test Results

Creative-Lee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cruelladevil left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creative-Lee Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Creative-Lee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creative-Lee Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

ukkodeveloper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukkodeveloper Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creative-Lee Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Creative-Lee Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

ukkodeveloper Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creative-Lee commented Aug 16, 2023

Creative-Lee commented Aug 14, 2023 •

edited

Loading

github-actions bot commented Aug 14, 2023 •

edited

Loading

cruelladevil left a comment •

edited

Loading

Creative-Lee Aug 15, 2023 •

edited

Loading

Creative-Lee Aug 15, 2023 •

edited

Loading

ukkodeveloper Aug 15, 2023 •

edited

Loading

Creative-Lee Aug 15, 2023 •

edited

Loading

Creative-Lee Aug 16, 2023 •

edited

Loading

ukkodeveloper Aug 15, 2023 •

edited

Loading