Conversation
There was a problem hiding this comment.
안녕하세요 태욱님! 과제하느라 정말 고생하셨습니다…
코드 보면서 많이 배울 수 있어서 좋았는데요. 특히 날짜 형식 변환하는 함수나 게임 관련 로직을 따로 파일로 분리하시니까 되게 코드가 깔끔해진 것 같아요. 또한, 글로벌과 테마를 설정해주신 모습을 보고 공통 스타일 관리를 되게 잘하신다고 느꼈습니다👍
아쉬운 점을 얘기하자면요… 먼저, 게임이 정상적으로 작동되고 있지 않습니다! 다음 숫자가 아닌 숫자 버튼을 클릭해도 버튼이 사라지고 있어요. 코드 리뷰 반영하면서 로직 수정해주시면 좋을 것 같습니다!
또한, 타이머를 Home.jsx에서 관리하고 있으신데 게임이 시작되면 타이머가 계속 업데이트되면서 자식 컴포넌트인 헤더와 게임 컴포넌트가 계속 리렌더링 되고 있습니다! 불필요한 리렌더링을 최대한 줄일 수 있는 방법을 한번 생각해보셔서 수정하는 것도 괜찮을 것 같습니다.
과제 너무 수고하셨습니다!!!
| //orderBy함수를 사용하여 정렬 | ||
| const sortedRank = orderBy(savedRank, ['level', 'time'], ['desc', 'asc']); |
There was a problem hiding this comment.
!!!!!! 처음 보는 함수인데 여러 기준으로 정렬하기가 되게 쉬워지네요..
덕분에 좋은 함수 알아갑니다👍
There was a problem hiding this comment.
넵 저도 정렬 관련해서 찾아보다가 이번에 알게 됐는데 간단한 코드로 정렬이 가능해서 자주 사용할 것 같아요!
| import formatDate from '../algorithm/formatDate'; | ||
| import { Theme } from '../styles/theme'; | ||
|
|
||
| const Rank =()=>{ |
There was a problem hiding this comment.
정말 안 중요한 질문이긴 한데 왜 이 파일만 탭 너비가 다른 건가요?!
There was a problem hiding this comment.
원래 vscode 코드 포매터를 한 번씩 돌리는데 적용이 안 된 부분인 것 같아요 😅 특별한 이유는 없었습니닷!!
| const gameData = gameAlgorithm(level, timer, handleTimerChange); | ||
| const rowNum = gameData.rowNum; | ||
| const maxNum = gameData.maxNum; | ||
| const beforeNums = gameData.beforeNums; | ||
| const currentNum = gameData.currentNum; | ||
| const isFinish = gameData.isFinish; | ||
| const checkNumberClick = gameData.checkNumberClick; | ||
| const closeModal = gameData.closeModal; |
There was a problem hiding this comment.
구조 분해 할당을 사용하면 더 간편하게 값들을 가져올 수 있을 것 같아요!
const { rowNum, maxNum, beforeNums, currentNum, isFinish, checkNumberClick, closeModal } = gameData;
이런 식으로 한 줄로 선언하면 가독성 측면에서도 더 좋지 않을까 제안드립니다!
There was a problem hiding this comment.
좋은 방법인 것 같아요 적용하도록 하겠습니다 감사합니다~~
|
|
||
| return ( | ||
| <> | ||
| <ThemeProvider theme={Theme}> |
There was a problem hiding this comment.
세미나 내용에 있었어서 한 번 적용해보고 싶었습니다 ㅎㅎ 감사합니다!
| const closeModal = () => { | ||
| setBeforeNums(createNums(1, boardSize)); | ||
| setAfterNums(createNums(boardSize + 1, maxNum)); | ||
| setIsFinish(false); | ||
| setCurrentNum(1); | ||
| handleTimeChange(0); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| setBeforeNums(createNums(1, boardSize)); | ||
| setAfterNums(createNums(boardSize + 1, maxNum)); | ||
| setIsFinish(false); | ||
| clearInterval(timeId); | ||
| setCurrentNum(1); | ||
| handleTimeChange(0); | ||
| return () => clearInterval(timeId); | ||
| }, [level]); //level이 변경될 때 마다 렌더링이 되도록 |
There was a problem hiding this comment.
closeModal과 useEffect에서 동일한 함수들이 호출되고 있는 것 같은데 중복되는 함수들은 하나의 함수로 묶어서 호출하는 건 어떨까요?
There was a problem hiding this comment.
동의합니다!! 말씀해주신대로 수정해볼게요
| border: none; | ||
| outline: none; |
There was a problem hiding this comment.
이런 스타일은 global.js에서 설정하는 것도 괜찮을 듯 합니다!
파일 보니 border: none은 이미 설정되어 있네요.
| <Head> | ||
| <LeftHead> | ||
| <h1>1 to 50</h1> | ||
| <div> |
There was a problem hiding this comment.
여기 div로 감싸준 이유가 있나요? 적용된 css는 없는 듯 합니다.
There was a problem hiding this comment.
처음 작성할 때 잘못 작성했던 코드인 것 같아요 꼼꼼한 리뷰 감사합니다!!
| font-size: 1.1rem; | ||
| padding: 0.3rem 0.8rem; | ||
| background-color: lightgray; | ||
| cursor: pointer; |
There was a problem hiding this comment.
cursor: pointer; 이 속성 이미 global에 설정되어 있습니다!
There was a problem hiding this comment.
제가 global css를 제대로 사용해 본 적이 없어서 요런 실수를 여러번 반복했네요 반영하겠습니다 감사합니닷~~
| const portalElement = document.getElementById("modal"); | ||
|
|
||
| const Modal = ({ timer, closeModal }) => { | ||
| return createPortal( | ||
| <> | ||
| <Backdrop onClick={closeModal} /> | ||
| <Container onClick={(e) => e.stopPropagation()}> | ||
| <Text>걸린시간 : {timer}</Text> | ||
| <Button onClick={closeModal}> | ||
| 확인 | ||
| </Button> | ||
| </Container> | ||
| </>, | ||
| portalElement | ||
| ); | ||
| }; |
There was a problem hiding this comment.
제가 createPortal을 처음 사용해봐서 다른 분들은 어떻게 사용했을지 궁금했는데 이렇게 한 파일에 작성하셨군요!!!
저는 포탈 생성 파일과 모달을 분리했었는데, 이 방식처럼 통합하는 쪽으로 바꿔야겠다는 생각이 드네여...
There was a problem hiding this comment.
아하 통합해서 사용하시는 것도 고려해보시면 좋을 것 같아요!!
gonn-i
left a comment
There was a problem hiding this comment.
로직에 많은 고민을 한게 느껴지는 코드들이었습니다! 세부 로직 분리에 신경을 많이 쓰신 것 같고 잘 분리된 것 같습니다! theme도 처음 사용해보셨다고 했는데 너무 잘 구성하신 것 같아 저도 많이 배워갑니다!
추가로, utils 와 custom hook으로 나누어서 구성하는 방법을 제안하고 싶습니다! 특히, GameAlgorithm은 -> 커스텀훅으로, formatDate 는 -> 유틸에 구분하는 것이 좋을 것 같습니다 :) 그리고 카드 숫자관련 상태 관리에 대해서 질문해주신 것에 대해서, 저는 하나의 상태관리에서 하나의 배열 안에 이중 객체를 둔 형태로 카드의 숫자와 클릭 여부를 관리해주었습니다!
이틀동안 멋지게 과제 완수하느라 너무 고생 많으셨습니다! 🔥 다음 주차도 아자아자
| const period = hour >= 12 ? '오후' : '오전'; | ||
| hour = hour % 12 || 12; |
There was a problem hiding this comment.
p5: 오호 오전 오후까지 고려하셨다니 세심한 생각 한수 배웠습니다 !!
| if (currentNum === 1) { | ||
| const id = setInterval(() => { | ||
| handleTimeChange((previousTime) => { | ||
| const updatedTime = (previousTime + 0.01).toFixed(2); | ||
| return parseFloat(updatedTime); | ||
| }); | ||
| }, 10); | ||
| setTimeId(id); | ||
| } |
There was a problem hiding this comment.
p1: 보니까 currentNum은 다음에 눌러야할 카드를 보여주는 상태인 것 같네요!
근데 처음 게임을 시작할때를 생각해보면, 클릭한 카드의 번호 == 1이어야 할거 같아요.
currentNum 대신에 내가 누른 카드 번호로 바꿔주시면 좋을 것 같습니다
There was a problem hiding this comment.
넵 말씀해주신대로 클릭한 number가 1일때 타이머가 시작되는게 맞는 것 같아요!! 반영하도록 하겠습니다
| if (number === maxNum) { | ||
| clearInterval(timeId); | ||
| saveResult(timer, level); | ||
| setIsFinish(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
p1: maxNum을 누르면 게임이 끝나고 있어요! 이렇게 되면 남은 숫자가 있는데도 게임이 종료되어서
요기 조건문을 수정해주시면 좋을 것 같습니다!
if(number === maxNum && number === currentNum) 이렇게 조건 바꿔주시면 해결될 것 같습니다 :)
There was a problem hiding this comment.
헉 정말 고려하지 못했던 부분이네요 좋은 리뷰 감사합니다!! 반영하도록 하겠습니다
| setBeforeNums(updatedBoard); | ||
| } | ||
|
|
||
| setCurrentNum(currentNum + 1); |
There was a problem hiding this comment.
p1: 카드를 클릭할때마다 currentNum가 증가하고 있는데요! 요거는 알맞은 카드를 선택할때만 작동하도록 조건문을 걸어주시면 될 것 같습니다
There was a problem hiding this comment.
넵 이 부분을 고려해야 알맞은 카드만 선택이 가능할 것 같아요 좋은 리뷰 감사합니다!
| const closeModal = () => { | ||
| setBeforeNums(createNums(1, boardSize)); | ||
| setAfterNums(createNums(boardSize + 1, maxNum)); | ||
| setIsFinish(false); | ||
| setCurrentNum(1); | ||
| handleTimeChange(0); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| setBeforeNums(createNums(1, boardSize)); | ||
| setAfterNums(createNums(boardSize + 1, maxNum)); | ||
| setIsFinish(false); | ||
| clearInterval(timeId); | ||
| setCurrentNum(1); | ||
| handleTimeChange(0); | ||
| return () => clearInterval(timeId); | ||
| }, [level]); //level이 변경될 때 마다 렌더링이 되도록 |
There was a problem hiding this comment.
p3:
모달 닫기 함수와 useEffect 에서 아래의 로직이 반복되어 동작하는데요! 요친구 함수로 만들어서 사용하면 더 좋을 것 같네요!
setBeforeNums(createNums(1, boardSize));
setAfterNums(createNums(boardSize + 1, maxNum));
setIsFinish(false);
setCurrentNum(1);
handleTimeChange(0);There was a problem hiding this comment.
넵 중복되는 부분 함수로 만들어서 적용해보겠습니다!
| const handleMenuChange = (menu) => { | ||
| setMenu(menu); | ||
| } | ||
| const handleLevelSelect = (level) => { | ||
| setLevel(level); | ||
| }; | ||
| const handleTimerChange = (timer) => { | ||
| setTimer(timer); | ||
| }; |
There was a problem hiding this comment.
오호 몰랐는데 자체를 넘기지 않는게 좋갰네요! 앗싸 새로운거 배워갑니다!
| const [beforeNums, setBeforeNums] = useState(createNums(1, boardSize)); | ||
| const [afterNums, setAfterNums] = useState(createNums(boardSize + 1, maxNum)); |
There was a problem hiding this comment.
p2: 상태관리 변수이름이 추상적인 것 같습니다! 조금 더 직관적인 이름으로 바꾸거나 각주를 추가하는 방법 추천 드립니다!
There was a problem hiding this comment.
넵 직관적인 이름이나 각주 달아보도록 하겠습니다!!
| const closeModal = () => { | ||
| setBeforeNums(createNums(1, boardSize)); | ||
| setAfterNums(createNums(boardSize + 1, maxNum)); | ||
| setIsFinish(false); | ||
| setCurrentNum(1); | ||
| handleTimeChange(0); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| setBeforeNums(createNums(1, boardSize)); | ||
| setAfterNums(createNums(boardSize + 1, maxNum)); | ||
| setIsFinish(false); | ||
| clearInterval(timeId); | ||
| setCurrentNum(1); | ||
| handleTimeChange(0); | ||
| return () => clearInterval(timeId); | ||
| }, [level]); //level이 변경될 때 마다 렌더링이 되도록 |
| @@ -0,0 +1,22 @@ | |||
| const formatDate = (timestamp) => { | |||
✨ 구현 기능 명세
💡 기본 과제
18까지 클릭해야한다면, 처음에는 19까지의 숫자가 랜덤으로 보여짐현재 시각,게임의 레벨,플레이 시간3개의 정보를 localStorage에 저장 (랭킹에서 사용)🔥 심화 과제
Level 1:
3 x 3, Level 2:4 x 4, Level 3:5 x 5createPortal
공유과제
제목: [React] useEffect 훅과 의존성 배열
https://wave-web.tistory.com/76
❗️ 내가 새로 알게 된 점
❓ 구현 과정에서의 어려웠던/고민했던 부분
숫자 클릭할 때 클릭되는 것 같은 효과와 과제 동영상처럼 색깔을 바꾸기 위해서 클릭된 번호를 저장할 빈 배열을 만들어서, 클릭되면 번호를 저장하고 클래스명을 추가해 해당 클래스명에만 따로 CSS를 입히는 방식을 사용했는데 적용이 되지 않아서 심화 과제 중 한 문제를 해결하지 못했습니다.
=>배열의 상태 업데이트의 비동기성 때문에 클릭했을 때 효과가 제대로 적용되지 않았던 것이 문제였습니다.
따라서 아예 gameAlgorithm으로 부터 maxNum을 가져와서
className={number > maxNum/2 && number <= maxNum ? 'clicked' : ''} 를 통해 클릭 이후 나오는 게임 판들은 clicked 클래스명을 추가하는 방식으로 문제를 해결할 수 있었습니다.
클릭하면서 숫자가 바뀌는 것을 처음에 사용할 숫자를 담은 배열(beforeNums)과 다음에 나올 숫자를 담은 배열(afterNums), 이렇게 두가지 배열을 활용하면 될 것이다를 생각하는데 적지 않은 고민을 했던 것 같습니다. 다른 방법으로 구현이 가능한지도 궁금합니다!
🥲 소요 시간
🖼️ 구현 결과
https://youtu.be/N4vclnXXYz0