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주차] 최민주 미션 제출합니다. #9

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hmuri
Copy link

@hmuri hmuri commented Mar 24, 2023

리액트 거의 처음 만져보는 거라 걱정이 이만저만이 아니었는데, 계속 붙잡고 있다보니 어..? 이게 왜 돌아가지.. 하며 버텨낸 한 주였습니다 ^ㅡ^ 해보고 싶은 게 많았는데, 확실히 아는 게 적다보니 기본적인 것에서 막히고, 공부하고, 분석하고.. 아쉬움이 많이 남네요. 구조적인 것 많이 봐주시면 감사하겠습니다! 지금 다른 분들 어떻게 코드 짰는지 배우고 싶어서 근질근질..

일단 다 구현은 했는데 아 그냥 toggle로 display: none 값 줄 걸 그랬나, 다른 방법 있지 않을까 자꾸 생각이 납니다. 열심히 공부할 결심!! 참고로, 빨간 박스 체크하면 맨 위로 정렬되게 하는 기능을 갑자기 넣고 싶어서 계속 붙들고 있었는데 뭐가 잘 안 돼서... ㅠ 일단 제출하고 추가해보겠습니다.

아, 갑자기 trim 함수랑 저번에 알려주신 스크롤 함수 안 넣은 것도 생각나네요.. 열심히 수정해놓겠습니다

배포 링크 : https://react-todo-17th-hmuri.vercel.app/

  1. Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
    아직 조금은 추상적으로 와닿지만, 운영체제에서 각 사용자들에게 가상의 컴퓨터 머신이 하나씩 부여된다고 여기는 것처럼 리액트의 Virtual-Dom 또한 본래의 Dom에 매번 접근하는 수고 대신 가상의 돔을 만들어 객체를 제어하게 된다. 이렇게 virtual DOM을 사용할 때의 장점은 그 무게가 상당히 가벼워진다는 점인데, 이유는 react가 가상의 돔을 만들어 상태를 비교하고, 변화된 부분만을 찾아내기 때문에 실제 DOM이 전체가 새로고침 되는 것이 아니라 일정 부분만 렌더링하여 운용이 가능하기 때문이다.

  2. 미션을 진행하면서 느낀, React를 사용함으로서 얻을수 있는 장점은 무엇이었나요?
    뭔가 바닐라를 쓸 때는 한 편의 글을 쓰는 느낌이었다. 서론이 이렇고, 본론이 이런 식으로 전개되어 결론적으로는 이러한 페이지가 나왔습니다! 하는 느낌. 그래서 그 논리 구조를 풀어가는 맛이 있는 대신, 상당히 조심스럽고, 논리를 따지며 한 자 한 자 적게 된달까? 반면에 React는 퍼즐 놀이를 하는 기분이었다. 정확히는 중학교 때 했던 스크래치 놀이? 내가 필요한 내 기능들을 Component로 만들고, 그 친구들을 능동적으로 app.js에 가져와 사용하고, 내 마음대로 빈 칸을 생성해놓고 그 안을 뭘로 채울까 고민하며 퍼즐을 맞춰가는 느낌이었다. 퍼즐을 맞춘다는 것은 조각조각 나 있는 부분들로 큰 그림을 완성시킬 수 있다는 것! 게다가 그 퍼즐 퍼즐을 내가 원하는 색으로 칠할 수 있으니까 그 편리함이 이루 말할 수 없다.

  3. React에서 상태란 무엇이고 어떻게 관리할 수 있을까요?
    [a, b] = useState(). b라는 함수를 통해 a라는 변수의 값을 조정해줄 수 있다. a의 상태를 조정해주는 b라는 함수. 약간 a는 음료수 컵 같다. b는 레시피고? 그래서 레시피에 따라서 내가 원하는 음료수를 만들어 그 결과물을 a에 넣어주는 것이다. 보통 b의 이름을 set'a' 로 저장을 하고, 그 때 그 때 b의 함수 내용을 바꾸어서(레시피를 바꾸어서) 다른 방식으로 a의 값을 도출해낼 수 있다.

  4. Styled-Components 사용 후기 (CSS와 비교)
    일단, 한 파일 안에 내가 제어해야 할 박스들과 그 박스들을 꾸며줄 style이 함께 들어있다는 건 상당히 편리하다. 내가 고치고 싶은 스타일이 생겼을 때 어디 함수인지 찾고, 그 파일이 저장된 html과 거기에 해당하는 css를 찾는 두 번의 일을 한 번으로 줄여줄 수 있다. 게다가 스타일이 각 components들에 포함되어 있기 때문에 컴포넌트 파일만 다른 파일에 넘겨주면 css 파일을 따로 연결시켜줄 필요 없이 함께 스타일까지 따라갈 수 있다. 정말 기능이 많은 것 같은데, 폰트 관리할 때도 편하고 특히 media query도 theme.js 사용해서 넣어보고 싶었는데... 다음 과제 때는 지금 기술한 것 이상으로 꼭 더 다양한 기능을 적용시켜보고 싶다.

