[Feat] 선착순 예외 처리 및 UX 개선 #190
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
빌드를 성공했습니다! 🎉 |
jhj2713
reviewed
Aug 22, 2024
Comment on lines
39
to
42
| const serverDate = new Date(rushData.serverTime).toISOString().split("T")[0]; | ||
|
|
||
| const currentEvent = rushData.events.find((event) => { | ||
| const eventDate = new Date(event.startDateTime).toISOString().split("T")[0]; |
There was a problem hiding this comment.
time 빼고 date를 반환해오는 것 같은데 아래에서도 쓰는 것 같아서 util로 빼는건 어때용?
function get...(dateTime: string) {
return new Date(dateTime).toISOString().split("T");
}
const [serverDate] = get...(rushData.serverTime)약간 요런 식으로,,?!
Comment on lines
57
to
64
| useEffect(() => { | ||
| if ( | ||
| runCountdown !== null && | ||
| runCountdown <= 0 && | ||
| gameState.phase === CARD_PHASE.IN_PROGRESS | ||
| ) { | ||
| dispatch({ type: RUSH_ACTION.SET_PHASE, payload: CARD_PHASE.COMPLETED }); | ||
| } |
There was a problem hiding this comment.
사소한거긴한데 if 안에 조건절이 길어지니까 정확히 어떤 상황에 발생하는지 헷갈리는 것 같아용 한번 묶어서 조건 넣어줘도 괜찮을 것 같아용
Suggested change
| useEffect(() => { | |
| if ( | |
| runCountdown !== null && | |
| runCountdown <= 0 && | |
| gameState.phase === CARD_PHASE.IN_PROGRESS | |
| ) { | |
| dispatch({ type: RUSH_ACTION.SET_PHASE, payload: CARD_PHASE.COMPLETED }); | |
| } | |
| useEffect(() => { | |
| const isInProgress = runCountdown !== null && runCountdown <= 0 && gameState.phase === CARD_PHASE.IN_PROGRESS; | |
| if (isInProgress) { | |
| dispatch({ type: RUSH_ACTION.SET_PHASE, payload: CARD_PHASE.COMPLETED }); | |
| } |
약간 요런 느낌으로!
Comment on lines
67
to
69
| if (initialRunCountdown === null || runCountdown === null) { | ||
| return <div className="flex flex-col gap-3 justify-center items-center h-[150px]"></div>; | ||
| } |
There was a problem hiding this comment.
아래 return 문의 container와 스타일이 같은데 굳이 따로 조건 처리한 이유가 있나용? return 문 안에서 삼항 연산으로 처리해줘도 괜찮지 않나 싶어서요!
| } | ||
| }, [isSuccessRush, rushData]); | ||
|
|
||
| if (initialPreCountdown === null) return null; |
There was a problem hiding this comment.
조건을 충족하지 않으면 null을 반환하는 로직이 몇 번 보이는데, admin에서 사용하는 Suspense 같은 컴포넌트 만들어서 조건을 충족하지 않으면 빈 컴포넌트 리턴하게 하는 방식으로 처리해주는건 어떤가용? HOC 내에서 처리하는 거니까 중간에 조건문이 등장하지 않아도 돼서 로직이 더 깔끔해질 것 같아서용
import { PropsWithChildren } from "react";
interface SuspenseProps extends PropsWithChildren {
isLoading?: boolean;
}
export default function Suspense({ children, isLoading = false }: SuspenseProps) {
return (
<>
{isLoading ? (
<></>
) : (
children
)}
</>
);
}
export default function Countdown() {
return (
<Suspense isLoading={initialPreCountdown === null}>
<motion.p className="h-heading-2-bold pt-10" {...SCROLL_MOTION(ASCEND)}>
이제 곧 하단에 밸런스 게임 주제가 공개돼요!
</motion.p>
<CountdownTimer initialPreCountdown={initialPreCountdown} />
<RushShareLink />
</Suspense>
);
}
Comment on lines
69
to
74
| const isValidData = | ||
| isWinner !== undefined && rank !== undefined && totalParticipants !== undefined; | ||
| if (!isValidData) return null; | ||
|
|
||
| return ( | ||
| <motion.div | ||
| className="flex flex-col justify-center items-center gap-8" | ||
| {...SCROLL_MOTION(ASCEND)} | ||
| > | ||
| <p className="h-heading-2-bold pt-10">{message}</p> | ||
| <span className="h-body-1-regular text-n-black flex flex-col justify-center items-center"> | ||
| <p>*이 화면은 밤 12시 이후 재접속이 불가능합니다.</p> | ||
| <p>입력하신 전화번호로 경품 수령 관련 메시지가 전송될 예정이에요.</p> | ||
| </span> | ||
| <div className="flex flex-col gap-12 w-[834px] h-[400px] bg-n-neutral-50 rounded-800 pt-12 pb-[94px] px-[57px] justify-between break-keep"> | ||
| <span className="flex flex-col justify-center items-center text-center gap-3 text-n-black"> | ||
| <p className="h-heading-4-bold">나의 선착순 등수</p> | ||
| <span className="flex gap-3 justify-center items-center "> | ||
| <p className="h-heading-1-bold">{rank}등</p> | ||
| <p className="h-body-1-regular text-n-neutral-500"> | ||
| / {totalParticipants.toLocaleString("en-US")}명 중 | ||
| </p> | ||
| </span> | ||
| <> |
There was a problem hiding this comment.
얘도 Suspense 만들어서 쓰면 더 깔끔할 것 같아용
| <span className="flex flex-col justify-center items-center text-center gap-3 text-n-black"> | ||
| <p className="h-heading-4-bold">나의 선착순 등수</p> | ||
| <span className="flex gap-3 justify-center items-center "> | ||
| <p className="h-heading-1-bold">{rank}등</p> |
There was a problem hiding this comment.
rank는 localeString 처리 안 해줘도 되나용? 아주아주 혹시나 1000 단위 넘어가는 경우를 대비해서요,,ㅎ
Comment on lines
+23
to
+45
| dispatch({ | ||
| type: RUSH_ACTION.SET_CARD_OPTIONS, | ||
| payload: { | ||
| option: CARD_OPTION.LEFT_OPTIONS, | ||
| updates: { | ||
| mainText: todayRushEventData.leftOption.mainText, | ||
| subText: todayRushEventData.leftOption.subText, | ||
| color: leftColor, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| dispatch({ | ||
| type: RUSH_ACTION.SET_CARD_OPTIONS, | ||
| payload: { | ||
| option: CARD_OPTION.RIGHT_OPTIONS, | ||
| updates: { | ||
| mainText: todayRushEventData.rightOption.mainText, | ||
| subText: todayRushEventData.rightOption.subText, | ||
| color: rightColor, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
left랑 right랑 같이 세팅해주는 action 만드는건 어때용? useFetchRushResult나 useFetchRushBalance에서도 left, right 같이 업데이트해줬던 것 같아서 한 번에 업데이트하는 action이 있으면 더 좋을 것 같아서요!
There was a problem hiding this comment.
그것도 좋은 방식인 것 같아여! 나중에 적용해볼게용 🤩
| option: CardOption; | ||
| } | ||
|
|
||
| export const getOptionRatio = ({ gameState, option }: GetOptionRatioProps) => { |
There was a problem hiding this comment.
gameState에서 cardOptions 속성만 사용되는데 gameState 통째로 넘겨주는 이유가 따로 있을까용? cardOptions만 넘겨주는게 더 깔끔하지 않을까 싶어서용
|
빌드를 성공했습니다! 🎉 |
|
빌드를 성공했습니다! 🎉 |
|
빌드를 성공했습니다! 🎉 |
|
빌드를 성공했습니다! 🎉 |
|
빌드를 성공했습니다! 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🖥️ Preview
default.mov
✏️ 한 일
- Flux 패턴 적용
- 패칭 및 상태 변경 로직 훅으로 분리
close 선착순 예외 처리 #170
close 선착순 UX 개선 #189
❗️ 발생한 이슈 (해결 방안)
❓ 논의가 필요한 사항