[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #269

rbgksqkr · 2024-09-14T17:26:16Z

Issue Number

#261

As-Is

게임화면에서 옵션만 선택하고 선택 버튼까지 안누르는 사용자가 많음
선택 버튼을 눌러야한다고 인지하지 못하는 사용자가 많음

To-Be

선택 버튼을 누르지 않아도, 선택된 옵션으로 투표
선택한 옵션의 id가 필요해 SelectContainer 하위에 Timer 포함
API 호출을 관리하는 useVoteTimer와 타이머만 관리하는 useTimer로 분리
투표 끝난 여부 폴링을 Timer 컴포넌트에서 하여 리렌더링 최적화

Check List

테스트가 전부 통과되었나요?
모든 commit이 push 되었나요?
merge할 branch를 확인했나요?
Assignee를 지정했나요?
Label을 지정했나요?

Test Screenshot

개선 전 / 개선 후 : 폴링으로 인한 불필요한 리렌더링 없애기

(Optional) Additional Description

GamePage에서 TopicContainer, Timer, SelectContainer 로 분리했었음
사용자 피드백으로 인해 선택 버튼을 누르지 않아도, 선택된 옵션으로 투표 기능 필요
따라서 SelectContainer에서 관리하는 선택한 옵션 데이터가 Timer 에도 필요해짐
Timer를 SelectContainer 하위 컴포넌트로 삽입

🌸 Storybook 배포 주소

https://woowacourse-teams.github.io/2024-ddangkong/storybook/

useon

PR 본문에 잘 설명해 주셔서 어느 작업 위주로 코드가 개선되었는지 파악하기가 쉬웠어요 !!! 어떻게 최적화 작업이 이루어졌는지 이미지도 첨부해 주셔서 좋았어요 ~ ! ! 구조적으로 분리가 필요한 부분도 분리가 되어 더 좋아졌네요 ~~ 몇 가지 코멘트 남겼고, 어프루브 드립니다 따봉 따봉😝!

useon · 2024-09-16T05:12:56Z

frontend/src/pages/ReadyPage/ReadyPage.tsx

 import ReadyMembersContainer from '@/components/ReadyMembersContainer/ReadyMembersContainer';
 import StartButtonContainer from '@/components/StartButtonContainer/StartButtonContainer';

 const ReadyPage = () => {
  return (
    <AsyncErrorBoundary pendingFallback={<ReadySkeleton />}>
-      <div css={readyPageLayout}>
+      <Content>


🌸 칭찬 🌸

공통 레이아웃을 사용하도록 수정한 것 너무 좋네요 !

useon · 2024-09-16T07:29:52Z

frontend/src/components/SelectContainer/Timer/hooks/useTimer.ts

      clearInterval(timeout.current);
    }
-  }, [leftRoundTime]);
+  }, [isVoteTimeout]);


💭 질문 💭

의존성 배열에 isSelectedOption, vote를 추가해 주지 않아도 괜찮은지 궁금합니다! 외부에서 주입 받는 값이라서 전달되는 값이 바뀌면 또 실행이 되어야 하지 않나 해서요!! 🙂

기능상으론 문제가 없습니다! 왜냐면 해당 로직은 타이머가 다됐을 때 선택된 옵션이 있으면 투표하는 로직이니까 타이머가 끝난 여부만 알면 되기 때문이죠
그런데 규칙대로 간다면 isSelectedOption을 넣는 게 맞죠! 추가하겠습니다. vote는 함수라서 매번 실행될텐데 useCallback으로 감싸는 걸 말씀하신걸까요??

근데 지금 테스트해보니 mutation이라서 넣어도 괜찮을 것 같네용

흠흠 그런데 또 다시 생각해 보니 vote는 mutation이라서 react-query 내부적으로 관리를 하니 의존성 배열에 넣지 않아도 의도대로 동작하지 않을까 하는 생각이 드네요 . . 😅 악 !!! 이랬다 저랬다 레전드 지송합니다 마루 .. 어떻게 생각하시나요 ..

ㅋㅋㅋㅋㅋㅋㅋㅋㅋ 원래도 의도대로 다 동작합니다! react가 제시하는 경고를 최대한 지키기 위해 코멘트를 반영한거였구요

