Skip to content
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.

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

[Feat/#321] 등록한 공연 목록 중 종료된 공연 라벨링 #336

Merged
merged 6 commits into from
Aug 12, 2024

Conversation

ocahs9
Copy link
Contributor

@ocahs9 ocahs9 commented Aug 9, 2024

📌 관련 이슈번호

🎟️ PR 유형

어떤 변경 사항이 있나요?

  • 새 기능 추가
  • 버그 수정
  • CSS 등 사용자 UI 디자인 변경
  • 리팩토링

✅ Key Changes

이번 PR에서 작업한 내용을 간략히 설명해주세요

  1. 작업 내용

📢 To Reviewers

  • 등록한 공연 중, 종료된 공연 라벨링해달라는 QA를 반영했습니다.
  • 그러나 우선 공통 컴포넌트인 Labal을 사용하느라 D-day 가 뜨는 것도 그대로 냅두었는데,
    만약 종료된 공연에만 라벨링을 해달라는 의도였다면, 추후 수정하도록 하겠습니다.
  • 코드 구조 이해를 돕기 위해 주석을 달아두었습니다.
  • 추가로, API 주소 변경해두었습니다. (예매자 입금 여부 수정 API, 예매자 삭제 API )

📸 스크린샷

image

🔗 참고 자료

Copy link

github-actions bot commented Aug 9, 2024

PR 작성하느라 고생 많았어요!! 라벨 잘 지정되었는지 확인 한 번 해 주기 🫶

@github-actions github-actions bot added the ✨ FEAT 기능 구현 label Aug 9, 2024
Copy link
Contributor

@sinji2102 sinji2102 left a comment

Choose a reason for hiding this comment

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

진짜 레전드 뿌듯 발생!!! 라벨 저밖에 안 쓰지만 그래도 편하게 쓰려고 공통으로 뺀 거긴 했는데 다른 곳에 또 쓰이니까 너무 뿌듯하네요...!!

저는 dueDate를 서버에서 넘겨주는 데이터를 받고 있는데 서버에서 dueDate 값으로 공연이 얼마나 남았는지 보내줄 수 있는 거로 알아요!! 어떤 게 더 좋은 방법인지 확신할 수는 없지만 현재 클라에서 dueDate를 따로 계산하고 있는 것 같아서 서버에서 날짜 받는 김에 dueDate도 요청하면 더 편하고 깔끔한 코드가 되지 않으까,, 싶습니다,,

<S.CardInfo>
<S.CardInfoTextBox
<>
<Labal dueDate={dueDate} />
Copy link
Contributor

Choose a reason for hiding this comment

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

p5) Label 컴포넌트 공통으로 뺀 보람이 있네요.... 너무 뿌듯해요 키키

Copy link
Member

Choose a reason for hiding this comment

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

  1. image
  2. image

p1) 관리 페이지에서는 1번 사진처럼 라벨이 공통적으로 보이고, 메인 페이지에서는 2번 사진처럼 보여용!
같은 컴포넌트를 사용하는데, 저렇게 보이면 안될 것 같아요~!

문제는 Label 컴포넌트에 dueDate props를 어떻게 주냐인데, 요거 관련해서는 @sinji2102@ocahs9 오늘 회의 끝나고 Live Share로 고쳐보는 거 어떨까요~??

Copy link
Member

@pepperdad pepperdad left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!
리뷰 남긴 거 확인하고, 오늘 밤에 시간되면 얘기해봐용 ~~

@@ -24,40 +25,69 @@ const RegisteredCard = ({
};

const getShowTypeText = (key: SHOW_TYPE_KEY): ShowTypes => SHOW_TYPE[key];
const calculateDueDate = (dateString: string): number => {
Copy link
Member

Choose a reason for hiding this comment

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

p5) 가독성을 위해서 띄어쓰기 해주세용

Suggested change
const calculateDueDate = (dateString: string): number => {
const calculateDueDate = (dateString: string): number => {

Copy link
Member

Choose a reason for hiding this comment

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

p5) 그리고 이 함수는 전체 유틸함수로 빼도 될 것 같아요!

<S.CardInfo>
<S.CardInfoTextBox
<>
<Labal dueDate={dueDate} />
Copy link
Member

Choose a reason for hiding this comment

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

  1. image
  2. image

p1) 관리 페이지에서는 1번 사진처럼 라벨이 공통적으로 보이고, 메인 페이지에서는 2번 사진처럼 보여용!
같은 컴포넌트를 사용하는데, 저렇게 보이면 안될 것 같아요~!

문제는 Label 컴포넌트에 dueDate props를 어떻게 주냐인데, 요거 관련해서는 @sinji2102@ocahs9 오늘 회의 끝나고 Live Share로 고쳐보는 거 어떨까요~??

Copy link

Copy link
Member

@pepperdad pepperdad left a comment

Choose a reason for hiding this comment

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

LGTM

@ocahs9 ocahs9 merged commit a460ccf into develop Aug 12, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ FEAT 기능 구현
Projects
Status: 🎉Done
Development

Successfully merging this pull request may close these issues.

[ Feat ] 등록된 공연 목록 중 '종료된 공연' 라벨링
3 participants