Skip to content
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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,17 @@
"preview": "vite preview"
},
"dependencies": {
"@emotion/react": "^11.13.3",
"@emotion/styled": "^11.13.0",
"emotion-normalize": "^11.0.1",
"normalize.css": "^8.0.1",
"prop-types": "^15.8.1",
"react": "^18.3.1",
"react-dom": "^18.3.1"
"react-dom": "^18.3.1",
"react-icons": "^5.3.0"
},
"devDependencies": {
"@emotion/babel-plugin": "^11.12.0",
"@eslint/js": "^9.9.0",
"@types/react": "^18.3.3",
"@types/react-dom": "^18.3.0",
Expand All @@ -24,5 +31,6 @@
"eslint-plugin-react-refresh": "^0.4.9",
"globals": "^15.9.0",
"vite": "^5.4.1"
}
},
"packageManager": "[email protected]"
}
19 changes: 18 additions & 1 deletion src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
/** @jsxImportSource @emotion/react */
import { css } from '@emotion/react';
import TodoTemplete from './components/TodoTemplete';

const appStyle = css`
background: radial-gradient(#e3fafc, #99e9f2);
width: 100vw;
height: 100vh;
display: flex;
align-items: center;
justify-content: center;
`;
Comment on lines +5 to +12

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emotion으로 css를 컴포넌트화 시켜서 꾸며주셨네요!
저는 아직 css를 다루는게 익숙하지 않아서 css를 태그에 바로 기입해주었는 데,
윤일님처럼 컴포넌트화 시켜서 하니까 코드가 보다 깔끔해보이는 것같아요!🤩

Copy link
Member Author

