-
Notifications
You must be signed in to change notification settings - Fork 8
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
[2주차] 오윤재 미션 제출합니다. #4
base: main
Are you sure you want to change the base?
Conversation
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.
안녕하세요 윤재님! 2주차 미션에서 윤재님의 코드 리뷰를 맡게 된 심여은입니다!
js를 리액트의 jsx로 적절하게 바꾸고, emotion의 css()를 잘 사용한 윤재님의 코드 열심히 읽어보았습니다!
React에서 어떤 기준으로 컴포넌트를 분리해야 하는지에 대해서는 저도 항상 고민이 많습니다🥲 다른 사람들의 좋은 코드를 많이 참고해서 스스로의 기준을 만들고, 협업하는 팀원들과 컴포넌트 분리에 대한 이야기도 해보는 게 좋을 것 같아요!
저희 같이 좋은 코드 짜면서 성장해봐요~~~😄😄
const toggleTodo = (id) => { | ||
const updatedTodos = todos.map((todo) => | ||
todo.id === id ? { ...todo, completed: !todo.completed } : todo | ||
); | ||
setTodos(updatedTodos); | ||
}; | ||
|
||
const deleteTodo = (id) => { | ||
const filteredTodos = todos.filter((todo) => todo.id !== id); | ||
setTodos(filteredTodos); | ||
}; |
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.
이벤트 핸들러에 할당한 함수 이름의 경우, 보통 접두사로 handle
을 붙여준다고 합니다!
https://blog.sonim1.com/220
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.
변수명을 조금 더 명확하게 명명하는 습관을 길러야겠어요...!
알려주셔서 감사합니다!😍
const addTodo = (text) => { | ||
const newTodo = { id: Date.now(), text, completed: false }; // id에 고유한 숫자를 부여하기 위해서 Date.now() 사용 | ||
setTodos([...todos, newTodo]); | ||
}; |
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.
고유한 id를 위해 Date.now()
로 값을 지정해준 부분 좋아요🥰
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.
Date.now()
로 고유한 id 지정을 할 수 있겠군요! 잘 알아갑니다 +-+
<div | ||
css={css({ | ||
display: 'flex', | ||
justifyContent: 'center', | ||
alignItems: 'center', | ||
width: '100vw', | ||
height: '100vh', | ||
margin: 0, | ||
background: 'linear-gradient(to right, #ffb3b3, #ffd9b3, #ffffb3)', | ||
fontFamily: 'Arial', | ||
})} |
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.
emotion의 css() 함수를 잘 사용하셨네요! 만약 css와 html 태그가 분리된 직관적인 코드를 원한다면 다음처럼 사용할 수 있어요!
const WrapperStyle = css`
display: flex;
justify-content: center;
align-items: center;
width: 100vw;
height: 100vh;
margin: 0;
background: linear-gradient(to right, #ffb3b3, #ffd9b3, #ffffb3);
font-family: Arial;
`;
https://www.howdy-mj.me/css/emotionjs-intro
다른 방법으로는 Styled-Component의 styled()가 있습니다. emotion에서도 @emotion/styled 패키지로 쉽게 사용해볼 수 있어요!
const WrapperStyle = styled.div`
display: flex;
justify-content: center;
align-items: center;
width: 100vw;
height: 100vh;
margin: 0;
background: linear-gradient(to right, #ffb3b3, #ffd9b3, #ffffb3);
font-family: Arial;
`;
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.
css()함수만 사용해서 구현했는데 styled()함수도 편해보이네요 @! 다음 미션에서 써봐야겠어요
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.
여은 님 말씀처럼 css()
를 이용한 코드를 변수에 할당하여 사용하면 훨씬 가독성이 높아질 것 같습니다!
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.
이번에 emotion을 쓰면서 가장 마음에 걸렸던 부분이에요...!
어떻게 해야 더 깔끔하게 쓸 수 있을까 고민했는데 이번 코드리뷰를 통해서 보다 가독성 높게 작성하는 법을 배우는 것 같습니다!
해당 부분 적용해서 리팩토링 하도록 하겠습니다. 모두 감사합니다!😻
if (text.trim()) { | ||
//공백 제거했을 때, 길이가 1이상일 경우에만 실행 | ||
addTodo(text); | ||
setText(''); //초기화 | ||
} |
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.
사용자의 공백 문자를 방지한 부분 좋아요!👍🏻 사용자가 잘못 입력했을 경우, alert()
등으로 입력 오류를 알려주는 코드를 추가하는 것도 좋을 것 같습니다!
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.
저는 이번에 모달창으로 공백 방지창을 추가해봤습니다! alert()
보다 더 신경쓸 게 많지만 UI적으로 더 이쁜 거 같아서 추천해봅니다❗️
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.
사용자에게 입력 제한을 명시해주는 게 좋을 것같네요!
해당 부분 반영해서 리팩토링 진행하도록 하겠습니다. 감사합니다!
<ul | ||
css={css({ | ||
marginBottom: '20px', | ||
})} | ||
> |
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.
TodoItem들을 감싸고 있는 ul 태그 스타일에 다음과 같은 코드를 추가한다면, Item들이 많아졌을 때 스크롤로 구현할 수 있어요!
height: 168px; //원하는 높이로 설정하기
overflow-y: scroll;
주어진 공간보다 더 많은 양의 컨텐츠를 보여줄 수 있는 overflow
속성이 궁금하다면, 다음 블로그를 참고해보세요!
https://www.daleseo.com/css-overflow/
textDecoration: todo.completed ? 'line-through' : 'none', | ||
color: todo.completed ? 'gray' : 'rgb(61, 61, 61)', | ||
cursor: 'pointer', |
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.
조건부 스타일링으로 상황에 따라 적절한 스타일을 보여주는 코드 아주 좋아요🥰🥰
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.
윤재님의 디테일을 볼 수 있는 코드 리뷰였습니다!
사소한 것도 신경 많이 쓰신 게 티가 나는 프로젝트였어요+-+
앞으로 함께 잘 성장해 보겠습니다 🥰
import TodoList from './components/TodoList'; | ||
|
||
// 아래 @jsxImportSource 이걸 안해주면 css props를 못 받아온다. | ||
/** @jsxImportSource @emotion/react */ |
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.
JSX Pragma에 대해 아시나요 ? 한번 참고해보시면 좋을 거 같아요! 저도 주석처리를 왜 해줘야하는지 의문이 들어서 찾아봤습니다 👍🏻
emotion의 css prop을 제대로 사용하기위해서는 파일 상단에 pragma 선언해 주어야합니다. pragma란 컴파일러에게하는 전처리 명령이라 생각하면 되는데,
/** @jsx jsx */
를 통해 컴파일 전처리 단계에서 jsx 문법들을 React.createElement 대신 jsx로 해석해야 css prop이 정상적으로 작동하기 때문입니다.
하지만, 프로젝트에서는 /** @jsx jsx */
를 선언해주어도 css props을 제대로 사용할 수 없었는데, 그 이유는 @babel/preset-react의 runtime: “automatic” 옵션을 켜주었기 때문입니다. (CRA v4 이후와 같은 환경)
import React from "react"
위와 같은 문법이 필요없도록하는 리액트: 새로운 jsx 변환을 이용했을 시,
/** @jsxImportSource @emotion/react */
를 선언해주어야 비로소 jsx 파일이 css prop을 읽을 수 있게 됩니다.
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.
오오! 궁금했던 부분을 명확하게 설명해주셔서 감샇바니다!
단순 주석이라 착각하여 초반에 기입하지 않았더니 emotion 적용이 안되어서 꽤나 힘들었는데...
덕분에 많이 배웠습니다! 😍
const addTodo = (text) => { | ||
const newTodo = { id: Date.now(), text, completed: false }; // id에 고유한 숫자를 부여하기 위해서 Date.now() 사용 | ||
setTodos([...todos, newTodo]); | ||
}; |
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.
Date.now()
로 고유한 id 지정을 할 수 있겠군요! 잘 알아갑니다 +-+
<div | ||
css={css({ | ||
display: 'flex', | ||
justifyContent: 'center', | ||
alignItems: 'center', | ||
width: '100vw', | ||
height: '100vh', | ||
margin: 0, | ||
background: 'linear-gradient(to right, #ffb3b3, #ffd9b3, #ffffb3)', | ||
fontFamily: 'Arial', | ||
})} |
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.
css()함수만 사용해서 구현했는데 styled()함수도 편해보이네요 @! 다음 미션에서 써봐야겠어요
height: '7.5%', | ||
paddingLeft: '16px', | ||
display: 'flex', | ||
boxAlign: 'center', |
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.
현재 box-align은 표준 트랙에 없다고 해요. 일부 브라우저에서는 지원이 안될 가능성이 있습니다! 현재 표준인 flexbox에 대해 참고삼아 보시면 좋을 듯 합니다
https://developer.mozilla.org/en-US/docs/Web/CSS/box-align
if (text.trim()) { | ||
//공백 제거했을 때, 길이가 1이상일 경우에만 실행 | ||
addTodo(text); | ||
setText(''); //초기화 | ||
} |
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.
저는 이번에 모달창으로 공백 방지창을 추가해봤습니다! alert()
보다 더 신경쓸 게 많지만 UI적으로 더 이쁜 거 같아서 추천해봅니다❗️
justifyContent: 'flex', | ||
alignItems: 'center', | ||
padding: '10px', | ||
gap: '10px', // 요소들 사이의 간격을 설정 |
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.
간격까지 설정해주는 디테일 좋습니다 !
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.
안녕하세요 윤재 님!
저번주부터 느끼는 것이지만 코드를 직관적이고 깔끔하게 잘 작성하시는 것 같아요!! 다른 분들이 이미 좋은 코멘트를 많이 달아 주셔서 리뷰해 드릴 내용이 별로 없었네요 😀
이번주 미션도 고생하셨습니다 👍🏻
|
||
const addTodo = (text) => { | ||
const newTodo = { id: Date.now(), text, completed: false }; // id에 고유한 숫자를 부여하기 위해서 Date.now() 사용 | ||
setTodos([...todos, newTodo]); |
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.
spread
연산자 사용 아주 좋습니다! 다만 setState
를 사용할 때 변경되는 state를 직접 참조하여 사용하는 것보다는 콜백 함수를 작성하는 방식이 조금 더 안정적인 방식입니다 🙂
setTodos([...todos, newTodo]); | |
setTodos((prev) => [...prev, newTodo]); |
그 이유가 궁금하다면...
프로젝트를 진행하다 보면 state 변경 -> 렌더링 -> state 변경 -> 렌더링 -> ...
과정을 반복하게 됩니다.
setState(state + 1)
은 현재의 state 값을 기반으로 상태를 업데이트 합니다. 하지만 이 방식은 state 값이 변경되는 과정에서 다른 setState 호출이 있을 경우, 이전 렌더링 시점의 상태를 기반으로 하기 때문에 최신 상태값을 반영하지 못할 수 있습니다.
반면 setState(prev => prev + 1)
은 콜백 함수를 사용하여 이전 상태(prev)를 기준으로 상태를 업데이트 합니다. 이 방법은 상태 업데이트가 비동기적으로 처리되는 동안에도 항상 최신 상태를 반영하므로, 상태가 여러 번 업데이트 될 때에도 정확한 결과를 보장한다고 합니다!
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.
코드 작동 과정에 대한 자세한 설명 너무 감사합니다!😍
비동기적 방식 및 콜백함수를 통해 정확한 결과를 보장하는 코드 작성이 체화되도록 많이 연습해야겠어요 😭
감사합니다👍
const toggleTodo = (id) => { | ||
const updatedTodos = todos.map((todo) => | ||
todo.id === id ? { ...todo, completed: !todo.completed } : todo | ||
); | ||
setTodos(updatedTodos); | ||
}; |
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.
👍🏻👍🏻
<div | ||
css={css({ | ||
display: 'flex', | ||
justifyContent: 'center', | ||
alignItems: 'center', | ||
width: '100vw', | ||
height: '100vh', | ||
margin: 0, | ||
background: 'linear-gradient(to right, #ffb3b3, #ffd9b3, #ffffb3)', | ||
fontFamily: 'Arial', | ||
})} |
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.
여은 님 말씀처럼 css()
를 이용한 코드를 변수에 할당하여 사용하면 훨씬 가독성이 높아질 것 같습니다!
안녕하세요, 여러분! LG U+ URECA 프론트엔드 대면반 1기 오윤재입니다.
지난 1주차에 이어서 이번 2주차도 제겐 결코 쉽지 않았던 미션이었습니다.😂
바닐라 자바스크립트 미션때에는 뭔가 정말 순수한 DOM조작 같다면,
이번 React 미션에는 컴포넌트를 나눠서 화면을 구성하는 부분이 가장 공부를 많이 하게된 부분이었습니다.📚
💡 알게 된 부분
📖 생각해 볼 질문들
1. 지난주 미션과 비교했을 때, React로 구현하는 과정에서 어떤 점이 더 편리하거나 복잡했나요?
/** @jsxImportSource @emotion/react */
이 부분이 있어야 css의 props를 받아올 수 있는데, 저는 주석인줄 알고 기입을 안했어서 꽤나 긴 시간동안 방황했습니다😭2. React의 Virtual DOM(가상 돔)이란 무엇이고, 이를 사용했을 때의 장점은 무엇이 있나요?
3. React에서 state와 props는 무엇이고 어떻게 사용하나요?
4. React에서 컴포넌트를 분리하는 기준은 무엇일까요?
이 부분이 이번주 미션을 하면서 가장 어렵게 느껴졌던 것 같습니다.
지난주에 사용했던 Semantic tag 형식(UI기준)으로 컴포넌트를 나누자니,
할일과 완료한 일들의 state와 props를 어떻게 주고받아야할 지에 대해 길게 고민했습니다.
그래서 이번 미션에서는 기능 단위로 컴포넌트를 나눠서 진행하니 컴포넌트 분리 기준을 직접 익히며 배웠습니다.
🌟 다음주 각오
🌹 배포