-
Notifications
You must be signed in to change notification settings - Fork 10
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: master
Are you sure you want to change the base?
Changes from 15 commits
62c533a
9e67710
509eca8
4231ce2
a96cc82
6cf7376
684325b
d492af5
92adef1
d0b3405
0b12d61
0caf6c8
51e3115
71b0f0a
feff246
d062a71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
import styled from "styled-components"; | ||
import { createGlobalStyle } from "styled-components"; | ||
import Header from "./components/Header"; | ||
import List from "./components/List"; | ||
import Editor from "./components/Editor"; | ||
import { useRef, useReducer } from "react"; | ||
|
||
const mockDate = [ | ||
{ | ||
id: 0, | ||
isDone: false, | ||
content: "React 공부하기", | ||
date: new Date().getTime(), | ||
}, | ||
{ | ||
id: 1, | ||
isDone: false, | ||
content: "고양이 밥 주기", | ||
date: new Date().getTime(), | ||
}, | ||
{ | ||
id: 2, | ||
isDone: false, | ||
content: "고양이 놀아주기", | ||
date: new Date().getTime(), | ||
}, | ||
]; | ||
|
||
function reducer(state, action) { | ||
switch (action.type) { | ||
case "CREATE": | ||
return [action.data, ...state]; | ||
case "UPDATE": | ||
return state.map((item) => | ||
item.id === action.targetId ? { ...item, isDone: !item.isDone } : item | ||
); | ||
case "DELETE": | ||
return state.filter((item) => item.id !== action.targetId); | ||
default: | ||
return state; | ||
} | ||
} | ||
|
||
function App() { | ||
const [todos, dispatch] = useReducer(reducer, mockDate); | ||
const idRef = useRef(3); | ||
|
||
const onCreate = (content) => { | ||
dispatch({ | ||
type: "CREATE", | ||
data: { | ||
id: idRef.current++, | ||
isDone: false, | ||
content: content, | ||
date: new Date().getTime(), | ||
}, | ||
}); | ||
}; | ||
|
||
const onUpdate = (targetId) => { | ||
dispatch({ type: "UPDATE", targetId: targetId }); | ||
}; | ||
|
||
const removeTodo = (targetId) => { | ||
dispatch({ | ||
type: "DELETE", | ||
targetId: targetId, | ||
}); | ||
}; | ||
Comment on lines
+48
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. App컴포넌트에서 상태 관리 함수가 많아질 경우 useReducer 를 활용해서 컴포넌트 외부에 따로 정리하는 것으로 알고 있었습니다! 혹시 사용기준을 잘 못 이해하고 있을까요?ㅠ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앗 아닙니다! 제가 말씀드린 의도는, 아무래도 함수는 사용되는 컴포넌트 코드 쪽에서 선언, 활용이 모두 이루어지는 것이 나중에 가독성 측면에 있어서 더 좋다고 생각해서 그렇게 리뷰 달았습니다 😁 |
||
return ( | ||
<> | ||
<GlobalStyle /> | ||
<AppContainer> | ||
<Header /> | ||
<Editor onCreate={onCreate} /> | ||
<List todos={todos} onUpdate={onUpdate} removeTodo={removeTodo} /> | ||
</AppContainer> | ||
</> | ||
); | ||
} | ||
|
||
const AppContainer = styled.div` | ||
display: flex; | ||
flex-direction: column; | ||
gap: 10px; | ||
width: 500px; | ||
margin: 0 auto; | ||
background-color: #f1f3ff; | ||
border-radius: 5px; | ||
padding: 30px; | ||
box-shadow: inset 0 0 20px #d6ddff; | ||
height: 80vh; | ||
box-sizing: border-box; | ||
|
||
/* 모바일 반응형 */ | ||
@media (max-width: 768px) { | ||
width: 360px; | ||
height: 560px; | ||
padding: 20px; | ||
} | ||
`; | ||
|
||
const GlobalStyle = createGlobalStyle` | ||
body { | ||
background-color: #788bff; | ||
margin: 0; | ||
padding: 0; | ||
font-family: 'Arial', sans-serif; | ||
display:flex; | ||
height:100vh; | ||
justify-content:center; | ||
align-items:center; | ||
|
||
/* 모바일 반응형 */ | ||
@media (max-width: 768px) { | ||
height:100vh; | ||
width:100%; | ||
} | ||
} | ||
`; | ||
Comment on lines
+114
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 모바일 반응형까지 고려하셨네요! 방금 폰으로 배포 링크 들어가봤는데 너무 멋집니다👍👍
Comment on lines
+103
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. styled-components 라이브러리에서 추후에 저희가 nextJS 과제를 진행할 때에 이렇게 클라이언트에서 새로운 또한 현재의 배포본에서는 페이지가 하나이고 요소도 많지 않기 때문에 |
||
|
||
export default App; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
import styled from "styled-components"; | ||
import { useRef, useState } from "react"; | ||
|
||
const Editor = ({ onCreate }) => { | ||
const [content, setContent] = useState(""); | ||
|
||
const contentRef = useRef(); | ||
Comment on lines
+5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용자가 아무 것도 입력하지 않은 상태에서 엔터를 눌러 입력하려고 할때, 여전히 dom 요소에 focus on 해주기 위해 너무 좋은 활용법이고, 나중에 form을 다룰 때에 양방향 바인딩 외에 |
||
|
||
const onChangeContent = (e) => { | ||
setContent(e.target.value); | ||
}; | ||
|
||
const onkeydown = (e) => { | ||
if (e.key === "Enter") { | ||
e.preventDefault(); | ||
onSubmit(); | ||
} | ||
}; | ||
|
||
const onSubmit = () => { | ||
if (content.trim() === "") { | ||
contentRef.current.focus(); | ||
return; | ||
} | ||
onCreate(content.trim()); | ||
setContent(""); | ||
}; | ||
|
||
return ( | ||
<EditorContainer> | ||
<Input | ||
ref={contentRef} | ||
type="text" | ||
placeholder="새로운 Todo..." | ||
onChange={onChangeContent} | ||
onKeyDown={onkeydown} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
이렇게 하시면 아마 되실 것 같아요! 그 차이가 onKeyDown, onKeyUp의 차이인데 onKeyUp은 키를 뗀 후에 트리거가 되어서 이미 input의 내용이 DOM에 적용된 후에 트리거가 발생합니다. 반대로 onKeyDown은 키를 누르는 순간에 트리거가 되기 때문에 글자가 아직 DOM에 반영되지 않았는데도 트리거가 될 수도 있는 경우가 있기에(그래서 가끔은 되는 경우도 있는 것 같습니다) 마지막 글자가 포함되는 것 같습니다! https://codevil.tistory.com/277 참고자료도 남길게요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 헉 너무 감사합니다ㅠㅠ 이거 때매 정말 뭐가 문제지 계속 생각했었는데... |
||
value={content} | ||
/> | ||
<Button onClick={onSubmit}>Add</Button> | ||
</EditorContainer> | ||
); | ||
}; | ||
|
||
const EditorContainer = styled.div` | ||
display: flex; | ||
gap: 10px; | ||
`; | ||
|
||
const Input = styled.input` | ||
flex: 1; | ||
padding: 15px; | ||
border: 1px solid rgb(220, 220, 220); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Input 컴포넌트에 border 설정이 중복해서 들어가있는 것 같습니다! border를 none으로 설정하시려면 요 라인은 삭제하셔도 좋을 것 같아요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앗 제가 border를 넣고 이후에 None을 했네요ㅠㅜ 알려주셔서 감사합니다💗 |
||
border-radius: 5px; | ||
background-color: #d6ddff; | ||
color: #788bff; | ||
outline: none; | ||
border: none; | ||
font-weight: 500; | ||
|
||
&::placeholder { | ||
color: #788bff; | ||
opacity: 1; | ||
} | ||
`; | ||
|
||
const Button = styled.button` | ||
cursor: pointer; | ||
width: 80px; | ||
border: none; | ||
background-color: #788bff; | ||
color: white; | ||
border-radius: 10px; | ||
font-size: 1rem; | ||
`; | ||
|
||
export default Editor; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import styled from "styled-components"; | ||
|
||
const Header = () => { | ||
return ( | ||
<HeaderContainer> | ||
<h1>TO DO</h1> | ||
<span>{new Date().toDateString()}</span> | ||
</HeaderContainer> | ||
); | ||
}; | ||
|
||
const HeaderContainer = styled.div` | ||
h1 { | ||
color: #788bff; | ||
} | ||
span { | ||
color: #8790ca; | ||
} | ||
`; | ||
|
||
export default Header; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,129 @@ | ||||||||||||||||||||||||||||||
import styled from "styled-components"; | ||||||||||||||||||||||||||||||
import TodoItem from "./TodoItem"; | ||||||||||||||||||||||||||||||
import { useState, useMemo } from "react"; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const List = ({ todos, onUpdate, setTodos, removeTodo }) => { | ||||||||||||||||||||||||||||||
const [search, setSearch] = useState(""); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const onChangeSearch = (e) => { | ||||||||||||||||||||||||||||||
setSearch(e.target.value); | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 저도 검색 기능은 생각해 보지 못했던 부분이라 참신하게 느껴졌어요! 만약 투두리스트 기능이 확대되어서 월 단위나 년 단위로 저장할 수 있게 된다면, 내가 언제 이 일을 했는지, 혹은 언제 해야 하는지 검색할 때 좋을 것 같아요~~ |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const getFilteredDate = () => { | ||||||||||||||||||||||||||||||
if (search === "") { | ||||||||||||||||||||||||||||||
return todos; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
return todos.filter((todos) => { | ||||||||||||||||||||||||||||||
return todos.content.toLowerCase().includes(search.toLowerCase()); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
Comment on lines
+12
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지금 getFilteredDate 함수를 보면 검색에 따라 todos를 필터링하는 기능을 수행하는 함수로 생각됩니다! 함수명을 getFilteredDate로 설정하셔서 이름만 보고는 날짜와 관련된 기능을 수행하는 것처럼 받아들여질 수도 있을 것 같아요! 함수/변수명 작명은 개발자의 평생 숙제죠...🥹 조금 더 직관적이고 가독성 높은 코드를 위해서 함수명을 getFilteredTodos와 같이 함수 기능에 더 가깝게 네이밍해보는 건 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 함수를 수정하면서 변수명도 같이 수정을 했었나봐요ㅠㅠ 함수명 연관되게 작성하도록 명심하겠습니당!! |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const filteredTodos = getFilteredDate(); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const { doneCount, notDoneCount } = useMemo(() => { | ||||||||||||||||||||||||||||||
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doneCount와 notDoneCount 계산 부분을 useMemo를 활용해서 효율적으로 코드를 작성해주신 것 같아요! todos 상태가 변경될 때만 계산이 일어나니까 불필요한 연산이 많이 줄어들 것 같습니다. 다만 지금 filteredTodos는 getFilteredDate() 함수에서 계속 계산되고 있는 것 같아요. 이 부분도 useMemo로 캐싱해서 todos나 search가 바뀔 때만 계산하도록 수정하면 더욱 좋을 것 같습니다..!!
Suggested change
|
||||||||||||||||||||||||||||||
const totalCount = todos.length; | ||||||||||||||||||||||||||||||
const doneCount = todos.filter((todo) => todo.isDone).length; | ||||||||||||||||||||||||||||||
const notDoneCount = totalCount - doneCount; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||
totalCount, | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 혹시 |
||||||||||||||||||||||||||||||
doneCount, | ||||||||||||||||||||||||||||||
notDoneCount, | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
}, [todos]); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||
<ListContainer> | ||||||||||||||||||||||||||||||
<TopWrapper> | ||||||||||||||||||||||||||||||
<h4>Todo List 🌱</h4> | ||||||||||||||||||||||||||||||
<CountWrapper> | ||||||||||||||||||||||||||||||
<span>todo : {notDoneCount}</span> | ||||||||||||||||||||||||||||||
<span>done : {doneCount}</span> | ||||||||||||||||||||||||||||||
</CountWrapper> | ||||||||||||||||||||||||||||||
</TopWrapper> | ||||||||||||||||||||||||||||||
<SearchInput | ||||||||||||||||||||||||||||||
type="text" | ||||||||||||||||||||||||||||||
placeholder="검색어를 입력하세요" | ||||||||||||||||||||||||||||||
value={search} | ||||||||||||||||||||||||||||||
onChange={onChangeSearch} | ||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||
<TodoWrapper> | ||||||||||||||||||||||||||||||
{filteredTodos.map((todos) => { | ||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||
<TodoItem | ||||||||||||||||||||||||||||||
key={todos.id} | ||||||||||||||||||||||||||||||
{...todos} | ||||||||||||||||||||||||||||||
onUpdate={onUpdate} | ||||||||||||||||||||||||||||||
todos={todos} | ||||||||||||||||||||||||||||||
setTodos={setTodos} | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TodoItem 컴포넌트에 onUpdate와 removeTodo를 전달하여 투두 업데이트와 삭제를 처리하고 있기 때문에 setTodos를 전달하지 않아도 괜찮을 것 같습니다! 실제로 TodoItem 컴포넌트에서 setTodos를 사용하고 있지 않기도 하고요 |
||||||||||||||||||||||||||||||
removeTodo={removeTodo} | ||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||
Comment on lines
+53
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 같은 가빈 님과 같은 생각입니다! 지금 setTodos를 props로 내보내고 있는데, 실제 TodoItem 컴포넌트에서 setTodos를 사용하지 않고 있어요. 불필요한 props는 제거하는 게 좋을 것 같습니다. 그리고 {...todos} 부분과 todos={todos} 부분도 중복된 것 같습니다. 이미 {...todos}로 todos 안의 모든 속성들을 전달하고 있기 때문에 굳이 todos={todos}로 두 번 전달할 필요가 없어요..! 이 부분도 삭제하면 더욱 깔끔한 코드가 될 것 같습니다 🙃
Suggested change
|
||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
})} | ||||||||||||||||||||||||||||||
</TodoWrapper> | ||||||||||||||||||||||||||||||
</ListContainer> | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const ListContainer = styled.div` | ||||||||||||||||||||||||||||||
display: flex; | ||||||||||||||||||||||||||||||
flex-direction: column; | ||||||||||||||||||||||||||||||
gap: 10px; | ||||||||||||||||||||||||||||||
margin-top: 40px; | ||||||||||||||||||||||||||||||
overflow-y: auto; | ||||||||||||||||||||||||||||||
scrollbar-width: none; | ||||||||||||||||||||||||||||||
scrollbar-width: none; | ||||||||||||||||||||||||||||||
-ms-overflow-style: none; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
&::-webkit-scrollbar { | ||||||||||||||||||||||||||||||
display: none; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const TopWrapper = styled.div` | ||||||||||||||||||||||||||||||
display: flex; | ||||||||||||||||||||||||||||||
justify-content: space-between; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
h4 { | ||||||||||||||||||||||||||||||
color: #788bff; | ||||||||||||||||||||||||||||||
margin: 0; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const CountWrapper = styled.div` | ||||||||||||||||||||||||||||||
display: flex; | ||||||||||||||||||||||||||||||
flex-direction: column; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
span { | ||||||||||||||||||||||||||||||
color: #8790ca; | ||||||||||||||||||||||||||||||
font-size: 0.8rem; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const SearchInput = styled.input` | ||||||||||||||||||||||||||||||
width: 100%; | ||||||||||||||||||||||||||||||
border: none; | ||||||||||||||||||||||||||||||
border-bottom: 1px solid #d9dfff; | ||||||||||||||||||||||||||||||
padding: 15px 0; | ||||||||||||||||||||||||||||||
color: #788bff; | ||||||||||||||||||||||||||||||
margin-bottom: 20px; | ||||||||||||||||||||||||||||||
background: transparent; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
&:focus { | ||||||||||||||||||||||||||||||
outline: none; | ||||||||||||||||||||||||||||||
color: #788bff; | ||||||||||||||||||||||||||||||
border-bottom: 1px solid #788bff; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
&::placeholder { | ||||||||||||||||||||||||||||||
color: #788bff; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const TodoWrapper = styled.div` | ||||||||||||||||||||||||||||||
display: flex; | ||||||||||||||||||||||||||||||
flex-direction: column; | ||||||||||||||||||||||||||||||
gap: 20px; | ||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
export default List; |
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.
모의 객체를 넣으신 것 같아요! (저두 류원님 고양이 만나고 싶어요🤍)
다만 보통 모의 객체는 데이터가 아직 준비되지 않은 상황이나, 서버가 연결되지 않은 상황에서 개발을 미리 진행하기 위해 넣는 것으로 알고 있습니다. 지금 투두리스트 프로젝트에서는 이미 할 일을 추가하고 삭제하는 기능이 잘 구현되어 있기 때문에 굳이 모의 객체가 있어야 하는지에 대해 살짝 의문이 듭니다! 더군다나 지금 모의 객체가 전부 고정된 데이터로 이루어져 있기 때문에 실제 해당 할 일을 하고 완료로 체크해도 새로고침하면 안 했다고 뜨게 됩니다. (is Done이 false로 고정되어 있으니까요..!) '투두리스트'라는 서비스의 목적을 고려하면 이 부분은 살짝 적절하지 않은 것 같습니다.
만약 저 세 가지 할 일을 단순히 모의 객체가 아니라 매일 하는 일들로 규정하고 싶으신 거면 아예 일반 투두리스트들과 선으로 분리해서 상단에 고정해놓으시는 것도 좋을 것 같아요! 그리고 mockDate라는 변수명은 배열의 내용을 제대로 설명하지 못하는 것 같습니다! mockTodos 등으로 수정해 보는 건 어떨까요?