@Yunil-dev Yunil-dev Sep 5, 2024

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;
  `,
};


function App() {
return <div>React Todo</div>;
return (
<div css={appStyle}>
<TodoTemplete />
</div>
);
}

export default App;
139 changes: 139 additions & 0 deletions src/components/TodoInput.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/** @jsxImportSource @emotion/react */
import React, { Component } from 'react';
import { useState, useRef, useEffect } from 'react';
import PropTypes from 'prop-types';
import { css } from '@emotion/react';
import { BsFillExclamationTriangleFill } from 'react-icons/bs';

// 할 일 입력 form
const inputContainer = css`
padding: 0 20px;
`;
// 할 일 입력 버튼
const todoInputButton = css`
border: none;
background: none;
cursor: pointer;
padding: 10px;
`;
// 할 일 입력 창
const inputBox = css`
width: 80%;
padding: 10px;
border: 1px solid lightgray;
border-radius: 15px;
`;
// Todo List 앱 제목
const appTitle = css`
padding: 20px 20px 0;
font-style: italic;
`;
// hr 태그 스타일
const line = css`
margin-bottom: 10px;
color: lightgray;
border-style: dashed;
border-width: 2px 0 0 0;
`;
// 할 일 입력 기능
const TodoInput = ({ todoList, setTodoList }) => {
const [text, setText] = useState('');
const inputRef = useRef(null); // 버튼 클릭 후 input 값 초기화 후 focusing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호 useRef까지 사용하셨네요! 👍🏻

const [popup, setPopup] = useState(false); // 공백 입력 방지 팝업
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boolean 변수는 의문문 형식으로 작성해주시는 것이 좋습니다 🤩 ex. is + 상태

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네이밍 컨벤션 ..!! 알고있지만 막상 네이밍할 땐 까먹네요 ㅠㅠ
좋은 지적 감사합니다 !!!😊

// input 값 가져오기
const onChangeInput = (e) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이벤트 핸들러의 이름은 주로 handle + 대상 + 동작 의 형태로 짓습니다!

Suggested change
const onChangeInput = (e) => {
const handleInputChange = (e) => {

setText(e.target.value);
};

const onClickAddButton = (e) => {
e.preventDefault();

if (text.length === 0) {
setPopup(true);
return; // 공백일 경우 이후 코드를 실행하지 않음
}
// TodoItemList에 값 추가
const nextTodoList = todoList.concat({
id: todoList.length,
text,
checked: false, // 완료 유무 flag
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 flag 용 boolean 변수의 네이밍도 의문문으로 수정하는 것을 추천드려요 🤩

deleted: false, // 삭제 flag
});
setTodoList(nextTodoList);
Comment on lines +56 to +62
Copy link
Member

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]);


setText(''); // input 값 초기화
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

popup이 한번 등장하고 사라지지 않는 문제가 있다 했는데,
이 코드 아래에 setPopup(false)를 추가하면 정상적으로 입력된 경우 팝업이 보이지 않게 될 것 같아요~


// inputRef.current가 null이 아닌지 확인한 후 focus 호출
if (inputRef.current) {
inputRef.current.focus(); // input 창에 focusing
Copy link
Member

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log는 개발할 때에만 필요하기 때문에 커밋할 때에는 지워 주세요!

}, [todoList]);
Comment on lines +72 to +75

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useEffect를 사용해서 재랜더링 조건을 부여해주신 부분이 좋은 것같습니다!


return (
<div className="todo-input">
<div className="app-title" css={appTitle}>
Comment on lines +77 to +79

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙋 궁금한 점이 있는데요...!
css는 emotion으로 처리해주셨는데, 각 div에 className을 유지 및 설정하신 이유가 따로 있을까요?

Copy link
Member Author

@Yunil-dev Yunil-dev Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맨 처음엔 각각의 태그를 구분하기 위해 className부터 적었습니다! 그 이후에 스타일을 적용하면서 className의 이름을 그대로 적용하니 네이밍하기 편하더라구요 ☺️☺️
추가로 className을 사용하지 않는다면 가독성을 위해 삭제하는 게 좋은 건지 모르겠기에 놔뒀습니다! 윤재님은 어떻게 생각하시나요 ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하! 저는 이번에 emotion을 적용하면서 className을 지웠는데,
생각해보니까 네이밍이나 가독성을 위해 두는 것이 더 좋을 것 같네요!
답변해주셔서 감사합니당😊

<h1>오느리 할 일 ! </h1>
<hr css={line} />
</div>
<form css={inputContainer}>
{/* 할 일 입력창 */}
<input
type="text"
name="todoItem"
value={text}
ref={inputRef}
onChange={onChangeInput}
placeholder="할 일을 입력해주세요."
css={inputBox}
/>
{/* 할 일 입력 버튼 */}
<button type="submit" onClick={onClickAddButton} css={todoInputButton}>
+
</button>
</form>

{popup ? <Popup popup={popup} setPopup={setPopup} /> : null}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null을 리턴하는 것보다는 && 연산자를 사용하는 게 더 좋을 것 같습니다!

Suggested change
{popup ? <Popup popup={popup} setPopup={setPopup} /> : null}
{popup && <Popup popup={popup} setPopup={setPopup} />}

참고자료 - 단축 평가 논리 계산법

</div>
Comment on lines +99 to +101

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

삼항연산자를 이용해서 빈 문자열 처리해주셨네요!
근데 해당 페이지 들어가서 실행해보았는데,
다시 새로운 문자열을 입력했을 때는 해당 문구(공백은 입력할 수 없어요 ㅠㅠ)삭제가 되지 않아서
혹시 alert형식으로 해보시는 건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 문구를 클릭하면 문구가 없어지도록 이벤트를 적용해 뒀습니다. 그럼 새로운 문자열을 입력하더라도 문구를 클릭하지 않으면 계속 있겠군요! 거기까진 미처 생각하지 못했네요. 감사합니다! 모달창 형식을 구현해 보고 싶어서 도입했는데 부족한 점이 많네요 ! 바로 수정해 보도록 하겠습니다👍🏻

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 클릭하면 없어지는 거였군요!
제가 놓쳤었네요 확인해주셔서 감사합니다!

);
};

// 팝업창 디자인
function Popup(props) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가로 기능 구현이 필요하다면 기존 파일 아래에 구현하는 것이 아닌, 하나의 기능만을 하는 파일을 따로 나누고 import 하여 사용하는 것이 좋을 것 같습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주희 님 말씀처럼 이 부분은 별도의 컴포넌트로 분리하는 게 좋을 것 같습니다!

return (
<div
className="error-message"
onClick={() => {
props.setPopup(!props.popup);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

경고 문구를 직접 클릭 시 사라지게 하는 것은 불필요한 동작같습니다! 윤재님 말씀처럼 alert 처리를 조금 더 추천 드립니다 🤩

}}
>
<p
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에서 style마다 변수를 작성해주신 것처럼 스타일 작성 방식을 한 가지로 통일하면 좋을 것 같습니다 🤩

css={css`
color: gray;
font-size: 13px;
padding: 0px;
margin: 10px 25px 0;
`}
>
<BsFillExclamationTriangleFill /> 공백은 입력할 수 없어요 ㅠㅠ
</p>
</div>
);
}

// props 값 검증
TodoInput.propTypes = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나중에 타입스크립트 잘하실 것 같아요 🙂

todoList: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
text: PropTypes.string.isRequired,
}).isRequired
),
setTodoList: PropTypes.func.isRequired,
};

export default TodoInput;
102 changes: 102 additions & 0 deletions src/components/TodoItem.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/** @jsxImportSource @emotion/react */
import { css } from '@emotion/react';
import React from 'react';
import PropTypes from 'prop-types';

const todoItemContent = css`
font-size: 16px;
margin: 10px;
`;
const doneTodoItemContent = css`
font-size: 16px;
margin: 10px;
color: lightgray;
text-decoration: line-through;
`;
const todoItemDeleteButton = css`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