vote가 mutation이 아니라면 오히려 문제가 발생합니다.
vote는 함수이므로 리렌더링 될 때마다 참조값이 바뀌는거죠 그래서 리렌더링될 때마다 useEffect가 실행될 것이고요!
하지만 우리가 의도한 동작은 timeout이 될 때 동작하는 걸 원하기 때문에 불필요한 리렌더링이 발생합니다. 그래서 useCallback 이야기도 했었던 겁니다!

아래 코드로 vote 함수만 일반 함수로 바꿔보면 왼쪽 스크린샷에 클릭할 때마다 찍힌 콘솔을 확인할 수 있습니다.

useEffect(() => { console.log('useEffect'); if (isVoteTimeout) { if (isSelectedOption) { vote(); } clearInterval(timeout.current); } }, [isVoteTimeout, isSelectedOption, vote]);

흠흠 그런데 또 다시 생각해 보니 vote는 mutation이라서 react-query 내부적으로 관리를 하니 의존성 배열에 넣지 않아도 의도대로 동작하지 않을까 하는 생각이 드네요 . .

어떤 걸 우려하는건지 말씀해주실 수 있나요???

아니에요 이해되었습니다 제가 혹시나 불필요한 코멘트를 했나 싶어서 잠시 혼란이 .. 😄 !!!! 코멘트 반영 감사드려요르 😇

useon · 2024-09-16T07:31:28Z

frontend/src/components/SelectContainer/Timer/Timer.tsx

@@ -1,6 +1,6 @@
 import { useParams } from 'react-router-dom';

-import useRoundTimer from './Timer.hook';
+import useVoteTimer from './hooks/useVoteTimer';


🌸 칭찬 🌸

안 그래도 커밋대로 차근 차근 보다가 useRoudTimer와 useTimer에 대해 네이밍이 헷갈린다고 생각했는데 수정 좋네요 따봉 !!!!!!!!!!

useon · 2024-09-16T07:33:05Z

frontend/src/components/SelectContainer/Timer/hooks/useTimer.ts

      clearInterval(timeout.current);
    }
