-
Notifications
You must be signed in to change notification settings - Fork 0
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
[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #269
Changes from 17 commits
a97403b
30495a6
b8723e8
7ff3866
5b15ddc
1b9d1bf
f8fa798
e8735a3
aa4419b
0267606
408bec7
ab85a2c
b22e1a6
bbd04ff
309a7bb
55cedd6
0fa5023
85ce80b
20d4ddf
2a572d7
2f8e096
70b2a92
b127e40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ interface myGameStatusParams { | |
currentRound: number; | ||
} | ||
|
||
interface RoundVoteIsFinished { | ||
interface VoteIsFinished { | ||
isFinished: boolean; | ||
} | ||
Comment on lines
-22
to
24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 질문 💭RoundVote라는 용어가 모두 Vote로 바뀌었는데 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 음 라운드 결과에서 다음 게임 화면으로 넘어가는 부분에서도 roundIsFinished 라는 필드가 쓰여 헷갈려서 바꿨습니다! 포메는 어떤가요?? |
||
|
||
|
@@ -113,12 +113,12 @@ export const fetchMatchingResult = async ({ | |
}; | ||
|
||
// 현재 라운드가 끝났는지 여부 확인하기 | ||
export const fetchRoundVoteIsFinished = async ({ | ||
export const fetchVoteIsFinished = async ({ | ||
contentId, | ||
roomId, | ||
}: ContentResultParams): Promise<RoundVoteIsFinished> => { | ||
}: ContentResultParams): Promise<VoteIsFinished> => { | ||
const res = await fetcher.get({ | ||
url: API_URL.roundVoteIsFinished(roomId, contentId), | ||
url: API_URL.voteIsFinished(roomId, contentId), | ||
}); | ||
|
||
const data = await res.json(); | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,6 @@ const shake = keyframes` | |
export const timerLayout = css` | ||
display: flex; | ||
position: relative; | ||
flex-basis: 5%; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 질문 💭storybook에서 Timer UI를 봤는데 감이 잘 안 잡혀서요. flex-basis: 5%;는 원래는 어떤 역할을 하고 있었나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. flex-basis는 flex box 내에서의 비율을 의미합니다! 그래서 원래는 Content의 레이아웃에 따라 비율을 가졌는데, Timer가 SelectContainer 내부로 들어가서 부모 레이아웃의 비율이 달라져 수정하였습니다! |
||
align-items: center; | ||
width: 100%; | ||
height: 3.2rem; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import { act, renderHook } from '@testing-library/react'; | ||
|
||
import useTimer from './hooks/useTimer'; | ||
import { convertMsecToSecond, formatLeftRoundTime } from './Timer.util'; | ||
|
||
describe('Timer 테스트', () => { | ||
describe('formatTimer 유틸 함수 테스트', () => { | ||
it('30초의 제한시간을 입력받으면 00:30 으로 포맷팅한 string값을 반환한다.', () => { | ||
const formattedTimer = formatLeftRoundTime(30); | ||
|
||
expect(formattedTimer).toBe('00:30'); | ||
}); | ||
|
||
it('10000 밀리초의 제한시간을 입력받으면 10초를 number 타입으로 반환한다.', () => { | ||
const convertedTime = convertMsecToSecond(10000); | ||
|
||
expect(convertedTime).toBe(10); | ||
}); | ||
}); | ||
describe('Timer 훅 테스트', () => { | ||
jest.useFakeTimers(); | ||
const voteMock = jest.fn(); | ||
const timeLimit = 10000; | ||
|
||
it('타이머가 종료되었을 때 선택 완료를 누르지 않아도 선택된 옵션이 있으면 투표한다.', () => { | ||
const isSelectedOption = true; | ||
|
||
const { result } = renderHook(() => | ||
useTimer({ timeLimit, isSelectedOption, vote: voteMock }), | ||
); | ||
|
||
// act : 인자로 받은 함수를 실행시켜서 가상의 DOM(jsdom)에 적용하는 역할 | ||
// 상태 변경과 그로 인한 DOM 업데이트가 모두 완료된 후에 테스트가 실행되도록 보장 | ||
act(() => { | ||
jest.advanceTimersByTime(timeLimit); | ||
}); | ||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🌸 칭찬 🌸신기한 기능을 하는 함수네요. 이렇게 하면 타이머 기능을 테스트할 때에도 타이머 시간을 다 기다리지 않아도 되겠군요! |
||
|
||
expect(result.current.leftRoundTime).toBe(0); | ||
expect(voteMock).toHaveBeenCalled(); | ||
}); | ||
it('타이머가 종료되었을 때 선택된 옵션이 없다면 투표되지 않고 기권한다.', () => { | ||
const isSelectedOption = false; | ||
|
||
const { result } = renderHook(() => | ||
useTimer({ timeLimit, isSelectedOption, vote: voteMock }), | ||
); | ||
|
||
act(() => { | ||
jest.advanceTimersByTime(timeLimit); | ||
}); | ||
|
||
expect(result.current.leftRoundTime).toBe(0); | ||
expect(voteMock).not.toHaveBeenCalled(); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { useParams } from 'react-router-dom'; | ||
|
||
import useRoundTimer from './Timer.hook'; | ||
import useVoteTimer from './hooks/useVoteTimer'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🌸 칭찬 🌸안 그래도 커밋대로 차근 차근 보다가 useRoudTimer와 useTimer에 대해 네이밍이 헷갈린다고 생각했는데 수정 좋네요 따봉 !!!!!!!!!! |
||
import { | ||
timerIcon, | ||
timerIconShake, | ||
|
@@ -10,12 +10,31 @@ import { | |
timerWrapper, | ||
} from './Timer.styled'; | ||
import { formatLeftRoundTime } from './Timer.util'; | ||
import useVoteIsFinished from '../hooks/useVoteIsFinished'; | ||
|
||
import DdangkongTimer from '@/assets/images/ddangkongTimer.png'; | ||
import useBalanceContentQuery from '@/hooks/useBalanceContentQuery'; | ||
|
||
const Timer = () => { | ||
interface TimerProps { | ||
selectedId: number; | ||
completeSelection: () => void; | ||
showModal: () => void; | ||
} | ||
|
||
const Timer = ({ selectedId, completeSelection, showModal }: TimerProps) => { | ||
const { roomId } = useParams(); | ||
const { barWidthPercent, leftRoundTime, isAlmostFinished } = useRoundTimer(Number(roomId)); | ||
const { balanceContent, isFetching } = useBalanceContentQuery(Number(roomId)); | ||
const { barWidthPercent, leftRoundTime, isAlmostFinished } = useVoteTimer({ | ||
roomId: Number(roomId), | ||
selectedId, | ||
completeSelection, | ||
showModal, | ||
}); | ||
|
||
useVoteIsFinished({ | ||
contentId: balanceContent.contentId, | ||
isFetching, | ||
}); | ||
|
||
return ( | ||
<section css={timerLayout}> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,35 @@ | ||
import { useEffect, useRef, useState } from 'react'; | ||
|
||
import { convertMsecToSecond } from './Timer.util'; | ||
import { convertMsecToSecond } from '../Timer.util'; | ||
|
||
import { POLLING_DELAY } from '@/constants/config'; | ||
import useBalanceContentQuery from '@/hooks/useBalanceContentQuery'; | ||
|
||
const INITIAL_WIDTH = 100; | ||
const DEFAULT_TIME_LIMIT_MSEC = 10000; | ||
const ALMOST_FINISH_SECOND = 5; | ||
|
||
const useRoundTimer = (roomId: number) => { | ||
const { balanceContent } = useBalanceContentQuery(roomId); | ||
const timeLimit = balanceContent.timeLimit || DEFAULT_TIME_LIMIT_MSEC; | ||
interface UseTimerProps { | ||
timeLimit: number; | ||
isSelectedOption: boolean; | ||
vote: () => void; | ||
} | ||
|
||
const useTimer = ({ timeLimit, isSelectedOption, vote }: UseTimerProps) => { | ||
const [leftRoundTime, setLeftRoundTime] = useState(convertMsecToSecond(timeLimit)); | ||
const [barWidthPercent, setBarWidthPercent] = useState(INITIAL_WIDTH); | ||
|
||
const isVoteTimeout = leftRoundTime <= 0; | ||
const isAlmostFinished = leftRoundTime <= ALMOST_FINISH_SECOND; | ||
|
||
const timeout = useRef<NodeJS.Timeout>(); | ||
|
||
useEffect(() => { | ||
if (leftRoundTime <= 0) { | ||
if (isVoteTimeout) { | ||
if (isSelectedOption) { | ||
vote(); | ||
} | ||
clearInterval(timeout.current); | ||
} | ||
}, [leftRoundTime]); | ||
}, [isVoteTimeout]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 질문 💭의존성 배열에 isSelectedOption, vote를 추가해 주지 않아도 괜찮은지 궁금합니다! 외부에서 주입 받는 값이라서 전달되는 값이 바뀌면 또 실행이 되어야 하지 않나 해서요!! 🙂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기능상으론 문제가 없습니다! 왜냐면 해당 로직은 타이머가 다됐을 때 선택된 옵션이 있으면 투표하는 로직이니까 타이머가 끝난 여부만 알면 되기 때문이죠 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 근데 지금 테스트해보니 mutation이라서 넣어도 괜찮을 것 같네용 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 흠흠 그런데 또 다시 생각해 보니 vote는 mutation이라서 react-query 내부적으로 관리를 하니 의존성 배열에 넣지 않아도 의도대로 동작하지 않을까 하는 생각이 드네요 . . 😅 악 !!! 이랬다 저랬다 레전드 지송합니다 마루 .. 어떻게 생각하시나요 .. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ㅋㅋㅋㅋㅋㅋㅋㅋㅋ 원래도 의도대로 다 동작합니다! react가 제시하는 경고를 최대한 지키기 위해 코멘트를 반영한거였구요 vote가 mutation이 아니라면 오히려 문제가 발생합니다. 아래 코드로 vote 함수만 일반 함수로 바꿔보면 왼쪽 스크린샷에 클릭할 때마다 찍힌 콘솔을 확인할 수 있습니다. useEffect(() => {
console.log('useEffect');
if (isVoteTimeout) {
if (isSelectedOption) {
vote();
}
clearInterval(timeout.current);
}
}, [isVoteTimeout, isSelectedOption, vote]);
어떤 걸 우려하는건지 말씀해주실 수 있나요??? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아니에요 이해되었습니다 제가 혹시나 불필요한 코멘트를 했나 싶어서 잠시 혼란이 .. 😄 !!!! 코멘트 반영 감사드려요르 😇 |
||
|
||
useEffect(() => { | ||
const DECREASE_RATE = INITIAL_WIDTH / convertMsecToSecond(timeLimit); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙏 제안 🙏이러한 계산 코드들은 유틸로 분리해서 관리하는 것은 어떻게 생각하셔요 ? 🤭 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 한줄짜리 계산 로직도 보통 다 분리하는 편이신가요?? 간단해서 안만들긴 했는데 함수로 만들고 의미를 부여한다면 좋긴 하겠네용 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지금 timer util 파일이 존재해서 분리할 수 있는 것은 분리해서 관리해도 좋지 않을까 생각했어요 😊 반영해 주셔서 감사합니다 !! |
||
|
@@ -37,9 +43,9 @@ const useRoundTimer = (roomId: number) => { | |
return () => { | ||
clearInterval(timeout.current); | ||
}; | ||
}, [balanceContent, timeLimit]); | ||
}, [timeLimit]); | ||
|
||
return { leftRoundTime, barWidthPercent, isAlmostFinished }; | ||
}; | ||
|
||
export default useRoundTimer; | ||
export default useTimer; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import useTimer from './useTimer'; | ||
|
||
import useCompleteSelectionMutation from '@/components/common/SelectButton/SelectButton.hook'; | ||
import useBalanceContentQuery from '@/hooks/useBalanceContentQuery'; | ||
|
||
const DEFAULT_TIME_LIMIT_MSEC = 10000; | ||
|
||
interface UseVoteTimerProps { | ||
roomId: number; | ||
selectedId: number; | ||
completeSelection: () => void; | ||
showModal: () => void; | ||
} | ||
|
||
const useVoteTimer = ({ roomId, selectedId, completeSelection, showModal }: UseVoteTimerProps) => { | ||
const { balanceContent } = useBalanceContentQuery(roomId); | ||
const timeLimit = balanceContent.timeLimit || DEFAULT_TIME_LIMIT_MSEC; | ||
|
||
const { mutate: vote } = useCompleteSelectionMutation({ | ||
selectedId, | ||
contentId: balanceContent.contentId, | ||
completeSelection, | ||
showModal, | ||
}); | ||
|
||
const { leftRoundTime, barWidthPercent, isAlmostFinished } = useTimer({ | ||
timeLimit, | ||
isSelectedOption: Boolean(selectedId), | ||
vote, | ||
}); | ||
|
||
return { leftRoundTime, barWidthPercent, isAlmostFinished }; | ||
}; | ||
|
||
export default useVoteTimer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 질문 💭
현재 PR의 storybook 링크에서 ReadyMembersContainer 컴포넌트의 ToastContext 부분에서 에러가 발생하는데 확인 한 번 부탁합니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 확인해보니 포메 PR에 커밋이 푸시되서 decorator에 ToastProvider가 포함되지 않았던 것 같아요! 제 PR 머지되면 상관없을 것 같네용