Feat/#166 메인페이지에서 킬링파트 등록 인기순 음악을 보여준다 #175

cruelladevil · 2023-08-01T10:56:00Z

📝작업 내용

이번 PR에서 작업한 내용을 간략히 설명(이미지 첨부 가능)

메인페이지에서 킬링파트 등록 인기순 노래를 보여줍니다.
- SongPopularPage
- SongRankCard - 등록 인기순 목록에서 하나의 음악 컴포넌트입니다.
- Thumbnail - 이미지를 불러오기 전, 크기를 강제하기 위해 div로 한 번 감쌌습니다.

💬리뷰 참고사항

원활한 리뷰를 위해 전달하고 싶은 맥락(특정 파일, 디렉터리 등등)
리뷰어가 특별히 봐주었으면 하는 부분

[웹 접근성] : SongRankCard 탭하면 -> 방문함, 링크, 킬링파트 인기순 {n}등 {가수} {제목}
[백엔드 미구현] - figma 시안상으로 votes 정보가 있어 컴포넌트를 구현해놓았지만 API 명세에 없기 때문에 현재 주석처리

웹 접근성이 충분한지 검토해주세요.
SongRankCard를 일단 이름을 깊이 고민하지 않고 지었기 때문에 이 부분을 한 번 검토해주세요.

#️⃣연관된 이슈

연관된 이슈 번호를 모두 작성

close #166

github-actions · 2023-08-01T10:57:19Z

Unit Test Results

114 tests 114 ✔️ 3s ⏱️
  34 suites     0 💤
  34 files     0 ❌

Results for commit 34a9f69.

♻️ This comment has been updated with latest results.

Creative-Lee

고생 많으셨습니다!
코멘트 확인하고 답변 부탁드릴게요~
내용이 얼마 없어서 대면으로 바로 하셔도 좋습니다!

Creative-Lee · 2023-08-01T17:00:19Z

frontend/src/pages/SongPopularPage/SongPopularPage.tsx