-  }, [leftRoundTime]);
+  }, [isVoteTimeout]);

  useEffect(() => {
    const DECREASE_RATE = INITIAL_WIDTH / convertMsecToSecond(timeLimit);


🙏 제안 🙏

이러한 계산 코드들은 유틸로 분리해서 관리하는 것은 어떻게 생각하셔요 ? 🤭

오호 한줄짜리 계산 로직도 보통 다 분리하는 편이신가요?? 간단해서 안만들긴 했는데 함수로 만들고 의미를 부여한다면 좋긴 하겠네용

지금 timer util 파일이 존재해서 분리할 수 있는 것은 분리해서 관리해도 좋지 않을까 생각했어요 😊 반영해 주셔서 감사합니다 !!

useon · 2024-09-16T07:34:47Z

frontend/src/pages/GamePage/GamePage.stories.ts

+
+type Story = StoryObj<typeof meta>;
+
+export const 게임_화면: Story = {};


🌸 칭찬 🌸

페이지 단위의 스토리북이 생겨서 페이지 단의 UI를 한번에 챙겨보기 좋겠네요 안 그래도 컴포넌트 단위로 생성된 스토리북이 많아져서 보기 힘들다고 생각했었는데 굿 아이디어 말우 ^~^

novice0840

수고 많았습니다~
files changed 개수가 많긴 한데 리팩토링 한 부분들이 많아서 새롭게 추가되었기 보다는 기존 기능을 수정한 부분인 것 같네요.

크게 신경 쓰이는 부분은 없어서 이번에는 바로 approve를 하겠습니다!

novice0840 · 2024-09-18T09:04:23Z

frontend/src/apis/balanceContent.ts

-interface RoundVoteIsFinished {
+interface VoteIsFinished {
  isFinished: boolean;
 }


💭 질문 💭

RoundVote라는 용어가 모두 Vote로 바뀌었는데
그럼 이제 Round라는 용어는 사용이 안되게 바뀌었나요?

음 라운드 결과에서 다음 게임 화면으로 넘어가는 부분에서도 roundIsFinished 라는 필드가 쓰여 헷갈려서 바꿨습니다! 포메는 어떤가요??

novice0840 · 2024-09-18T09:07:14Z

frontend/.storybook/preview.tsx

+              <ToastProvider>
+                <Story />
+              </ToastProvider>


💭 질문 💭

현재 PR의 storybook 링크에서 ReadyMembersContainer 컴포넌트의 ToastContext 부분에서 에러가 발생하는데 확인 한 번 부탁합니다

음 확인해보니 포메 PR에 커밋이 푸시되서 decorator에 ToastProvider가 포함되지 않았던 것 같아요! 제 PR 머지되면 상관없을 것 같네용

novice0840 · 2024-09-18T09:14:41Z

frontend/src/components/SelectContainer/Timer/Timer.styled.ts

@@ -32,7 +32,6 @@ const shake = keyframes`
 export const timerLayout = css`
  display: flex;
  position: relative;
-  flex-basis: 5%;


💭 질문 💭

storybook에서 Timer UI를 봤는데 감이 잘 안 잡혀서요. flex-basis: 5%;는 원래는 어떤 역할을 하고 있었나요?

flex-basis는 flex box 내에서의 비율을 의미합니다! 그래서 원래는 Content의 레이아웃에 따라 비율을 가졌는데, Timer가 SelectContainer 내부로 들어가서 부모 레이아웃의 비율이 달라져 수정하였습니다!

novice0840 · 2024-09-18T10:45:28Z

frontend/src/components/SelectContainer/Timer/Timer.test.tsx

+      act(() => {
+        jest.advanceTimersByTime(timeLimit);
+      });


🌸 칭찬 🌸

신기한 기능을 하는 함수네요. 이렇게 하면 타이머 기능을 테스트할 때에도 타이머 시간을 다 기다리지 않아도 되겠군요!

novice0840 · 2024-09-18T11:25:17Z

frontend/src/components/SelectContainer/Timer/Timer.util.ts

+export const calculateWidthDecreasePercent = (width: number, timeLimit: number) => {
+  return width / timeLimit;
+};


💭 질문 💭

함수명과 인자의 관계가 잘 이해가 안되는데 width에도 ms 가 들어가서 "전체 시간 대비 남은 시간"은 반환하는 함수인가요?

오 저도 맥락을 모르고 보면 파악하기 어려울 것 같네용
msec 단위의 timeLimit을 인자로 받고, timeLimit으로 width를 나눈 단위값만큼을 1초마다 줄이는 과정입니다. 그런데 width와 timeLimit이 같이 있으니 어색하네요.
이야기한대로 좀더 일반적인 함수명을 써서 바꾸면 좋을 것 같아요!

export const calculateUnitRatio = (total: number, divisor: number) => { return total / divisor; };

novice0840 · 2024-09-19T02:25:04Z

frontend/src/components/SelectContainer/hooks/useVoteIsFinishedQuery.ts

+    },
+    enabled,
+    refetchInterval: POLLING_DELAY,
+    refetchIntervalInBackground: true,


🙏 제안 🙏

저희 서비스가 모바일에서 사용자가 홈 화면으로 이동했을 때 polling이 동작을 안 하면서 UI 전환이 제때 일어나지 않아 특정 페이지에 갇히는 상황이 발생하는 것 같은데
혹시 refetchIntervalInBackground를 이것 뿐만 아니라 다른 polling도 refetchIntervalInBackground을 추가해 줄 수 있나요?

오 원래 안되는줄 알았는데 백그라운드에 폴링을 열어두면 페이지 전환이 되더라구요 굿굿 반영했습니다!

…안넘어간 오류 개선 #261

rbgksqkr added 17 commits September 6, 2024 10:44

merge: conflict 해결 #261

a97403b

refactor: SelectButton 쿼리 호출 제거하고 props로 받기 #261

30495a6

refactor: SelectContainer hooks 디렉토리로 분리 #261

b8723e8

refactor: 라운드 타이머가 바뀔 때마다 실행하지 않고, 타이머가 끝날 때만 clearTimeout #261

7ff3866

merge: conflict 해결 #261

5b15ddc

refactor: Timer hooks 디렉토리로 분리 #261

1b9d1bf

refactor: 타임아웃되도 선택 API를 호출하도록 구조 리팩토링 #261

f8fa798

test: 게임 화면 UI 테스트를 위해 storybook 작성 #261

e8735a3

design: timer 구조 변경으로 인한 스타일 수정 #261

aa4419b

design: 대기 화면 공통 레이아웃 적용 #261

0267606

refactor: 시간 재는 타이머 관련 로직을 API 로직과 분리 #261

408bec7

refactor: 투표 시간 측정 타이머에 맞게 voteTimer로 이름 수정 #261

ab85a2c

refactor: 선택 완료 버튼 클릭 이벤트리스너명 vote로 수정 #261

b22e1a6

test: 타이머가 종료되었을 때 선택된 옵션이 있으면 투표 테스트 코드 작성 #261

bbd04ff

refactor: 투표 종료 여부를 Timer 컴포넌트가 가지면서 리렌더링 최적화 #261

309a7bb

refactor: roundVoteIsFinished 네이밍을 voteIsFinished로 수정 #261

55cedd6

refactor: Timer를 SelectContainer 하위 디렉토리로 위치 수정 #261

0fa5023

rbgksqkr added ♻️ refactor 리팩토링 ✨ feat 기능 추가 🫧 FE front end labels Sep 14, 2024

rbgksqkr added this to the FE Sprint5 milestone Sep 14, 2024

rbgksqkr requested review from useon and novice0840 September 14, 2024 17:26

rbgksqkr self-assigned this Sep 14, 2024

rbgksqkr linked an issue Sep 14, 2024 that may be closed by this pull request

[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #261

Closed

1 task

useon approved these changes Sep 16, 2024

View reviewed changes

rbgksqkr added 2 commits September 16, 2024 22:13

refactor: useTimer 의존성 배열 수정 #261

85ce80b

refactor: timer 유틸함수 분리 #261

20d4ddf

novice0840 approved these changes Sep 19, 2024

View reviewed changes

refactor: 모호한 함수명 수정 #261

2a572d7

rbgksqkr added 3 commits September 19, 2024 20:05

refactor: refetchInterval 및 refetchIntervalInBackground 를 통해 게임 화면으로 …

2f8e096

…안넘어간 오류 개선 #261

fix: 투표를 한 상태여도 타이머가 끝난 후 투표 API 요청을 하는 오류 해결 #261

70b2a92

test: 타이머가 종료되었을 때 이미 투표를 했다면 또 투표를 하지 않는 테스트 코드 구현 #261

b127e40

rbgksqkr merged commit af686cd into develop Sep 19, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #269

[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #269

rbgksqkr commented Sep 14, 2024 •

edited

Loading

useon left a comment

useon Sep 16, 2024

useon Sep 16, 2024

rbgksqkr Sep 16, 2024

rbgksqkr Sep 16, 2024

useon Sep 17, 2024

rbgksqkr Sep 17, 2024

useon Sep 18, 2024

useon Sep 16, 2024

useon Sep 16, 2024

rbgksqkr Sep 16, 2024

useon Sep 17, 2024

useon Sep 16, 2024

novice0840 left a comment

novice0840 Sep 18, 2024

rbgksqkr Sep 19, 2024

novice0840 Sep 18, 2024

rbgksqkr Sep 19, 2024 •

edited

Loading

novice0840 Sep 18, 2024

rbgksqkr Sep 19, 2024

novice0840 Sep 18, 2024

novice0840 Sep 18, 2024

rbgksqkr Sep 19, 2024

novice0840 Sep 19, 2024

rbgksqkr Sep 19, 2024


		type Story = StoryObj<typeof meta>;

		export const 게임_화면: Story = {};

[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #269

[REFACTOR] 게임화면에서 선택 버튼을 누르지 않아도 옵션이 선택되도록 리팩토링 #269

Conversation

rbgksqkr commented Sep 14, 2024 • edited Loading

Issue Number

As-Is

To-Be

Check List

Test Screenshot

(Optional) Additional Description

🌸 Storybook 배포 주소

useon 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

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

🌸 칭찬 🌸

novice0840 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

💭 질문 💭

rbgksqkr Sep 19, 2024 • edited Loading

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

rbgksqkr commented Sep 14, 2024 •

edited

Loading

rbgksqkr Sep 19, 2024 •

edited

Loading