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주차] 이효린 미션 제출합니다. #7

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

hyorish03
Copy link
Member

@hyorish03 hyorish03 commented Sep 3, 2024

안녕하세요 ! 이번주차부터 참여하게 된 이효린입니다. 잘 부탁드립니다 !
PR 작성 시간이 거의 개발한 시간 만큼 길어졌네요...ㅎㅎ 덕분에 PR 작성하는 과정에서 더 많이 배우는 것 같습니다.

이번 코드를 작성하면서 모듈화에 많이 신경썼는데.. 어떨지 잘 모르겠네요..😅 코드르 작성하면서 중복되는 부분이 생긴다면 최대한 모듈화 및 컴포넌트화 하려 노력했습니다.

또한 onkeydown 이벤트를 사용하는 과정에서 버그가 발생했었는데, 이를 해결하는 과정에서 새로운 것을 많이 배웠습니다. 이에 대한 내용은 하단에 정리해두었습니다.

미션을 수행하면서는 copilot을 사용하지 않았는데, 덕분에 제가 무심코 지나쳤던 문법들도 다시 한 번 되새길 수 있는 좋은 시간이었습니다. 감사합니다 !

알게 된 부분

Vercel 사용법

  • 지금까지 netlify만 사용해 배포해왔었는데 vercel이라는 새로운 도구를 활용할 수 있는 좋은 기회였습니다.

Emotion 사용법

  • Emotion을 사용해볼 기회가 없었는데 이번 기회를 통해 사용해보며 문법에 익숙해질 수 있었습니다.
  • Emotion과 Styled-components를 비교하며 생각하는 과정에서 스타일링 라이브러리의 성능에 관해 공부할 수 있었고, 두 라이브러리가 성능상 큰 차이는 없지만 Emotion의 경우 css 변수를 조립하여 컴포넌트 스타일링을 할 수 있다는 점, ts사용시 자동 타입지정까지 가능하다는 장점이 있다는 것을 알게되었습니다.

onKeyDown 함수와 한글 조합시 �발생하는 버그

recorder:한글입력후onkeydown 실행시 두 번 실행되는 버그

  • input 태그 내의 onKeyDown 이벤트 헨들러를 설정하여 'Enter' 키가 입력되었을 시 addTodo 함수를 실행하도록 코드를 작성하였습니다.

  • 그러나 위와 같은 버그가 발생하였고, 원인 분석 후 두 가지 방식으로 문제에 해결하려 하였습니다.

  • 원인

    • 한글 입력 방식에는 조합기, 즉 자음과 모음을 조합하여 완성형 글자를 만드는 방식을 사용합니다. 이를 IME라고 부릅니다.
    • 'Enter' 키 입력 당시 가장 최근에 조합된 한글의 자음과 모음이 조합이 완료된 상태인지 아닌지 알 수 없기 때문에 각각의 상태에서 모두 이벤트를 호출해주기 때문에 onKeyDown 이벤트가 2번 실행되는 것이었습니다.
  • 해결방안 1: onkeypress 이벤트로 대체

    • 그러나 최근 onkeypress 이벤트가 웹 표준에서 제외되어 더이상 지원하지 않을 예정이라해 다른 방안을 찾아보았습니다.
  • 해결방안 2: e.nativeEvent.isComposing 사용

    • input 태그에 입력중인 한글 문자의 자모가 조합중이라면 true를, 그게 아니라면 false를 값으로 갖는 isComposing을 활용하였습니다.
    • 따라서 isComposing이 true라면 addTodo 함수가 실행되지 않도록 구현함으로써 해결할 수 있었습니다.

생각해 볼 질문들

1. 지난주 미션과 비교했을 때, React로 구현하는 과정에서 어떤 점이 더 편리하거나 복잡했나요?

  • 컴포넌트 기반 개발
    리액트는 컴포넌트 기반으로 UI를 구성하기 때문에 UI를 재사용 가능한 단위로 나눌 수 있어 편리했습니다.
  • 리액트 훅
    리액트 훅을 사용하며 상태관리를 간편하게 할 수 있어 편리했습니다.

