[3주차] 한규진 미션 제출합니다. #16

9yujin · 2022-04-01T05:53:55Z

3주차 미션: React-Todo: Refactor!💌

안녕하세요 한규진입니다!!

0.

https://9yujin.github.io/react-todo-15th/

배포링크는 이전과 같습니다. 2주차 과제에서 폴더 이름의 대소문자를 변경하면서 깃에 반영이 되지 않아 vercel 배포 시 문제가 됐던 경험이 있었습니다. 이 방법으로 해결할 수 있습니다. 하지만 세팅해둔게 있다보니 주소는 그대로 쓰도록 하겠습니다..!

1.

애를 많이 먹었습니다. 타입스크립트를 아예 처음 접했다보니, 이해하고 썼다기보단 한줄 한줄 구글링해가며 붙여넣은 것에 가깝다고 볼 수 있을 것 같습니다. 타입스크립트 활용에 대해서 많은 코드리뷰 부탁드립니다😻

커스텀 훅에 관한 부분은 2주차 미션에서 현재 님과 선종 님의 코드를 많이 참고했습니다. 보다 감탄할 정도로 배워갈게 정말 많은 코드였습니다. 정말 감사합니다😁

useForm에서 handler 함수를 넘겨받아서 사용하는 점이 인상깊었습니다. 그 부분을 활용하기 위해서 등록된 투두를 수정하는 기능을 추가해보았습니다. 투두를 추가할 때와 동일하게 useForm과 useInput 커스텀 훅을 사용합니다. 커스텀훅을 사용하니 재사용이 정말 편해지네요. 많이 배워갑니다. 다만, handler 함수의 타입을 설정할 때 any가 아닌 다른 방법을 찾지 못한 게 아쉽습니다.

추가로, 과제를 진행하면서 구글링했던 글들을 파일 안에 주석으로 남겨놓았습니다. 참고하시면 좋을 것 같습니다!!

2.

다만, 이런 오류가 종종 나왔습니다.

정상	오류

기존의 값을 initialValue로 주어서 그대로 사용할 수 있도록 했는데, 빈 input 박스가 나오는 경우가 꽤 자주 있습니다. 새로고침하면 다시 정상적으로 작동되네요. 왜 이럴까요..?ㅠㅠ

+) 헉 찾았습니다,,, useForm에서 handler 함수를 실행한후에 setValue('')로 비워주는데, 이게 계속 남는것 같네요.. 수정해보겠습니다

3.

두번째로는 해결하지 못한 부분은 Modal.tsx 부분입니다.
처음에는 todoItem 오른쪽에 threeDot 버튼 하나만 두고 클릭했을 시 모달을 띄워서 삭제와 수정을 선택하도록 디자인을 했었습니다. 이전에 만들어두었던 모달 컴포넌트가 있어서 그대로 타입스크립트로 바꿔서 가져오는 과정에서 막히고 말았습니다. children을 받아와 사용하고, 상위 컴포넌트에서 ref를 받아오기 위해서 forwardRef라는 것도 사용하는데 (사실 정확히 이해한 개념은 아닙니다ㅠ), 이 두가지 이유 때문에 타입스크립트 적용이 뭔가 복잡해진 것 같습니다. 혹시 이 부분도 아시는 분 계시면 가르침을 주시면 정말 감사하겠습니다ㅠㅠ

그래서 모달창을 만드는 방법 대신, 이전 성우님 과제를 본따 remove 버튼 옆에 edit 버튼을 추가했습니다. 작은 버튼 두개가 한쪽에 붙어있는 것이 깔끔해보이지는 않아서 많이 아쉽네요.

9yujin

feat : 투두 삭제 구현

refactor : normalize.css 와 구글 웹폰트 embed link로 변경

build : github page 배포 설정

…into refactor

S-J-Kim

안녕하세요 규진님

프론트엔드 멘토 김선종입니다.

제 코드를 참고해서 많은 도움이 되셨나요? 리뷰하는 내내 멘토로서의 역할을 잘 한것 같아 굉장히 뿌듯했답니다. 하지만 제 코드는 참고할수 있는 코드지만 정답인 코드는 아니고, 리팩토링의 여지가 많이 남아있습니다. 규진님께서 이 코드들을 조금 더 재사용성 좋게 리팩토링 해보시면 또 많은 것들을 배울 것이라 확신합니다.

과제 하느라 고생하셨습니다

S-J-Kim · 2022-04-01T15:35:58Z

src/components/Modal.tsx

+// 예전에 모달창 컴포넌트로 만들어놨던 소스가 있어서, 사용하려고 갖고왔는데 타입스크립트로 바꾸는 과정에
+// 막혀서 사용하지 못했습니다.. children을 넘길땐 React:FC 타입을 사용하라고 봤는데, forwardRef와는 같이 사용을 못하는것 같네요.
+// 혹시 이런 상황에선 어떻게 해야하는지 알수 있을까요??