Copy link
Member

@chaeyeonan chaeyeonan left a comment

Choose a reason for hiding this comment

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

민주님 안녕하세요! 프론트운영진 안채연입니다

우선 이번주 과제 너무 수고많으셨어요! 열심히 고민한 흔적이 잘 보여서 좋았습니다 ㅎㅎ
저도 처음 리액트를 다룰때 너무나도 어려웠던 경험이 있는데 아마 이렇게 열심히 하시다보면 금방 익숙해지실거라고 생각합니다!! (저도 아직은 많이 부족한 ㅜㅜㅎㅎ)

이번주 과제 너무 수고하셨고 스터디 화이팅입니다!

Comment on lines +21 to +23
margin-left: 5%;
margin-top: 3rem;
margin-bottom: 1.5rem;
Copy link
Member

Choose a reason for hiding this comment

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

  1. 각각 따로 지정하고 싶으실 경우 margin: 5px 10px 15px 20px 이렇게 한 줄로 작성하시면 더 좋을 것 같아요! 참고자료 놓고 갑니다
  2. %rem을 혼용해서 쓰셨는데 혹시 이유가 있을까요?

Comment on lines +34 to +56
const TodoTitle = styled. div`
width: 82%;
height: 32px;
color: white;
font-size: 28px;
text-align: left;
font-weight: bold;
margin-left : 15%;
margin-top : 1.5rem;
margin-bottom: 4px;
display: flex;
`
const DoneTitle = styled. div`
width: 82%;
height: 32px;
color: white;
font-size: 28px;
text-align: left;
font-weight: bold;
margin-left : 15%;
margin-top : 1.5rem;
margin-bottom: 4px;
display: flex;
Copy link
Member

Choose a reason for hiding this comment

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

현재 TodoTitleDoneTitle의 스타일이 동일한데 이럴 경우 굳이 컴포넌트를 나누지 않고 하나의 컴포넌트로 사용하는게 코드를 줄일 수 있을 것 같네요!

+) 밑에 있는 컴포넌트들도 중복된게 많아보이는데 하나의 컴포넌트로 사용해주세요!

Comment on lines +58 to +64
const TodoBox = styled.div`
position: relative;
width: 82%;
height: 30%;
margin-left: 8%;
margin-top: 1rem;
`
Copy link
Member

Choose a reason for hiding this comment

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

할 일의 개수가 box 크기보다 많아졌을 경우 처리가 안되어서 밑의 영역까지 넘어가게 되는데, overflow: auto 사용해서 스크롤 적용해주시는게 좋을 것 같아요!

Comment on lines +112 to +117
const onRemove = id => {
setTodos(todos => todos.filter(todo => todo.id !==id))
}
const onRemove_dn = id =>{
setDones(dones => dones.filter(done => done.id !==id))
}
Copy link
Member

Choose a reason for hiding this comment

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

코드가 중복되어 보이는데 하나의 함수로 만들면 어떨까요!

Comment on lines +157 to +177
<body>
<Container>
<Title>T O - D O &ensp;L I S T</Title>
<TodoInput
addTodoList={addTodoList}
/>
<TodoTitle>To-Do ({todos.length})<TodoRldBtn onClick = {() => {setTodos([])}}>🔄</TodoRldBtn></TodoTitle>
<TodoBox>
<TodoList todos={todos}
onRemove={onRemove}
addDoneList={addDoneList}
onChecked = {onChecked}/>
</TodoBox>
<DoneTitle>Done ({dones.length})<DoneRldBtn onClick = {() => {setDones([])}}>🔄</DoneRldBtn></DoneTitle>
<DoneBox>
<DoneList dones={dones}
onRemove_dn={onRemove_dn}
addTodoList={addTodoList}/>
</DoneBox>
</Container>
</body>
Copy link
Member

Choose a reason for hiding this comment

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

혹시 여기 body 태그는 왜 작성하셨는지 이유가 궁금합니다!
App.css에 적용한 css때문이라면 Container에도 같은 속성이 적용되어 있어서 굳이 써주지 않아도 될 것 같아요~

Comment on lines +20 to +41
const RtnBtn = styled.div`
background-color: black;
border: 1.5px solid red;
border-radius: 5%;
height: 23px;
width: 60px;
color : red;
font-size: 19px;
font-weight: medium;
margin-right: 25px;
text-align: center;
`
const DltBtn = styled.div`
background-color: black;
border: none;
border-radius: 5%;
height: 22px;
width: 22px;
color : white;
font-size: 20px;
font-weight: medium;
margin-left : 10px;
Copy link
Member