+
+const SongPopularPage = () => {
+  const { data: popularSongs } = useFetch<PopularSong[]>(() =>
+    fetcher('/songs/recommended', 'GET')


요 통신 로직은 도메인별로 분리되어 관리되면 좋을 것 같아요.
컴포넌트에서 바로 url을 적어 바로 통신하니, 다소 명령형으로 느껴지는 것 같네요
apis/ 하위에 분리하면 어떨까요?

관리 측면에서 추상화 레이어를 하나 추가하여 api 폴더에 쿼리 함수가 있어야 된다는 의미인건가요?
우리 팀의 query 로직이 부족한 부분이 많아서 이에 대해 정리하는 시간을 한 번 가지면 좋을 것 같아요.

관리 측면에서 추상화 레이어를 하나 추가하여 api 폴더에 쿼리 함수가 있어야 된다는 의미인건가요?

이 의도라면 이번 PR에서는 이대로 통과시켜도 될까요?

넵 맞습니다~
도메인이 들어간 실제 통신 로직은 함수로 한번 감싸지고,
그것을 hook에 넣어서 사용하는게 좋겠다고 생각했어요!

코난 말대로 당장 api 로직이 먼저 수정되어야 할 것 같네요 ㅋㅋ 넘어가시죠!

frontend/src/router.tsx

Creative-Lee · 2023-08-01T17:05:27Z

frontend/src/pages/SongPopularPage/SongPopularPage.style.ts

+  color: white;
+
+  font-size: 20px;
+  font-weight: 800;


💬 bold를 의도하신거라면 700이 어떨까요?

이 부분도 따로 이야기 해보시죵

https://developer.mozilla.org/ko/docs/Web/CSS/font-weight

bold의 기본이 700이군요!
폰트와 관련해서 이야기 나눠보면 좋을 것 같습니다.

일단 700으로 수정해놓겠습니다

✍️ 수정 커밋 - 1b5a397

frontend/src/components/SongRankCard/Thumbnail.tsx

frontend/src/components/SongRankCard/Thumbnail.style.ts

frontend/src/components/SongRankCard/SongRankCard.style.ts

ukkodeveloper

고생하셨어요. 컴포넌트가 간단해서 별로 말씀드릴게 없네요. SongRankCard 누를 때 background 분홍생 뜨는 부분 외에는 사소한 커멘트라 바로 approve할게요.

ukkodeveloper · 2023-08-02T07:25:42Z

frontend/src/pages/SongPopularPage/SongPopularPage.style.ts

+  font-weight: 800;
+`;
+
+export const PopularSongList = styled.section`


section > ol > li . 이런 계층 구조였으면 좋겠어요. section이 없는 건 ok. 하지만, ol > li 가 더 시맨틱하다고 생각합니다.

목록이라는 점을 고려하지 못하였네요 😨

✍️ 수정 커밋 - 2cc5ef8

ukkodeveloper · 2023-08-02T07:39:38Z

frontend/src/pages/SongPopularPage/SongPopularPage.style.ts

+  &:active {
+    background-color: ${({ theme }) => theme.color.primary};
+  }


💬 이 스타일 속성 때문에 노래 아이템을 클릭하면 순간 분홍색으로 깜빡하거든요. 어차피 다른 페이지로 바로 넘어가는 것임에도 순간 분홍색 번쩍이는 게 불필요하고 정신 없게 느껴집니다.. 이 부분 코난은 어떻게 생각하시나요?

active pseudo class는 마우스 버튼을 누르는 순간부터 떼는 시점까지를 나타냅니다.
https://developer.mozilla.org/ko/docs/Web/CSS/:active

불필요하고 정신 없게 느껴집니다..

라고 느껴진다면 삭제하는 것이 좋을 것 같네요!

✍️ 수정 커밋 - cf679c2

ukkodeveloper · 2023-08-02T07:40:43Z

frontend/src/pages/SongPopularPage/SongPopularPage.tsx

+        {popularSongs.map(({ id, thumbnail, title, singer }, i) => (
+          <StyledLink
+            key={id}
+            to={`/song/${id}`}


router.tsx 소스코드 정리하면서 도메인 path 상수화해도 좋을 것 같네요! 이 부분 이슈로 남겨놓겠습니다.

frontend/src/components/SongRankCard/SongRankCard.style.ts

ukkodeveloper · 2023-08-02T07:55:02Z

frontend/src/components/SongRankCard/Thumbnail.tsx

+
+interface ThumbnailProps extends ImgHTMLAttributes<HTMLImageElement> {}
+
+const Thumbnail = ({ ...props }: ThumbnailProps) => {


(props: ThumbnailProps) => { 이렇게 작성해도 될 것 같네요.

const Thumbnail = ({ newProp, anyOtherProp, ...props }: ThumbnailProps) => {
위처럼 prop이 추가될 상황을 고려하고 작성하였습니다.
props와 { ...props}의 차이중 특별히 고려할 사항이 없으면 수정하지 않고 넘어가겠습니다 😊

frontend/src/components/SongRankCard/Thumbnail.tsx

- 순서 있는 목록이므로 ol, li로 표현

Creative-Lee

반영 사항 다 확인했습니다!
고생 많으셨습니다 ㅎ

- thumbnail을 imageUrl로 변경 - totalVoteCount 추가

cruelladevil added 10 commits August 1, 2023 18:43

feat: 인기있는 킬링파트 서버응답 msw 구현

92e5fc7

feat: Thumbnail 컴포넌트 구현

66e0d18

feat: SongRankCard 컴포넌트 구현

6ce500a

feat: SongPopularPage 구현

7490919

refactor: alt 속성을 prop으로 받지 않고, src prop을 thumbnail로 변경

ce93f0d

feat: SongRankCard에 color white 적용

942b4f6

feat: SongRankCard 스토리 작성

660b79d

design: 미구현으로 인한 하드코딩된 votes 정보 주석처리

3b8a8e8

design: focus 및 active 시 background color 적용

eee83f0

feat: lazy loading 및 default image 적용

fdda8cb

cruelladevil added [ 🌞 FE ] 프론트엔드 크루들의 빛나는 개발 이야기 하나둘셋 호! ✨ Feat 꼼꼼한 기능 구현 중요하죠 labels Aug 1, 2023

cruelladevil added this to the 노래 목록 및 상세 정보 조회 기능 구현 milestone Aug 1, 2023

cruelladevil requested review from Creative-Lee and ukkodeveloper August 1, 2023 10:56

cruelladevil self-assigned this Aug 1, 2023

Creative-Lee requested changes Aug 1, 2023

View reviewed changes

design: font-weight 700으로 수정

1b5a397

ukkodeveloper approved these changes Aug 2, 2023

View reviewed changes

cruelladevil added 4 commits August 2, 2023 17:09

fix: overflow hidden 수정 및 인라인 스타일 삭제

1e1950a

design: SongRankCard의 Link에서 active 요소 삭제

cf679c2

design: 태그를 시맨틱하게 수정

2cc5ef8

- 순서 있는 목록이므로 ol, li로 표현

chore: 컴포넌트명을 PopularSongItem으로 수정

e948798

Creative-Lee self-requested a review August 2, 2023 11:55

Creative-Lee approved these changes Aug 2, 2023

View reviewed changes

cruelladevil added 4 commits August 3, 2023 11:45

refactor: 인기있는 킬링파트 서버 엔드포인트 변경

1f303f8

test: api fixture 백엔드와 동기화

90ab7f8

feat: 백엔드 응답 변경

ada740c

- thumbnail을 imageUrl로 변경 - totalVoteCount 추가

design: 썸네일 크기 및 간격 조정

1215b89

Merge remote-tracking branch 'origin' into feat/#166

34a9f69

cruelladevil merged commit 116317f into main Aug 3, 2023

cruelladevil deleted the feat/#166 branch August 3, 2023 03:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/#166 메인페이지에서 킬링파트 등록 인기순 음악을 보여준다 #175

Feat/#166 메인페이지에서 킬링파트 등록 인기순 음악을 보여준다 #175

cruelladevil commented Aug 1, 2023

github-actions bot commented Aug 1, 2023 •

edited

Loading

Creative-Lee left a comment

Creative-Lee Aug 1, 2023

cruelladevil Aug 2, 2023

Creative-Lee Aug 2, 2023

Creative-Lee Aug 1, 2023

cruelladevil Aug 2, 2023

cruelladevil Aug 2, 2023

ukkodeveloper left a comment

ukkodeveloper Aug 2, 2023

cruelladevil Aug 2, 2023

ukkodeveloper Aug 2, 2023

cruelladevil Aug 2, 2023

ukkodeveloper Aug 2, 2023

ukkodeveloper Aug 2, 2023

cruelladevil Aug 2, 2023

Creative-Lee left a comment


		interface ThumbnailProps extends ImgHTMLAttributes<HTMLImageElement> {}

		const Thumbnail = ({ ...props }: ThumbnailProps) => {

Feat/#166 메인페이지에서 킬링파트 등록 인기순 음악을 보여준다 #175

Feat/#166 메인페이지에서 킬링파트 등록 인기순 음악을 보여준다 #175

Conversation

cruelladevil commented Aug 1, 2023

📝작업 내용

💬리뷰 참고사항

#️⃣연관된 이슈

github-actions bot commented Aug 1, 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

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

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 left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 1, 2023 •

edited

Loading