2. React의 Virtual DOM(가상 돔)이란 무엇이고, 이를 사용했을 때의 장점은 무엇이 있나요?

  • 가상 돔은 실제 돔의 가벼운 복사본으로, 메모리 상에 자바스크립트 객체 형태로 존재합니다
  • 리액트는 두 개의 가상 돔을 사용하며, 이전 가상돔과 새로운 가상돔을 diffing 알고리즘을 활용해 두 가상 돔 트리를 비교하며 어떤 요소가 변경되었는지 파악합니다.
  • 이렇게 변경사항이 파악되면 해당 변경사항만 실제 DOM에 반영하여 엽데이트 합니다.
  • 이러한 특장으로 인해 효울적인 렌더링이 가능하고, 실제 DOM을 직접 조작하는 것이 아니기에 렌더링 성능이 향상됩니다.

3. React에서 state와 props는 무엇이고 어떻게 사용하나요?

  • props: 컴포넌트가 외부에서 받는 데이터 즉, 부모 컴포넌트가 자식 컴포넌트로 전달하는 데이터
    입니다. props는 읽기 전용으로 컴포넌트 내부에서 직접 수정이 불가합니다.
  • state: 컴포넌트 내부에서 관리되는 데이터로, 컴포넌트의 상태를 변경하거나 업데이트 할 때 사용됩니다. state는 읽기와 쓰기 모두 가능하며, setState 함수를 활용해 값이 변경됩니다.

4. React에서 컴포넌트를 분리하는 기준은 무엇일까요?
재사용성, 단일 책임 원칙, 컴포넌트 계층 구조, 컨테이너 컴포넌트와 프레젠테이션 컴포넌트를 고려해 분리할 수 있습니다.

배포

Copy link

@Sieonn Sieonn left a comment

Choose a reason for hiding this comment

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

효린님이 지금 제가 구현하고 싶어하는 코드 추구미셔요.

너무 보기 좋고 깔끔하고 아주 많이 참고가 되었습니다. 저는 처음에 LocalStorage생각을 안하고 일단 구현하고 그거를 담으면 되지 않을까? 했던 것이 코드를 왕왕 부풀게 만들었답니다..

효린님이 파일을 생성하는 순서랑 방식을 보고 놀랐습니다!!😭
고생많으셨고 앞으로도 화이팅해봐요!! 홧팅!!! 효린최고!

}
}, []);
const notDoneTodoList = todoList.filter((todo) => todo.done === false);
const doneTodoList = todoList.filter((todo) => todo.done === true);
Copy link

Choose a reason for hiding this comment

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

그냥 Done기능 todoList 기능 다 작성했던 제가 하고 싶었던 코드예요..ㅠㅠㅠ 엄청 깔끔하고... 다음엔 꼭 이렇게 작성해보고 싶어요.

Copy link
Member Author

Choose a reason for hiding this comment

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

과분한 칭찬 너무 감사합니다 .. 🥹 js의 Array 메서드는 공부해두면 항상 도움이 되는 것 같아요 !

<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<link rel="icon" type="image/svg+xml" href="/public/redHeart.svg" />
Copy link

Choose a reason for hiding this comment

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

디테일이 진짜 멋져요. 지나치기 쉬운데 이런 꼼꼼함을 배워야겠습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

아이고 과찬이십니다

Copy link

Choose a reason for hiding this comment

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

진짜 꼼꼼함 인정!!!저도 효린이 코드보고 많이 배웁니다!!!!

import { getLocalStorage } from './getLocalStorage';