Choose a reason for hiding this comment

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

RtnBtnDltBtn처럼 완전히 동일한 css 속성은 아니지만 비슷한 속성이 많을 경우 props를 이용하면 효율적으로 코드 작성이 가능합니다!
참고자료 놓고 갑니다

Comment on lines +7 to +9
{dones.map(done =>(
<DoneItem done={done} key = {done.id} onRemove_dn = {onRemove_dn} addTodoList={addTodoList}/>
// 아 뭔가 이렇게 이중으로 안 넘기고 바로 지우는 방법도 있을 것 같은데...
Copy link
Member

Choose a reason for hiding this comment

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

DoneItem의 리스트를 따로 컴포넌트로 만들지 않고 App.js에 바로 map문을 적용하면 한번 더 props로 넘기는 과정을 생략할 수 있을 것 같네요!


}

export default TodoItem;
Copy link
Member

Choose a reason for hiding this comment

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

지금 TodoItemDoneItem의 역할이 동일해보이는데, 이럴경우 예를 들어 Item.js 이라는 하나의 파일 안에 두 개의 컴포넌트로 분리하여 작성하는 방법도 있습니다!
그렇게 되면 공통적으로 쓰이는 styled-component를 재사용함으로써 중복코드를 줄일 수 있고 파일 개수를 줄일 수 있겠죠!

);
};

export default TodoList;
Copy link
Member

Choose a reason for hiding this comment

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

마찬가지로 굳이 TodoList 컴포넌트를 만들지 않고 App.js에 바로 적용하는 방법이 있을 것 같고, 위에서 말씀드린대로 이것도 DoneListTodoList를 하나의 파일로 묶음으로써 조금 더 효율적으로 작성할 수 있을 것 같네요!

Comment on lines +36 to +44
background-color: black;
border: none;
border-radius: 5%;
height: 22px;
width: 22px;
color : white;
font-size: 20px;
font-weight: medium;
margin-left : 10px;
Copy link
Member

Choose a reason for hiding this comment

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

각 버튼에 cursor: pointer 를 추가하면 동작이 좀 더 명확하게 보일 것 같습니다!

@@ -1,8 +1,180 @@
import {React, useState} from 'react'