forwardRef도 좋지만, 리액트에서 모달을 구현하기 위해 portal을 많이 사용합니다. Portals

덕분에 처음 알게 되었습니다..!

S-J-Kim · 2022-04-01T15:40:46Z

src/hooks/useForm.tsx

+
+//handler : any... 방법을 못찾겠습니다.ㅜㅜㅜ
+const useForm = (initialValue: string, handler: any, id: string | null) => {


handler는 useTodoContext의 함수중 하나이기 때문에 직접 handler의 타입을 선언해주면 됩니다.

interface IHandler { handler: (value: string, id: string) => void; }

이런 식으로 해보려고 했는데, handler로 들어오는 두 함수 addItem(value)과 editItem(value,id)에 쓰이는 매개변수가 달라서 너무 어렵습니다.. 혹시 제가 잘못 이해하고 있는걸까요..?ㅠㅠ

type TodoContextHandler = (id: string) => void // delete, toggle | (value: string) => void // add | (value: string, id: string) => void // edit

Union type을 통해 해결이 가능합니다. 타입스크립트 참 어렵죠 이런거때문에

S-J-Kim · 2022-04-01T15:45:34Z

src/hooks/useLocalStorage.tsx

+const useLocalStorage = (key: string, initialState: ITodoItem[]) => {
+    // list state 초기화
+    const [list, setList] = useState<ITodoItem[]>(() => JSON.parse(localStorage.getItem(key) || '') || initialState);
+
+    // list 상태 변화할 때마다 로컬스토리지 동기화
+    useEffect(() => {
+        localStorage.setItem(key, JSON.stringify(list));
+    }, [key, list]);
+
+    return { list, setList };
+};


Suggested change

const useLocalStorage = (key: string, initialState: ITodoItem[]) => {

// list state 초기화

const [list, setList] = useState<ITodoItem[]>(() => JSON.parse(localStorage.getItem(key) || '') || initialState);

// list 상태 변화할 때마다 로컬스토리지 동기화

useEffect(() => {

localStorage.setItem(key, JSON.stringify(list));

}, [key, list]);

return { list, setList };

};

const useLocalStorage<T> = (key: string, initialState: T) => {

// list state 초기화

const [list, setList] = useState<T>(() => JSON.parse(localStorage.getItem(key) || '') || initialState);

// list 상태 변화할 때마다 로컬스토리지 동기화

useEffect(() => {

localStorage.setItem(key, JSON.stringify(list));

}, [key, list]);

return { list, setList };

};

이렇게 제네릭으로 hooks를 리팩토링하고, 이 훅이 필요한 컴포넌트에서는,

const { list, setlist } = useLocalStorage<ITodoItem[]>(null, todoItem)

이렇게 쓸수 있겠죠. 이러면 useLocalStorag의 범용성이 증가할거 같네요

제네릭이 이렇게 사용하는것이었군요..!

S-J-Kim · 2022-04-01T15:47:50Z

src/hooks/useTodoContext.tsx

@@ -0,0 +1,53 @@
+// https://github.com/CEOS-Developers/react-todo-15th/pull/11/files#diff-4fd5b35d6c3fbfd63c97f711bd5fe3408b11551836f6cec1ecfb67a498d2843a
+// custom hook에 관한 부분 김선종 멘트님 코드 많이 참고했습니다..!


열심히 배워가겠습니다ㅎㅎ

S-J-Kim · 2022-04-01T16:01:14Z

src/hooks/useInput.tsx

+const useInput = (initialState: string) => {
+    const [value, setValue] = useState(initialState);
+
+    const onChange = (e: ChangeEvent<HTMLInputElement>) => {
+        setValue(e.target.value);
+    };
+
+    return { value, setValue, onChange };
+};


제 코드에서 많은 부분을 참고하셨으니 추가로 더 말씀을 드려보자면, 제가 일부러 상태와 메서드의 이름을 value, onChange로 지은 이유는, 컴포넌트에서 바인딩할 때 코드를 더 줄일 수 있기 때문입니다.

import useInput from 'hooks' ... const bindInput = useInput('') return ... <input {...bindInput} /> ...

이런식으로 코드를 크게 줄일수 있죠..! 하지만 setValue는 useForm을 위해 같이 리턴시킨 것이기 때문에, 해당 함수를

Suggested change

const useInput = (initialState: string) => {

const [value, setValue] = useState(initialState);

const onChange = (e: ChangeEvent<HTMLInputElement>) => {

setValue(e.target.value);

};

return { value, setValue, onChange };

};

const useInput = (initialState: string) => {

const [value, setValue] = useState(initialState);

const onChange = (e: ChangeEvent<HTMLInputElement>) => {

setValue(e.target.value);

};

return { inputBindings: { value, onChange}, setValue };

};

로 바꾼다면 <input/> 만 사용하는 컴포넌트에서도 useInput을 사용할 수 있겠죠?

대박이네요... 이게 진짜 컴포넌트 같습니다...

jhj2713

안녕하세요. 코드리뷰 파트너 주효정입니다! 지난번에도 코드리뷰 하면서 느꼈지만 ui도 신경 많이 쓰시고 코드도 꼼꼼하게 작성하셔서 리뷰하면서 감탄을 많이 하게 되는 것 같습니다! 토스트 메시지도 그렇고 이번에는 모달창 사용도 고려하신 점이 대단하신 것 같아요:thumbsup:
특히 이번에는 custom hook을 다양하게 사용하신 점이 인상적이었습니다.

코드 보면서 느꼈던 점과 몇몇 생각들을 짧게 남겨봤습니다. 감사합니다!

jhj2713 · 2022-04-02T13:26:45Z

src/components/TodoItem.tsx

@@ -0,0 +1,176 @@
+// 1. https://blog.devgenius.io/using-styled-components-and-props-with-typescript-react-a3c32a496f47
+// 2. https://krpeppermint100.medium.com/ts-useref-%EC%9E%90%EC%84%B8%ED%9E%88-%EC%95%8C%EC%95%84%EB%B3%B4%EA%B8%B0-typescript-uselayouteffect-c9f1cf02ca5a
+// 3. https://close-up.tistory.com/entry/React-%EC%BB%B4%ED%8F%AC%EB%84%8C%ED%8A%B8-%ED%8A%B9%EC%A0%95-%EC%98%81%EC%97%AD-%EC%99%B8-%ED%81%B4%EB%A6%AD-%EA%B0%90%EC%A7%80


오... 저는 늘 어떤 자료 참고했는지 다시 검색하면서 찾아봤는데 이렇게 주석으로 표시해놓으면 다시 검색할 필요가 없어서 코드 짜면서 편할 것 같네요!

jhj2713 · 2022-04-02T13:28:00Z

src/components/TodoItem.tsx

+    item: ITodoItem;
+}
+
+interface IBtn {


IBtn이라고 선언하는 건 너무 포괄적인 느낌이라 IRadioBtn이나 더 구체적인 네이밍도 좋지 않을까요..?!

jhj2713 · 2022-04-02T13:30:12Z

src/components/TodoItem.tsx

+    border-radius: 50%;
+    border: 2px solid #8989bb;
+    cursor: pointer;
+    background-color: ${(props: IBtn) => (props.done ? '#8989bb' : 'none')};


Suggested change

background-color: ${(props: IBtn) => (props.done ? '#8989bb' : 'none')};

background-color: ${({ done }) => (done ? '#8989bb' : 'none')};

이렇게 destructuring 해줘도 좋을 것 같아요!

jhj2713 · 2022-04-02T13:30:49Z

src/components/TodoItem.tsx

+                    }}
+                    ref={editFormRef}
+                >
+                    <InputEdit value={value} onChange={onChange} ref={inputRef} />