export const addToLocalStorage = (value) => {
const prevTodo = getLocalStorage();
Copy link

Choose a reason for hiding this comment

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

이렇게 따로 js를 분리해서 보니까 코드 보기가 더 편한 것 같네요!
이번에 진행하면서 컴포넌트, js, css 나누는 기준이나 방법이 너무 어려웠는데 이런 방법이 있다는 것을 배우고 갑니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 저도 이게 옳은 코드 스타일이다라고 말하긴 어렵지만 (아직은 이런 부분에 대한 확신이 없네요 ..🥲) 함수를 따로 js 파일로 빼서쓰니 깔끔하게 느껴져서 해봤씁니다 !

@@ -25,10 +26,10 @@ function DoneList({ doneTodoList, setTodoList }) {
paddingRight: '20px',
})}
>
<TodoItem todoList={doneTodoList} setTodoList={setTodoList} />
<TodoItem todoList={list} setTodoList={setTodoList} />
Copy link

Choose a reason for hiding this comment

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

제가 정말 구현하고 싶었던 내용이예요. 너무 깔끔해서 속이 시원합니다. 모양이 같은데 각각의 item과 섹션에만 영향을 줘야하는데 그런걸 처음에 다 설계를 하고 구현하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

엇 아닙니다 !! 제 커밋 내역을 보면 알겠지만 처음엔 별 생각 없이 구현하다가 동일한 구조가 보이면 그때 컴포넌트화 시키는 편인 것 같습니다 ! 사실 처음부터 설계하고 개발하는게 최고겠지만.. 아직 많이 부족하네요 😅

</div>
</div>
))}
<TodoItem todoList={notDoneTodoList} setTodoList={setTodoList} />
Copy link

Choose a reason for hiding this comment

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

DoneList, TodoList, TodoItem를 따로 분리하셨는데 따로 분리하신 이유가 있으실까요??? 이렇게 분리해서 관리하면 더 편할까요???(๑•̀ㅂ•́)و✧