Choose a reason for hiding this comment

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

저도 이렇게 사용했었는데 React를 적어주지 않아도 되는 걸 이번 코드 리뷰 하면서 처음 알게 됐어요!!
예린님 과제에서 문기님이 코드 리뷰 해주신 부분 참고하시면 도움 되실 것 같아요!!! 코드 리뷰

text,
checked: false
}
setTodos(todos => todos.concat(todo));

Choose a reason for hiding this comment

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

concat 함수보다 push나 ...(spread) 연산자를 사용하시는 것도 좋을 것 같아요!! 이것도 문기님 코드 리뷰 보고 알았습니당!! 참고하시면 좋을 것 같아요💪🏻💪🏻 코드 리뷰

Copy link

@hyosin-Jang hyosin-Jang left a comment

Choose a reason for hiding this comment

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

민주님 안녕하세요~~
완료/복귀라고 워딩 다르게 하신거랑 투두 리스트 내부에서도 체크할 수 있는 기능 두신거 좋았어요!
디자인도 게임화면 같고 재밌네요 ㅎㅎ
이번 주차도 고생하셨습니다 ㅎㅎ 스터디 때 뵐게요! ☺️

const {id, text, checked} = todo;
const addDone = (text, id) =>{
onRemove(id);
return addDoneList(id, text)

Choose a reason for hiding this comment

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

doneList에 추가할 때 addDoneList를 리턴하지 않고 실행만 시켜도 될 것 같아요!

import React from 'react';
import styled from 'styled-components';

const TextBox = styled.div`

Choose a reason for hiding this comment

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

주로 div를 사용하셨는데 TextBox의 경우 h1 같이 의미를 담고 있는 태그들을 사용하는게 가독성이 좋을거 같아요!!

const onRemove = id => {
setTodos(todos => todos.filter(todo => todo.id !==id))
}
const onRemove_dn = id =>{

Choose a reason for hiding this comment

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

함수명에 camelCase와 underscore를 혼용하셨는데 camelCase로 통일하는건 어떨까요~?

addDoneList={addDoneList}
onChecked = {onChecked}/>
</TodoBox>
<DoneTitle>Done ({dones.length})<DoneRldBtn onClick = {() => {setDones([])}}>🔄</DoneRldBtn></DoneTitle>

Choose a reason for hiding this comment

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

버튼 누르면 한번에 초기화시키는 기능 좋네요 ㅎㅎ

Copy link

@paya17 paya17 left a comment

Choose a reason for hiding this comment

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

안녕하세요 민주님!
투두리스트에 다양한 기능들이 있어서 좋았어요~!!
리액트 거의 처음 다뤄보셨다고 하셨는데 잘 구현하신 것 같아요!👍
수고 많으셨고, 다음 과제도 같이 화이팅해봐요~!💪💪

text,
checked: false
}
setTodos(todos => todos.concat(todo));
Copy link

Choose a reason for hiding this comment

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

예지님 말씀처럼 spread(...) 연산자를 사용하는 게 좋을 것 같아요!

Suggested change
setTodos(todos => todos.concat(todo));
setTodos([...todos,todo]);

margin: 0;
`
const DoneBox = styled.div`
position: relative;
Copy link

Choose a reason for hiding this comment

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

position속성도 좋지만 flex-box도 많이 활용해보셨으면 좋겠어요! 정말 편리한 아이랍니다ㅎㅎ

Comment on lines +9 to +11
border-style: solid;
border-width: 1px;
border-color: white;
Copy link

Choose a reason for hiding this comment

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

이렇게 한 줄로 적을 수 있어요!

Suggested change
border-style: solid;
border-width: 1px;
border-color: white;
border: 1px solid white;

Comment on lines +3 to +4
import {MdCheckBoxOutlineBlank} from "react-icons/md";
import {BsSquareFill} from "react-icons/bs";
Copy link

Choose a reason for hiding this comment

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

react-icons 저도 나중에 사용해보고 싶어요..!

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