Edit 기능까지 구현하신 점 너무 대단합니다:thumbsup:

jhj2713 · 2022-04-02T13:33:31Z

src/components/TodoItem.tsx

+const StyledRemove = styled(Remove)`
+    padding-right: 10px;
+    fill: #d3d3d3;
+    &:hover {
+        fill: black;
+    }
+`;


Suggested change

const StyledRemove = styled(Remove)`

padding-right: 10px;

fill: #d3d3d3;

&:hover {

fill: black;

}

`;

const StyledRemove = styled(Remove)`

cursor: pointer;

padding-right: 10px;

fill: #d3d3d3;

&:hover {

fill: black;

}

`;

마우스 포인터가 활성화 되어있어야 누르는 버튼이라는 느낌이 들어서 Remove버튼과 Edit버튼에는 cursor: pointer를 추가하는 것도 좋지 않을까 하는 개인적인 생각입니다..!

jhj2713 · 2022-04-02T13:38:43Z

src/hooks/useTodoContext.tsx

+        if (value) {
+            if (list.find((item) => item.content === value)) {
+                runToast('동일한 내용으론 작성할 수 없어요!');
+            } else {
+                createItem();
+            }


제가 기억하기로는 1주차 코드에서는 content로 delete 연산을 진행했기 때문에 동일한 내용을 작성할 수 없도록 처리한 걸로 아는데요 이번에는 id로 delete 연산을 진행하니까 동일한 내용 필터링하는 부분은 이제 없어도 되지 않을까 하는 개인적인 생각입니다..!

Suggested change

if (value) {

if (list.find((item) => item.content === value)) {

runToast('동일한 내용으론 작성할 수 없어요!');

} else {

createItem();

}

if (value.replace(/\s/g, "")) {

createItem();

그리고 value 자체만 검증하면 공백만 입력된 경우에도 할 일로 추가되기 때문에 정규식 사용해서 공백 제거한 후 검증하는 코드로 작성해봐도 괜찮을 것 같아요!

꼼꼼한 리뷰 감사드립니다..!!

9yujin and others added 28 commits March 23, 2022 02:45

feat : 기초 마크업 & 디렉토리 구조 설정

3e1a06f

feat : 투두 아이템 추가 & 로컬스토리지 동기화

160f6cf

feat : 토글 기능 구현 & 토스트 알림 구현

cd8c2ed

Merge pull request #1 from 9yujin/9yujin

dd3d336

9yujin

feat : 투두 삭제 구현

270f5e7

Merge pull request #2 from 9yujin/9yujin

7f5e3f3

feat : 투두 삭제 구현

refactor : normalize.css 와 구글 웹폰트 embed link로 변경

bc6386a

Merge pull request #3 from 9yujin/9yujin

08f81f0

refactor : normalize.css 와 구글 웹폰트 embed link로 변경

build : github page 배포 설정

5276d22

Merge pull request #4 from 9yujin/9yujin

319b745

build : github page 배포 설정

docs : Update README.md

aec0aac

docs : update README.md

a91d5d0

fix : input form 오류 수정

c29c2dc

fix : 깃에서 폴더명 변경이 되지 않았던 오류 수정

6377143

refactor : 2주차 코드리뷰 반영

aba38c2

feat : context api 이용해서 상태관리 / useLocalStorage custom hooks

5ca67c7

refactor : addTodo, deleteTodo custom hooks

5749406

refactor : toggleItem function

65388f8

refactor : app.js에서 props 내려주던거 삭제 / tsconfig.json

ae643e2

refactor : typescript 적용..ㅎㅅㅎ

1cc003b

refactor : typescript 적용완료..ㅎㅅㅎ

ea7a9a0

design : radiobutton hover effect

737db69

feat : 할일 수정하기 기능 추가ㅎㅅㅎ

fbd032f

모달.. 넣어보려다 실패

c45ada6

docs : update README.md

e6057df

Update README.md

249bcd3

refactor : 버려진 코드 삭제

d7eeff8

Merge branch 'refactor' of https://github.com/9yujin/react-todo-15th …

5715038

…into refactor

S-J-Kim reviewed Apr 1, 2022

View reviewed changes

fix: 로컬스토리지 비어있을때 json 파싱 문법 오류 수정

b7e4dc8

jhj2713 reviewed Apr 2, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3주차] 한규진 미션 제출합니다. #16

[3주차] 한규진 미션 제출합니다. #16

9yujin commented Apr 1, 2022 •

edited

Loading

S-J-Kim left a comment

S-J-Kim Apr 1, 2022

9yujin Apr 2, 2022

S-J-Kim Apr 1, 2022

9yujin Apr 2, 2022

S-J-Kim Apr 2, 2022

S-J-Kim Apr 1, 2022

9yujin Apr 2, 2022

S-J-Kim Apr 1, 2022

9yujin Apr 2, 2022

S-J-Kim Apr 1, 2022 •

edited

Loading

9yujin Apr 2, 2022

jhj2713 left a comment

jhj2713 Apr 2, 2022

jhj2713 Apr 2, 2022

jhj2713 Apr 2, 2022

jhj2713 Apr 2, 2022

jhj2713 Apr 2, 2022

jhj2713 Apr 2, 2022

9yujin Apr 2, 2022


		//handler : any... 방법을 못찾겠습니다.ㅜㅜㅜ
		const useForm = (initialValue: string, handler: any, id: string \| null) => {

		@@ -0,0 +1,53 @@
		// https://github.com/CEOS-Developers/react-todo-15th/pull/11/files#diff-4fd5b35d6c3fbfd63c97f711bd5fe3408b11551836f6cec1ecfb67a498d2843a
		// custom hook에 관한 부분 김선종 멘트님 코드 많이 참고했습니다..!

	background-color: ${(props: IBtn) => (props.done ? '#8989bb' : 'none')};
	background-color: ${({ done }) => (done ? '#8989bb' : 'none')};

[3주차] 한규진 미션 제출합니다. #16

Are you sure you want to change the base?

[3주차] 한규진 미션 제출합니다. #16

Conversation

9yujin commented Apr 1, 2022 • edited Loading

3주차 미션: React-Todo: Refactor!💌

0.

1.

2.

3.

S-J-Kim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S-J-Kim Apr 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhj2713 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9yujin commented Apr 1, 2022 •

edited

Loading

S-J-Kim Apr 1, 2022 •

edited

Loading