-
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주차] 김윤일 미션 제출합니다. #6
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.
윤일님! 한 주동안 고생하셨습니다~!
윤일님 코드 보면서 emotion을 어떻게 써야 조금 더 효율적으로,
그리고 깔끔한 코드를 작성할 수 있을 지 알게 된 것같아요!
또한 각 디자인 및 기능별 컴포넌트마다 주석으로 설명해주셔서 보다 빠르게 코드를 이해할 수 있었습니다!
다시 한 번 한 주동안 수고하셨습니다!
|
||
{popup ? <Popup popup={popup} setPopup={setPopup} /> : null} | ||
</div> |
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.
해당 문구를 클릭하면 문구가 없어지도록 이벤트를 적용해 뒀습니다. 그럼 새로운 문자열을 입력하더라도 문구를 클릭하지 않으면 계속 있겠군요! 거기까진 미처 생각하지 못했네요. 감사합니다! 모달창 형식을 구현해 보고 싶어서 도입했는데 부족한 점이 많네요 ! 바로 수정해 보도록 하겠습니다👍🏻
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 appStyle = css` | ||
background: radial-gradient(#e3fafc, #99e9f2); | ||
width: 100vw; | ||
height: 100vh; | ||
display: flex; | ||
align-items: center; | ||
justify-content: 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.
emotion으로 css를 컴포넌트화 시켜서 꾸며주셨네요!
저는 아직 css를 다루는게 익숙하지 않아서 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.
감사합니다! 이번에 다른 분 코드리뷰하면서 느낀 점인데요! css()
를 사용하면 const를 사용해서 계속 선언해야 하는 게 가독성이 좋아 보이지 않더라구요! 그래서 다른 분(주희님)거 참고해 보니Style()
을 사용하면 Style함수 안에 태그 명을 기재해서 여러 개를 작성해서 사용할 수 있습니다! 참고해 보시면 좋을 거 같아요👍🏻
export const Style = {
Wrapper: styled.div`
width: 100%;
height: 260px;
overflow-y: scroll;
margin-top: 10px;
`,
ItemWrapper: styled.div`
display: flex;
align-items: center;
width: 90%;
height: 40px;
`,
};
return ( | ||
<div className="todo-input"> | ||
<div className="app-title" css={appTitle}> |
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는 emotion으로 처리해주셨는데, 각 div에 className을 유지 및 설정하신 이유가 따로 있을까요?
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.
맨 처음엔 각각의 태그를 구분하기 위해 className부터 적었습니다! 그 이후에 스타일을 적용하면서 className의 이름을 그대로 적용하니 네이밍하기 편하더라구요
추가로 className을 사용하지 않는다면 가독성을 위해 삭제하는 게 좋은 건지 모르겠기에 놔뒀습니다! 윤재님은 어떻게 생각하시나요 ?
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을 적용하면서 className을 지웠는데,
생각해보니까 네이밍이나 가독성을 위해 두는 것이 더 좋을 것 같네요!
답변해주셔서 감사합니당😊
const onClickDeleteButton = () => { | ||
if (window.confirm('할 일을 삭제하시겠습니까?')) { | ||
const nextTodoList = todoList.map((item) => ({ | ||
...item, | ||
deleted: item.id === todoItem.id ? true : item.deleted, | ||
})); | ||
setTodoList(nextTodoList); | ||
} | ||
}; |
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.
할일 삭제할 때 한 번 더 확인해주는 센스~!
너무 좋은 것 같아요! 🌟
return ( | ||
<section className="todo-item-list" css={todoItemWrapper}> | ||
<div> |
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.
저번 미션에 이어서 Semantic tag 형식을 유지하셔서 해당 항목이 어디에 return되고 화면에 표시될지 알 수 있어서 좋은 것같아요!
const baseStyle = css` | ||
${emotionNormalize} | ||
@font-face { | ||
font-family: 'EF_jejudoldam'; | ||
src: url('https://fastly.jsdelivr.net/gh/projectnoonnu/[email protected]/EF_jejudoldam.woff2') | ||
format('opentype'); | ||
font-weight: normal; | ||
font-style: normal; | ||
} |
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.
오 font도 이렇게 컴포넌트로 만들어서 꾸며줄 수가 있군요!
많이 배워갑니다!😍
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.
@font-face
는 컴포넌트가 아니고 CSS에서 제공하는 규칙입니다! 이 규칙을 사용하면 웹페이지에서 특정 폰트를 불러와서 사용할 수 있습니다. 폰트 사이트로 유명한 눈누 보시면 웹 폰트를 제공해주고 있습니다! 그래서 전역으로 복붙만 하면 바로 사용할 수 있는 것이죠! 공식문서 한번 참고삼아보시면 좋을 거 같아요!
import App from './App.jsx' | ||
import { StrictMode } from 'react'; | ||
import { createRoot } from 'react-dom/client'; | ||
import { Global, css } from '@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.
Global style은 처음 봐서 신기하네요!
글로벌 스타일을 사용하면 모든 컴포넌트에 공통적으로 적용되어야 하는 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에서는 초기화를 어떻게 해야할 지 몰라서 찾아보다가 알게되었습니다. 앞으로 자주 사용하게 될 거 같아요 🥰
useEffect(() => { | ||
// todoList가 변했을 때만 실행 | ||
console.log(todoList); | ||
}, [todoList]); |
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.
useEffect를 사용해서 재랜더링 조건을 부여해주신 부분이 좋은 것같습니다!
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.
컨셉 확실한 디자인이 귀여워요! 이번 윤일님의 과제에서는 자잘한 디테일 처리가 돋보였던 것 같습니다 👍
현재 배포된 기능 상 "공백은 입력할 수 없어요" 라는 경고문구가 떠있는 상황에서 한 일과 할 일이 각각 5개 이상일 때에는 기존 틀 밖으로 글자가 튀어나오는 현상이 있습니다! 이 부분도 참고하여 리팩토링 해주시면 좋을 것 같아요 🤩 고생하셨습니다!
const TodoInput = ({ todoList, setTodoList }) => { | ||
const [text, setText] = useState(''); | ||
const inputRef = useRef(null); // 버튼 클릭 후 input 값 초기화 후 focusing | ||
const [popup, setPopup] = useState(false); // 공백 입력 방지 팝업 |
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.
boolean 변수는 의문문 형식으로 작성해주시는 것이 좋습니다 🤩 ex. is + 상태
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 nextTodoList = todoList.concat({ | ||
id: todoList.length, | ||
text, | ||
checked: false, // 완료 유무 flag |
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.
이런 flag 용 boolean 변수의 네이밍도 의문문으로 수정하는 것을 추천드려요 🤩
color: lightgray; | ||
text-decoration: line-through; | ||
`; | ||
const todoItemDeleteButton = 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.
button에 cursor: pointer; 를 주면 더 좋을 것 같아요 🤩
<div | ||
className="error-message" | ||
onClick={() => { | ||
props.setPopup(!props.popup); |
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 처리를 조금 더 추천 드립니다 🤩
}; | ||
|
||
// 팝업창 디자인 | ||
function Popup(props) { |
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 하여 사용하는 것이 좋을 것 같습니다!
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.
주희 님 말씀처럼 이 부분은 별도의 컴포넌트로 분리하는 게 좋을 것 같습니다!
props.setPopup(!props.popup); | ||
}} | ||
> | ||
<p |
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.
위에서 style마다 변수를 작성해주신 것처럼 스타일 작성 방식을 한 가지로 통일하면 좋을 것 같습니다 🤩
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.
안녕하세요 윤일 님!
고심하여 코드를 작성하신 부분이 눈에 보이네요 ㅎㅎ 특히 popup
state를 두어 메시지를 출력하는 기능이 인상적이었습니다!
이번주 미션도 고생하셨습니다 👍🏻
// 할 일 입력 기능 | ||
const TodoInput = ({ todoList, setTodoList }) => { | ||
const [text, setText] = useState(''); | ||
const inputRef = useRef(null); // 버튼 클릭 후 input 값 초기화 후 focusing |
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.
오호 useRef
까지 사용하셨네요! 👍🏻
const nextTodoList = todoList.concat({ | ||
id: todoList.length, | ||
text, | ||
checked: false, // 완료 유무 flag | ||
deleted: false, // 삭제 flag | ||
}); | ||
setTodoList(nextTodoList); |
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.
concat
사용 아주 좋습니다!
개인적인 느낌이긴 하지만 concat
보다는 spread
연산자를 조금 더 많이 사용하는 것 같아요!
const nextTodo = {
id: todoList.length,
text,
checked: false, // 완료 유무 flag
deleted: false, // 삭제 flag
};
setTodoList(prev => [...prev, nextTodo]);
|
||
// inputRef.current가 null이 아닌지 확인한 후 focus 호출 | ||
if (inputRef.current) { | ||
inputRef.current.focus(); // input 창에 focusing |
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.
굿굿!
|
||
useEffect(() => { | ||
// todoList가 변했을 때만 실행 | ||
console.log(todoList); |
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.
console.log
는 개발할 때에만 필요하기 때문에 커밋할 때에는 지워 주세요!
const inputRef = useRef(null); // 버튼 클릭 후 input 값 초기화 후 focusing | ||
const [popup, setPopup] = useState(false); // 공백 입력 방지 팝업 | ||
// input 값 가져오기 | ||
const onChangeInput = (e) => { |
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
+ 대상
+ 동작
의 형태로 짓습니다!
const onChangeInput = (e) => { | |
const handleInputChange = (e) => { |
}; | ||
|
||
// 팝업창 디자인 | ||
function Popup(props) { |
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.
주희 님 말씀처럼 이 부분은 별도의 컴포넌트로 분리하는 게 좋을 것 같습니다!
} | ||
|
||
// props 값 검증 | ||
TodoInput.propTypes = { |
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.checked ? ( // 완료한 일인 경우에는 null을 반환하여 수정버튼이 보이지 않도록 함 | ||
<button type="button" className="todo-item-edit-button"> | ||
수정 | ||
</button> | ||
) : null} */} |
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.
불필요한 코드는 삭제해 주세요!
if (window.confirm('할 일을 삭제하시겠습니까?')) { | ||
const nextTodoList = todoList.map((item) => ({ | ||
...item, | ||
deleted: item.id === todoItem.id ? true : item.deleted, |
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.
삭제한 항목을 배열에서 제거하는 게 아니라 deleted
로 관리하는 이유가 있으신가요?! 😯
|
||
createRoot(document.getElementById('root')).render( | ||
<StrictMode> | ||
<Global styles={baseStyle} /> |
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.
👍🏻👍🏻
안녕하세요! 유레카 프론트엔드 1기 김윤일입니다.
리액트로 무언갈 만들어보는 게 처음이다 보니 저번 미션보다 더 많이 막히고 어려웠던 거 같아요! 그래서 블로그나 지선생님에 의존을 많이 한 거 같습니다 ㅠㅠ 코드를 보면서 읽고 이해하는 과정을 반복했고요. 특히나 한 파일에서 모든 기능을 넣어서 구현했었는데 기능별로 컴포넌트를 나누고 그 나눈 파일에서 스타일 적용하는 것이 제일 어려웠던 과정 중 하나 입니다! 이번 미션으로 인해 어렵고 와닿지 않던 리액트랑 조금이나마 친해진 거 같아서 좋습니다!
잘 모르는 제가 봐도 리액트가 훨씬 간결하고 구현하기 쉬운 라이브러리인 거 같아요! 저번 미션에서 구현하지 못한 로컬 스토리지도 리액트에서는 이렇게 쉽게 할 수 있다는 점에 놀랐습니다! 리액트는 상태관리가 제일 중요한 포인트 같아요. 앞으로 더 공부해서 리액트에 더 익숙해지겠습니다👍🏻
💡 알게 된 부분
prop-types
라이브러리를 사용하여 type을 검사합니다.🏷️ 개선하고 싶은 부분
📖 생각해 볼 질문들
1) 지난주 미션과 비교했을 때, React로 구현하는 과정에서 어떤 점이 더 편리하거나 복잡했나요?
script.js
파일에 모든 기능을 구현했는데 이번에는 기능마다 컴포넌트를 만들어서 컴포넌트를 조합하는 게 복잡했어요. 특히나 스타일 적용하는 것도 조합한 결과물을 생각하면서 적용하려고 하니 익숙하던 css도 복잡하게 느껴진 거 같아요! 새로운 emotion을 사용해서 그런 것 같기도 합니다.2) React의 Virtual DOM(가상 돔)이란 무엇이고, 이를 사용했을 때의 장점은 무엇이 있나요?
3) React에서 state와 props는 무엇이고 어떻게 사용하나요?
리액트 컴포넌트는 기본적으로 두 가지 데이터 타입을 다룹니다:
4) React에서 컴포넌트를 분리하는 기준은 무엇일까요?
🚀 배포
https://react-todo-seven-delta.vercel.app