@@ -42,7 +42,10 @@ function Header({ setTodoList }) {
type="text"
ref={inputRef}
onKeyDown={(e) => {
e.key === 'Enter' && addTodo();
if (e.nativeEvent.isComposing) return;
Copy link

Choose a reason for hiding this comment

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

오잉? e.nativeEvent.isComposing 처음 보는데 이런 오류도 생길 수 있군요. 이번트 핸들링 관련 같은데 설명도 듣고싶어요. 이해가 안 가기 때문이죠..

Copy link
Member

Choose a reason for hiding this comment

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

@Sieonn
저도 이 문제로 한참 애먹어서 블로그 글도 작성했었다죠... 그냥 자바스크립트 자체의 버그입니다 🥹
keydown/keyup에서 한글 입력 시 함수가 두 번 실행되는 경우

Copy link
Member

Choose a reason for hiding this comment

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

그래서 keyDown 핸들러를 구현하는 것보다는 <form> 태그로 감싸서 사용하는 게 훨씬 편한 것 같아요!

Copy link

Choose a reason for hiding this comment

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

헐 저도 오류는 처음보네요! 자바스크립트 자체 버그도 존재했다니ㅠㅠ저도 설명 듣고싶어요!

import App from './App.jsx'

createRoot(document.getElementById('root')).render(
<StrictMode>
Copy link

Choose a reason for hiding this comment

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

이거 보고 StricMode가 뭔지 왜 자동으로 생성되어 감싸고 있는지 찾아보는 계기가 됐어요! 감사합니다. 휘발성 지식 획득~🦖

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 StrictMode를 사용하면 개발 과정에서 발생하는 버그를 빠르게 잡을 수 있다는 장점이 있지만,, 전 디버깅하는 과정에서 console.log가 2회씩 찍히는게 불편해서 삭제했습니다 😞

StrictMode 관련 문서 추천드려용 ! (공식문서임)
https://ko.react.dev/reference/react/StrictMode

Copy link

Choose a reason for hiding this comment

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

헐 저도 문서 같이 봐볼께요! 좋은 정보 감사합니다!

@@ -1,8 +1,9 @@
import { getLocalStorage } from './getLocalStorage';

export const setLocalStorage = (value) => {
const newTodo = getLocalStorage().concat({
id: localStorage.getItem('todo').length + 1,
const prevTodo = getLocalStorage();
Copy link

Choose a reason for hiding this comment

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

오... 이렇게 아이디를 미리 지정해 두고 하면 깔끔하겠네여??. 우와...

Copy link
Member

@corinthionia corinthionia left a comment

Choose a reason for hiding this comment

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

안녕하세요 효린 님!
효린 님의 코드를 보면서 추상화에 많은 신경을 쓰신 게 보였습니다 ㅎㅎ 컴포넌트나 함수 분리를 잘해 주셔서 직관적으로 이해하기 편했던 것 같아요 👍🏻
미션 수행하시느라 고생하셨습니다 🙌

@@ -0,0 +1,9 @@
body {
Copy link
Member

Choose a reason for hiding this comment

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

emotion의 <Global> 컴포넌트를 이용하여 Global style을 적용해 봐도 좋을 것 같아요!
Emotion - Global Styles

function App() {
return <div>React Todo</div>;
const [todoList, setTodoList] = useState(getLocalStorage() || []);
Copy link
Member

Choose a reason for hiding this comment

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

로컬스토리지 관련 코드를 util 함수로 분리하니 훨씬 깔끔하네요 👍🏻

const doneTodoList = todoList.filter((todo) => todo.done === true);
return (
<div
css={css({
Copy link
Member

Choose a reason for hiding this comment

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

css() 함수 부분을 별도로 분리하여 작성하면 가독성이 더욱 좋아질 것 같아요!

display: 'flex',
justifyContent: 'center',
alignItems: 'center',
background: 'linear-gradient(pink,skyblue)',
Copy link
Member

Choose a reason for hiding this comment

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

색상이 아주 예쁘네요 👍🏻

};

useEffect(() => {
inputRef.current.focus();
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻👍🏻

@@ -42,7 +42,10 @@ function Header({ setTodoList }) {
type="text"
ref={inputRef}
onKeyDown={(e) => {
e.key === 'Enter' && addTodo();
if (e.nativeEvent.isComposing) return;
Copy link
Member

Choose a reason for hiding this comment

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

@Sieonn
저도 이 문제로 한참 애먹어서 블로그 글도 작성했었다죠... 그냥 자바스크립트 자체의 버그입니다 🥹
keydown/keyup에서 한글 입력 시 함수가 두 번 실행되는 경우

@@ -42,7 +42,10 @@ function Header({ setTodoList }) {
type="text"
ref={inputRef}
onKeyDown={(e) => {
e.key === 'Enter' && addTodo();
if (e.nativeEvent.isComposing) return;
Copy link
Member

Choose a reason for hiding this comment

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

그래서 keyDown 핸들러를 구현하는 것보다는 <form> 태그로 감싸서 사용하는 게 훨씬 편한 것 같아요!

import { css } from '@emotion/react';
import { TodoItem } from './TodoItem';

function List({ list, setTodoList, listName }) {
Copy link
Member

Choose a reason for hiding this comment

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

할일/완료한일 컴포넌트를 따로 분리하지 않고 List 컴포넌트 하나로 각각을 렌더링할 수 있도록 추상화한 점 정말 좋습니다!! 👍🏻👍🏻

Copy link

Choose a reason for hiding this comment

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

저도 추상화하는 부분을 어떻게해야할지가 너무 고민이였는데 효린님 코드보고 배우는거밖에 없는 것 같습니다!!


export const addToLocalStorage = (value) => {
const prevTodo = getLocalStorage();
const newTodo = prevTodo.concat({
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 연산자를 더 많이 사용하는 것 같아요!

Comment on lines +37 to +38
updateFromLocalStorage(todo.id);
setTodoList(getLocalStorage());
Copy link
Member

Choose a reason for hiding this comment

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

현재 순서가 로컬 스토리지에 반영 -> 로컬 스토리지에서 불러오기 -> 불러온 값으로 state 변경이 되고 있는데,
state 변경 -> 변경된 state를 로컬 스토리지에 적용 하는 방식이 더욱 간단할 것 같아요!

Copy link

@jissssu jissssu left a comment

Choose a reason for hiding this comment

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

효린님! 첫 미션 수행하시느라 정말 수고하셨어요!
효린님 코드 보면서 그전에 저도 해보고싶던 utils 폴더 구조와 localstorage 코드 로직부분을 정말 많이 배울 수 있었습니다! 또한 컴포넌트 하나로 렌더링 가능하게 추상화 하신 점도 제가 못했던 부분이라 덕분에 또 알아갑니다! 앞으로의 미션도 같이 화이팅해요!

<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<link rel="icon" type="image/svg+xml" href="/public/redHeart.svg" />
Copy link

Choose a reason for hiding this comment

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

진짜 꼼꼼함 인정!!!저도 효린이 코드보고 많이 배웁니다!!!!

Comment on lines +10 to +16
useEffect(() => {
if (getLocalStorage() === null) {
localStorage.setItem('todo', JSON.stringify([]));
}
}, []);
const notDoneTodoList = todoList.filter((todo) => todo.done === false);
const doneTodoList = todoList.filter((todo) => todo.done === true);
Copy link

Choose a reason for hiding this comment

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

useEffect를 활용하여 로컬 스터리지 초기화 처리 저도 적용해보겠습니다! 직관적인 변수,함수명 제가 잘 못하는 부분인데 참고하겠습니다!

import { css } from '@emotion/react';
import Header from './Components/Header';
import { useEffect, useState } from 'react';
import { getLocalStorage } from './utils/getLocalStorage';
Copy link

Choose a reason for hiding this comment

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

utils로 따로 분리한 부분! 진짜 깔끔하고 제가 다음번에 꼭해야겠다고 생각했던 부분인데 또 한번 배우네요!

deleteFromLocalStorage(id);
setTodoList(getLocalStorage());
}}
>
Copy link

Choose a reason for hiding this comment

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

uills부분 한번 더 배우고갑니다! onClick이벤트 핸들러를 통해로컬 스토리지 적용한 부분 정말 간결하고 저도 이렇게 작성하고싶어요!

@@ -42,7 +42,10 @@ function Header({ setTodoList }) {
type="text"
ref={inputRef}
onKeyDown={(e) => {
e.key === 'Enter' && addTodo();
if (e.nativeEvent.isComposing) return;
Copy link

Choose a reason for hiding this comment

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

헐 저도 오류는 처음보네요! 자바스크립트 자체 버그도 존재했다니ㅠㅠ저도 설명 듣고싶어요!

import { css } from '@emotion/react';
import { TodoItem } from './TodoItem';

function List({ list, setTodoList, listName }) {
Copy link

Choose a reason for hiding this comment

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

저도 추상화하는 부분을 어떻게해야할지가 너무 고민이였는데 효린님 코드보고 배우는거밖에 없는 것 같습니다!!

Comment on lines +29 to +32
<div
css={css({
cursor: 'pointer',
textDecoration: todo.done ? 'line-through' : 'none',
Copy link

Choose a reason for hiding this comment

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

할 일 항목 완료여부에 따라 동적으로 설정하신 부분도 명확하게 보여서 너무 좋아요!!!!!

import App from './App.jsx'

createRoot(document.getElementById('root')).render(
<StrictMode>
Copy link

Choose a reason for hiding this comment

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

헐 저도 문서 같이 봐볼께요! 좋은 정보 감사합니다!

Comment on lines +1 to +8
import { getLocalStorage } from './getLocalStorage';

export const deleteFromLocalStorage = (id) => {
const prevTodo = getLocalStorage();
const newTodo = prevTodo.filter((todo) => todo.id !== id);
localStorage.setItem('todo', JSON.stringify(newTodo));
alert('삭제되었습니다.');
};
Copy link

Choose a reason for hiding this comment

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

함수 이름인 addToLocalStorage, deleteFromLocalStorage, getLocalStorage, updateFromLocalStorage가 각 함수의 역할을 명확하게 설명하고 있어서 코드를 이해하기 정말 쉬웠습니다! 함수 이름을 짓는 기준이 혹시 있으신가요? 저도 다음번에는 더 직관적이게 개선하고싶어요!

@@ -10,6 +10,8 @@
"preview": "vite preview"
},
"dependencies": {
"@emotion/react": "^11.13.3",
"eslint-plugin-emotion": "^11.0.0",

Choose a reason for hiding this comment

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

devDependencies에 추가하면 좋을 것 같습니다.

@@ -1,5 +1,54 @@
/** @jsxImportSource @emotion/react */

Choose a reason for hiding this comment

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

다음과 같이 vite.config 수정하는 것으로 babel pragma를 따로 쓰지 않아도 됩니다.

export default defineConfig({
  plugins: [
    react({
      jsxImportSource: '@emotion/react',
    }),
  ],
});

그리고 이게 왜 필요한건지 찾아보시는 것도 좋을 것 같습니다.
https://emotion.sh/docs/css-prop

@@ -1,5 +1,54 @@
/** @jsxImportSource @emotion/react */
import { css } from '@emotion/react';
import Header from './Components/Header';

Choose a reason for hiding this comment

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

이 경우 디렉토리 이름이 소문자인 것이 좋겠네요.

Suggested change
import Header from './Components/Header';
import Header from './components/Header';

Comment on lines +9 to +14
const [todoList, setTodoList] = useState(getLocalStorage() || []);
useEffect(() => {
if (getLocalStorage() === null) {
localStorage.setItem('todo', JSON.stringify([]));
}
}, []);

Choose a reason for hiding this comment

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

초기 로컬스토리지 값은 undefined라 의미가 없는 코드일 것 같네요.
만약 값이 없을 경우 예외 처리 해주고 싶다면 이미 useState 쓸때 되어 있는 것 같아요.

>
<Header setTodoList={setTodoList} />
<List
listName="📋 TO DO"

Choose a reason for hiding this comment

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

이런 매직 스트링은 지양하는 것이 좋습니다. 상수로 분리해주세요.

Suggested change
listName="📋 TO DO"
const LISTNAME = {
TODO: "TO DO",
DONE: "DONE",
}
<List
listName={`📋 ${LISTNAME.TODO}`}

Comment on lines +44 to +48
onKeyDown={(e) => {
if (e.nativeEvent.isComposing) return;
if (e.key === 'Enter') {
addTodo();
}

Choose a reason for hiding this comment

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

로직이 퍼져있는데요 handleKeyDown을 만들고 그 안에서 관리를 하는 것이 좋을 것 같습니다.

height: '220px',
width: '100%',
padding: '18px 18px 0 18px',
borderBottom: listName == 'TO DO' ? 'solid 1px lightgray' : 'none',

Choose a reason for hiding this comment

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

listName이 실제로는 "📋 TO DO" 이런 식이라서 실제로는 적용이 안되고 있습니다. 역시 매직 스트링을 써서 문제가 되는 부분입니다.

paddingRight: '20px',
})}
>
<TodoItem todoList={list} setTodoList={setTodoList} />

Choose a reason for hiding this comment

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

네이밍이 어색한 부분이 있네요. TodoItem인데 list를 전달해주고 있어서 그런 것 같습니다. 현재 TodoItem의 역할이 단순 map만 돌고 있는데. 차라리 다음과 같이 하는 게 좋겠습니다.

Suggested change
<TodoItem todoList={list} setTodoList={setTodoList} />
todoList.map((todo, index) => (
<TodoItem key={index} todo={todo} setTodoList={setTodoList} />
)

Comment on lines +7 to +8
export const TodoItem = ({ todoList, setTodoList }) =>
todoList.map((todo, index) => (

Choose a reason for hiding this comment

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

위에서 이어서, todo를 받고 todo에 대한 UI를 렌더링 하는 역할을 하는 것이 적절해보입니다.

@@ -0,0 +1,3 @@
export const getLocalStorage = () => {

Choose a reason for hiding this comment

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

만약 util 함수로 getLocalStorage라는 이름으로 함수를 만들고 싶었다면 key를 매개변수로 받아서 쓸수 있게 하는 것이 좋을 것 같습니다.

현재는 todo라는 관심사에 강하게 결합되어 있습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants