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] RushEvent 컴포넌트 구현 #70

Merged
merged 6 commits into from
Jul 31, 2024
Merged

[Feat] RushEvent 컴포넌트 구현 #70

merged 6 commits into from
Jul 31, 2024

Conversation

sooyeoniya
Copy link
Member

@sooyeoniya sooyeoniya commented Jul 30, 2024

🖥️ Preview

image

✏️ 한 일

❗️ 발생한 이슈 (해결 방안)

👺 커밋명 하나 잘못 올려버렸다..

refactor: lotteryEventData storybook 경로 수정
-> feat: RushEvent 컴포넌트 UI 구현

❓ 논의가 필요한 사항

  • 아까 백엔드와 상의 후에 "오늘 날짜"를 기준으로 "선착순 마감"을 처리했는데 생각해보니 이슈가 있음,,
    • "선착순 마감" 처리 자체가 오늘 날짜 기준이 아니라 선착순 밸런스 게임 자체가 끝났을 때 "선착순 마감" 처리가 되어야 함
    • 이 상태 자체를 프론트에서 전부 처리하기에는 무리가 있는 것 같음
    • 아예 이 작업 자체를 백엔드로 넘겨서 오늘 날짜를 기준으로 따로 분기처리 하는 것이 아니라, 해당 날짜의 게임이 끝났는지에 대한 상태를 받아서 분기처리 해야할 듯

@sooyeoniya sooyeoniya added the feat 기능 구현 label Jul 30, 2024
@sooyeoniya sooyeoniya requested a review from jhj2713 July 30, 2024 10:19
@sooyeoniya sooyeoniya self-assigned this Jul 30, 2024
Copy link

빌드를 성공했습니다! 🎉

Copy link

빌드를 성공했습니다! 🎉

Copy link
Member

@jhj2713 jhj2713 left a comment

Choose a reason for hiding this comment

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

고생하셨서용🙌

@@ -0,0 +1,8 @@
export const formatDate = (dateString: string): string => {
const date = new Date(dateString);
Copy link
Member

Choose a reason for hiding this comment

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

이때 date 포맷이 아닌 string이 들어오는 경우에 대한 예외처리를 해줘도 좋을 것 같아욥!

Copy link
Member Author

Choose a reason for hiding this comment

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

감사합니당 반영했습니다!

const date = new Date(dateString);
const month = date.getMonth() + 1;
const day = date.getDate();
const dayOfWeek = ["일", "월", "화", "수", "목", "금", "토"][date.getDay()];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const dayOfWeek = ["일", "월", "화", "수", "목", "금", "토"][date.getDay()];
const DAY_OF_WEEK = ["일", "월", "화", "수", "목", "금", "토"];
...
const today = dayOfWeek[date.getDay()];

["일", "월", "화", "수", "목", "금", "토"]랑 index가 바로 붙어있으니까 쪼금 헷갈리는 것 같아서 위 예시처럼 따로 상수로 빼주는게 좀 더 가독성이 좋지 않을까 하는 생각입니덩

Copy link
Member Author

Choose a reason for hiding this comment

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

역시!! 너무 좋은생각입니다 :) 리팩토링 여신님 말따르겠습니다!

className={`relative w-[160px] h-[200px] py-7 px-5 rounded-500 bg-n-white flex flex-col gap-4 justify-between items-center border ${borderClass} ${opacityClass}`}
>
<p
className={`h-body-2-bold ${isTodayEvent ? "text-s-red" : "text-n-neutral-950"} text-nowrap`}
Copy link
Member

Choose a reason for hiding this comment

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

요 삼항 연산자도 borderClassopacityClass 처럼 빼주는게 통일성 있지 않을까 싶어용

Copy link
Member Author

Choose a reason for hiding this comment

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

네 바로 반영하겠습니당!!

Copy link

빌드를 성공했습니다! 🎉

Copy link

빌드를 성공했습니다! 🎉

Copy link

빌드를 실패했습니다. ❌ 자세한 내용은 로그를 참고해주세요.

Copy link

빌드를 성공했습니다! 🎉

@sooyeoniya sooyeoniya merged commit baa25ce into dev Jul 31, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 기능 구현
Projects
None yet
Development

Successfully merging this pull request may close these issues.

선착순 이벤트 경품 컴포넌트 구현
2 participants