refactor: 임시 저장 api 보완 #250
Conversation
Walkthrough이 풀 리퀘스트에서는 Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/hooks/game/useBalanceGameCreation.ts (2)
80-96: 유효성 검사 함수의 메시지 관리 제안
validateStage함수에서 반환하는 오류 메시지가 하드코딩되어 있습니다. 향후 다국어 지원이나 유지보수를 위해 메시지를 상수나 별도의 메시지 관리 모듈로 분리하는 것을 권장합니다.
126-137: 함수 정의 방식의 일관성 개선 제안
handleOptionChange와handleDescriptionChange함수의 정의 방식이 다릅니다. 하나는 커링을 사용하고, 다른 하나는 일반 함수로 정의되어 있습니다. 코드의 일관성과 가독성을 위해 동일한 방식으로 함수를 정의하는 것을 권장합니다.예를 들어,
handleOptionChange를 일반 함수로 변경할 수 있습니다:- const handleOptionChange = - (optionType: 'A' | 'B') => (event: React.ChangeEvent<HTMLInputElement>) => { + const handleOptionChange = ( + optionType: 'A' | 'B', + event: React.ChangeEvent<HTMLInputElement> + ) => { updateOption(currentStage, optionType, { name: event.target.value }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
src/components/organisms/BalanceGameCreation/BalanceGameCreation.style.ts(1 hunks)src/components/organisms/BalanceGameCreation/BalanceGameCreation.tsx(3 hunks)src/hooks/game/useBalanceGameCreation.ts(5 hunks)src/hooks/game/useStageNavigation.ts(0 hunks)src/pages/BalanceGameCreationPage/BalanceGameCreationPage.tsx(2 hunks)src/types/game.ts(1 hunks)src/utils/balanceGameUtils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/hooks/game/useStageNavigation.ts
✅ Files skipped from review due to trivial changes (1)
- src/pages/BalanceGameCreationPage/BalanceGameCreationPage.tsx
🔇 Additional comments (9)
src/hooks/game/useBalanceGameCreation.ts (3)
35-51: 입력 초기화를 위한 useEffect 사용 적절성 확인
clearInput 상태를 관리하기 위해 useEffect와 타이머를 활용한 로직이 적절하게 구현되었습니다. 타이머의 참조를 timerRef로 관리하여 메모리 누수를 방지하고 있습니다.
97-107: 마지막 단계 처리 로직 확인 필요
handleNextStage 함수에서 currentStage < totalStage - 1 조건으로 다음 단계로의 이동을 제한하고 있습니다. 마지막 단계에서도 유효성 검사를 수행하고 필요한 처리가 이루어지는지 확인이 필요합니다.
마지막 단계에서의 처리가 올바르게 이루어지는지 확인해주세요.
9-11:
함수 시그니처 변경에 따른 영향 확인 필요
useBalanceGameCreation 훅에 showToast 매개변수가 추가되고 currentStage가 삭제되었습니다. 이 변경으로 인해 해당 훅을 사용하는 모든 곳에서 함수 호출부를 업데이트해야 합니다.
기존에 useBalanceGameCreation을 사용하던 코드를 모두 확인하여 새로운 시그니처에 맞게 수정해야 합니다.
src/components/organisms/BalanceGameCreation/BalanceGameCreation.style.ts (1)
33-39: Toast 모달 스타일 추가 적절성 확인
toastModalStyling 스타일이 추가되어 토스트 모달의 위치와 레이어가 적절하게 설정되었습니다. 고정 위치와 변환(transform) 속성을 사용하여 화면 중앙에 표시되도록 구현되었습니다.
src/utils/balanceGameUtils.ts (1)
11-11: fileId 초기값 변경에 따른 영향 확인 필요
createInitialGameStages 함수에서 fileId의 초기값이 0에서 null로 변경되었습니다. 이로 인해 fileId를 참조하는 다른 부분에서 null 값을 적절히 처리하는지 확인이 필요합니다.
fileId가 null인 경우에 대한 예외 처리가 누락되지 않았는지 점검해주세요.
Also applies to: 19-19
src/types/game.ts (1)
79-79: TempGameOption에 imgUrl 속성 추가 확인
TempGameOption 인터페이스에 imgUrl 속성이 추가되어 이미지 URL을 옵션에서 사용할 수 있게 되었습니다. 이 변경은 이미지 처리 기능을 향상시킵니다.
src/components/organisms/BalanceGameCreation/BalanceGameCreation.tsx (3)
8-9: useToastModal 및 ToastModal 모듈 임포트 확인
useToastModal 훅과 ToastModal 컴포넌트가 올바르게 임포트되었습니다. 토스트 메시지 기능 추가에 필요한 모듈들이 적절히 포함되었습니다.
34-34: 토스트 모달 상태 관리 추가 확인
useToastModal 훅을 사용하여 isVisible, modalText, showToastModal 상태를 관리하고 있습니다. 이는 사용자에게 유효성 검사 메시지를 표시하기 위한 적절한 구현입니다.
107-109: ToastModal 컴포넌트의 적절한 렌더링 확인
ToastModal 컴포넌트가 조건부 렌더링을 통해 화면에 표시되고 있습니다. isVisible 상태에 따라 모달이 나타나도록 구현하여 사용자 경험을 향상시켰습니다.
| const validateStage = (): true | string => { | ||
| if (!currentOptions[0]?.name.trim() || !currentOptions[1]?.name.trim()) { | ||
| return '모든 옵션의 설명을 입력해주세요!'; | ||
| } | ||
|
|
||
| const handleDescriptionChange = ( | ||
| optionType: 'A' | 'B', | ||
| event: React.ChangeEvent<HTMLInputElement>, | ||
| ) => { | ||
| const { value } = event.target; | ||
| updateOption(currentStage, optionType, { description: value }); | ||
| const hasBothImages = | ||
| currentOptions[0]?.imgUrl.trim() && currentOptions[1]?.imgUrl.trim(); | ||
| const hasNoImages = | ||
| !currentOptions[0]?.imgUrl.trim() && !currentOptions[1]?.imgUrl.trim(); | ||
|
|
||
| if (!(hasBothImages || hasNoImages)) { | ||
| return 'A와 B의 이미지가 모두 없거나 모두 있어야 합니다!'; | ||
| } | ||
|
|
||
| return true; | ||
| }; |
There was a problem hiding this comment.
38b30fc 메시지 텍스트는 전부 상수화 처리했습니다!!
| }; | ||
| console.log('임시 저장 실행 데이터:', tempGameData); | ||
| console.table(tempGameData); |
There was a problem hiding this comment.
머지 전에 불필요한 콘솔문은 삭제 부탁드려요!! 🙌
There was a problem hiding this comment.
저장 관련 추가 수정 사항이 생겨서 우선 삭제하지 않았었는데, 우선 삭제하도록 하겠습니다 ㅎㅎ
There was a problem hiding this comment.
86e0f59 콘솔 제거 완료입니닷!!
| const handleOptionChange = | ||
| (optionType: 'A' | 'B') => (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| updateOption(currentStage, optionType, { name: event.target.value }); | ||
| }; | ||
|
|
||
| const handleDescriptionChange = ( | ||
| optionType: 'A' | 'B', | ||
| event: React.ChangeEvent<HTMLInputElement>, | ||
| ) => { | ||
| const { value } = event.target; | ||
| updateOption(currentStage, optionType, { description: value }); | ||
| }; |
There was a problem hiding this comment.
두 함수가 비슷한 작업을 수행하고 있는데, 통합하여 props로 전달받은 후 상태를 업데이트 하는 방식은 어떨까요?
| const handleOptionChange = | |
| (optionType: 'A' | 'B') => (event: React.ChangeEvent<HTMLInputElement>) => { | |
| updateOption(currentStage, optionType, { name: event.target.value }); | |
| }; | |
| const handleDescriptionChange = ( | |
| optionType: 'A' | 'B', | |
| event: React.ChangeEvent<HTMLInputElement>, | |
| ) => { | |
| const { value } = event.target; | |
| updateOption(currentStage, optionType, { description: value }); | |
| }; | |
| const handleOptionUpdate = | |
| (optionType: 'A' | 'B', field: 'name' | 'description') => | |
| (event: React.ChangeEvent<HTMLInputElement>) => { | |
| const { value } = event.target; | |
| updateOption(currentStage, optionType, { [field]: value }); | |
| }; |
There was a problem hiding this comment.
오 같은 핸들러로 처리하는 방법도 있었네요! 시도해보고 추가 코멘트 남기겠습니다!
There was a problem hiding this comment.
2523713 에서 처리했습니다!
| const [currentOptions, setCurrentOptions] = useState<BalanceGameOption[]>([]); | ||
| const [currentDescription, setCurrentDescription] = useState<string>(''); | ||
| const [clearInput, setClearInput] = useState(false); | ||
|
|
||
| const timerRef = useRef<number | NodeJS.Timeout | null>(null); |
There was a problem hiding this comment.
timerRef의 타입을 number | NodeJS.Timeout로 정의하고 초기값을 굳이 null로 설정하지 않아도 안전하게 동작할 수 있다고 생각하는데, null을 포함신 이유가 따로 있을까용?
There was a problem hiding this comment.
성능적인 면에서는 차이가 없으나, useRef<number | NodeJS.Timeout>(); 로 처리하게 될 경우 timerRef.current 에 대한 추론이 모호해질 수 있는 거 같습니다!
추가적인 undefined 에 대한 검증을 별도로 하지 않고, 타이머가 설정되지 않았다는 걸 명시하기 위해 null을 사용했습니다ㅏㅏ
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/organisms/BalanceGameCreation/BalanceGameCreation.tsx (1)
70-75: 렌더링 시 생성되는 익명 함수 최소화 제안
onChange핸들러에 익명 함수를 직접 전달하면 컴포넌트가 렌더링될 때마다 새로운 함수가 생성되어 성능에 영향을 줄 수 있습니다. 핸들러 함수를 컴포넌트 내부에서 미리 선언하거나useCallback훅을 사용하여 함수를 메모이제이션하는 것을 권장합니다.Also applies to: 86-91
src/constants/message.ts (2)
51-51: 오타 수정: 불필요한 '가' 제거
ERROR.IMAGE.DELETE메시지에서 불필요한 '가'가 있습니다.'이미지 삭제에 실패했습니다. 다시 시도해주세요.'로 수정해주세요.다음의 변경사항을 적용해주세요:
- DELETE: '이미지가 삭제에 실패했습니다. 다시 시도해주세요.', + DELETE: '이미지 삭제에 실패했습니다. 다시 시도해주세요.',
91-91: 불필요한 공백 제거
SUCCESS.TEMPGAME.SAVE메시지의 시작 부분에 불필요한 공백이 있습니다. 공백을 제거해주세요.다음의 변경사항을 적용해주세요:
- SAVE: ' 임시 저장이 완료되었습니다!', + SAVE: '임시 저장이 완료되었습니다!',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
src/components/organisms/BalanceGameCreation/BalanceGameCreation.tsx(5 hunks)src/constants/message.ts(2 hunks)src/hooks/api/game/useCreateBalanceGameMutation.ts(0 hunks)src/hooks/api/game/useSaveTempGameMutation.ts(0 hunks)src/hooks/game/useBalanceGameCreation.ts(5 hunks)src/pages/BalanceGameCreationPage/BalanceGameCreationPage.tsx(7 hunks)
💤 Files with no reviewable changes (2)
- src/hooks/api/game/useSaveTempGameMutation.ts
- src/hooks/api/game/useCreateBalanceGameMutation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/BalanceGameCreationPage/BalanceGameCreationPage.tsx
🔇 Additional comments (1)
src/hooks/game/useBalanceGameCreation.ts (1)
22-22: 타이머 참조 변수의 타입 정의 개선 제안
timerRef의 타입을 React.MutableRefObject<number | null>로 변경하는 것이 좋을 것 같습니다. 브라우저 환경에서는 setTimeout이 숫자 ID를 반환하므로 NodeJS.Timeout 타입은 불필요합니다.
💡 작업 내용
💡 자세한 설명
✅ 임시 저장, 임시 저장 불러오기
와 같이 임시 저장 시에는 imgUrl을 사용하지 않고, 임시 저장 불러오기 시에는 사용하기 때문에 해당 항목을 옵셔널 처리 하였습니다.
해당 함수는 유틸리티 함수로, 기본 밸런스 게임 세트를 생성하는 함수입니다. 기존 fileId의 기본 값이 0으로 처리되어있었는데, 실제 fileId와 동기화 시키기 위해 해당 항목이 없다면 null 처리를 하였습니다.
+.mp4
default.mp4
✅ BalanceGameCreation 내부의 훅의 순환 참조 문제
useBalanceGameCreation훅과useStageNavigation훅의 순환 참조 문제를 해결해야 했습니다.- 문제 원인 : currentStage 의 경우 useStageNavigation 의 결과 물인데 useBalanceGameCreation 훅의 매개 변수로 사용되고 있고, validateStage는 useBalanceGameCreation의 결과물인데 useStageNavigation의 매개변수로 사용되고있음.
-해결 방안 : 원래 해당 훅의 의도는 스테이지 이동과 스테이지 데이터 관리를 분할하기 위해서 였습니다. 제약 조건을 추가된다면 해당 훅의 복잡성이 증가하기 때문에 하나의 훅으로 스테이지 이동 및 데이터 관리를 모두 담당하도록 처리했습니다.
<선택지 관련 요구사항>
useBalanceGameCreation 훅의
의 함수에서 확인할 수 있습니다.
default.mp4
✅ 선택지의 이벤트 핸들러가 정상적으로 감지되지 않는 문제
default.mp4
📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #246
Summary by CodeRabbit
New Features
toastModalStyling으로 모달의 스타일을 개선하였습니다.TempGameOption인터페이스에imgUrl속성이 추가되어 유연성이 향상되었습니다.Bug Fixes
fileId속성을 기본값0에서null로 수정하였습니다.Chores
useCreateBalanceGameMutation및useSaveTempGameMutation에서 디버깅을 위한 콘솔 로그를 제거하였습니다.