button에 cursor: pointer; 를 주면 더 좋을 것 같아요 🤩

border: none;
background: transparent;
`;
const todoItemLi = css`
list-style-type: none;
`;

const TodoItem = ({ todoItem, todoList, setTodoList }) => {
const onChangeCheckbox = () => {
const nextTodoList = todoList.map((item) => ({
...item,

// id 값이 같은 항목의 checked 값을 Toggle 함
checked: item.id === todoItem.id ? !item.checked : item.checked,
}));
setTodoList(nextTodoList);
};

const onClickDeleteButton = () => {
if (window.confirm('할 일을 삭제하시겠습니까?')) {
const nextTodoList = todoList.map((item) => ({
...item,
deleted: item.id === todoItem.id ? true : item.deleted,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

삭제한 항목을 배열에서 제거하는 게 아니라 deleted로 관리하는 이유가 있으신가요?! 😯

}));
setTodoList(nextTodoList);
}
};
Comment on lines +35 to +43

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

할일 삭제할 때 한 번 더 확인해주는 센스~!
너무 좋은 것 같아요! 🌟

return (
<>
<li className="todo-item-li" css={todoItemLi}>
{/* 완료 표시를 위한 체크박스 */}
<input
className="todo-item-checkbox"
type="checkbox"
checked={todoItem.checked}
onChange={onChangeCheckbox}
/>

{/* todo item 내용 */}
{!todoItem.checked ? ( // 완료한 일인 경우에는 밑줄 스타일 적용
<span className="todo-item-content" css={todoItemContent}>
{todoItem.text}
</span>
) : (
<span className="done-todo-item-content" css={doneTodoItemContent}>
{todoItem.text}
</span>
)}

{/* 수정 버튼
{!todoItem.checked ? ( // 완료한 일인 경우에는 null을 반환하여 수정버튼이 보이지 않도록 함
<button type="button" className="todo-item-edit-button">
수정
</button>
) : null} */}
Comment on lines +67 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

불필요한 코드는 삭제해 주세요!


{/* 삭제 버튼 */}
<button
type="button"
className="todo-item-delete-button"
onClick={onClickDeleteButton}
css={todoItemDeleteButton}
>
🗑️
</button>
</li>
<br />
</>
);
};

TodoItem.propTypes = {
todoItem: PropTypes.shape({
id: PropTypes.number,
text: PropTypes.string.isRequired,
}),
todoList: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
text: PropTypes.string.isRequired,
})
),
setTodoList: PropTypes.func.isRequired,
};

export default TodoItem;
74 changes: 74 additions & 0 deletions src/components/TodoItemList.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/** @jsxImportSource @emotion/react */
import React from 'react';
import { css } from '@emotion/react';
import TodoItem from './TodoItem';
import PropTypes from 'prop-types';

const todoItemWrapper = css`
display: inline-block;
height: 40%;
padding: 0 20px 0;
`;
const todoItemList = css`
margin-top: 40px;
padding-left: 20px;
height: 60%;
overflow: auto;
`;
const todoItemTitle = css`
margin: 10px;
`;
const line = css`
margin-bottom: 10px;
color: lightgray;
border-style: dashed;
border-width: 2px 0 0 0;
`;

const TodoItemList = ({ title, todoList, setTodoList, checkedList }) => {
return (
<section className="todo-item-list" css={todoItemWrapper}>
<div>
Comment on lines +29 to +31

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저번 미션에 이어서 Semantic tag 형식을 유지하셔서 해당 항목이 어디에 return되고 화면에 표시될지 알 수 있어서 좋은 것같아요!

<hr css={line} />
<span className="todo-item-title" css={todoItemTitle}>
{title}
</span>
<hr css={line} />
</div>
<ul css={todoItemList}>
{todoList && // todoList가 있을 때만 출력
todoList.map((todoItem) => {
// map을 이용하여 TodoItem을 출력
// checkedList 값에 따라 '할 일 목록' 또는 '완료한 목록'을 출력
if (checkedList != todoItem.checked) return null;

// 삭제한 항목인 경우, 출력하지 않음 (deleted가 true)
if (todoItem.deleted) return null;

return (
<TodoItem
key={todoItem.id}
todoItem={todoItem}
todoList={todoList}
setTodoList={setTodoList}
/>
);
})}
</ul>
</section>
);
};

TodoItemList.propTypes = {
title: PropTypes.string.isRequired,
todoList: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
text: PropTypes.string.isRequired,
})
),
setTodoList: PropTypes.func.isRequired,
checkedList: PropTypes.bool.isRequired,
};

export default TodoItemList;
Loading