MISSION3 / 곽민준 #26
MISSION3 / 곽민준 #26
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
민준님 안녕하세요ㅎㅎ 복잡한 미션 구현하시느라 고생많으셨습니다ㅎㅎ
PR 배달 알림이 slack에 구현되어있는데 알림이 안와서 이번주 미션 안하시는 줄 알고 너무 확인을 늦게 했어요ㅠㅠ 죄송합니다ㅠㅠ
전반적인 코드를 보면서 느낀 점을 말씀드릴게요!
- 반복되는 로직들을 최대한 컴포넌트로 만들어서 관리하려고 하는 모습이 인상깊었어요.
깔끔한 코드는 중복제거에서부터 나오니 항상 지금과 같이 고민하시면서 코드를 구현하시면 좋을 것 같아요! 하나 조언을 드린다면 추상화가 깊어질수록 오히려 코드 파악하는데 더 많은 시간이 소요될 수 있다고 생각해요ㅎㅎ 지금은 한가지 미션이기 때문에 컴포넌트를 활용하는 방향으로 구현하는게 좋을 것 같고, 기획 사항이 많은 대규모 프로젝트에서는 컴포넌트 단위를 조금은 큼직하게 잡는 연습도 해보시면 좋을 것 같아요ㅎㅎ - 커밋 이력이 깔끔해서 보기 좋았어요.
커밋 컨벤션을 지키면서 커밋을 작성하신 것 같아 보기가 편했습니다! 앞으로도 계속 컨벤션을 지키면서 개발해주시면 좋을 것 같아요ㅎㅎ - styled 컴포넌트와 일반 컴포넌트의 이름을 구분해주세요.
코드를 볼 때, styled 컴포넌트와 일반 컴포넌트가 이름에서부터 구분이 되면 로직을 파악할 때 어느 컴포넌트를 타고 들어가야될지가 조금 더 명확하게 보일 것 같아요! - input, form 태그의 기본 기능에 조금 더 집중해주세요.
내부적으로 submit 액션이나 name, value 등 활용할 수 있는 DOM 기본 프로퍼티가 많아요! 활용해가면서 코딩해보시면 조금 더 재밌을 것 같아요ㅎㅎ
아래는 민준님께서 주신 질문에 대해 답변해볼게요.
카드 번호를 입력받는 부분에서 네개의 입력창을 하나로 묶고, 거기에 라벨을 달고싶었으나 코드를 아무리 바꿔봐도 잘 적용되지가 않았는데, 네개의 입력창을 받아오는 부분을 div로 감싸고 그 위에 라벨을 작성하는 방법이 맞을까요?
=> 네개의 입력창을 받아오는 부분은 flex-direction: row인 div로 감싸져있고, label과 입력창을 감싼 div는 flex-direction: column으로 상위에 div가 한번 더 감싸져야할 것 같아요. 아니면 CardRegisterForm 자체를 flex-direction: column로 두면 label과 입력참을 감싼 div를 다시한번 감쌀 필요는 없을 것 같기도 하구요ㅎㅎ 저는 전자를 선호합니다ㅎㅎ
DataInput이라는 컴포넌트에서 전체를 감싸는 div 태그인 InputForm을 작성했는데요, 이처럼 컴포넌트를 만들 때 전체를 감싸는 컴포넌트의 이름을 정할 때 자꾸 해당 컴포넌트 파일 이름과 비슷하게 가져가게 됩니다. Card.jsx에서도 전체를 감싸는 컴포넌트를 CardList라고 지정해주었네요, 이런 과정이 반복되다 보니 해당 코드를 아우르는 이름을 두 번 짓게 되는거같아 어떤 이름을 지정해주어야 할 지에 대한 고민이 생기는데 이런 부분은 어떻게 해결하면 좋을까요?
=> 좋은 고민인 것 같아요ㅎㅎ 위에서도 말씀드렸고 코드에서도 직접 리뷰 달아두었어요ㅎㅎ 간단하게 말씀드리면 styled 컴포넌트에는 접두사로 Styled를 추가해주시는 방법도 괜찮을 것 같아요!
| margin: 0 auto; | ||
| padding: 2rem; | ||
| text-align: center; | ||
| html, |
There was a problem hiding this comment.
사용하지 않는 태그도 포함해서 스타일 속성을 부여한 이유가 궁금합니다!
There was a problem hiding this comment.
CSS초기화를 하고 시작해야할 거 같아서 블로그 게시글을 보고 코드를 붙여넣었는데, 잘못된 방법일까요?!
There was a problem hiding this comment.
궁금해서 여쭤봤어요ㅎㅎ 이런 것도 있네요~ 하나 알아갑니다ㅎㅎ
| <GreyBackground> | ||
| <CardRegisterForm /> | ||
| </GreyBackground> |
There was a problem hiding this comment.
사람은 위에서 아래로 내리면서 보기 때문에 일반 컴포넌트와 styled 컴포넌트에 대한 컴포넌트 명을 구분할 수 있다면 가독성이 더 좋아질 것 같아요ㅎㅎ
There was a problem hiding this comment.
저 역시도 제 코드를 읽을 때, 스타일드 컴포넌트인지 일반 컴포넌트인지 구분하기가 어려웠는데 이름에 차별화를 두면 구분이 훨씬 쉽겠네요!!!
| width: "100%", | ||
| justifyContent: "space-between", | ||
| }} | ||
| > |
There was a problem hiding this comment.
일관성있게 styled 컴포넌트로 구현해주시면 좋을 것 같은데, style을 인라인으로 삽입한 이유가 있을까요??
There was a problem hiding this comment.
저 당시에 제가 왜 저렇게 작성했는지 모르겠네요ㅠㅠ 일관성 있게 작성하도록 하겠습니다!
| import React from "react"; | ||
| import DataInput from "./DataInput"; | ||
|
|
||
| const CardNumberInputs = ({ cardNumbers, handleInputChange }) => { |
There was a problem hiding this comment.
CardRegisterForm에서 DataInput을 바로 사용하지 않고 CardNumberInputs 컴포넌트를 따로생성한 이유가 있을까요??
해당 기능만 있다면 아래와 같이 구현할 수도 있을 것 같아요ㅎㅎ
| const CardNumberInputs = ({ cardNumbers, handleInputChange }) => { | |
| // CardRegisterForm.jsx | |
| <StyledCardNumberInputs> | |
| {cardNumbers.map((number, index) => ( | |
| <DataInput | |
| key={index} | |
| placeholder={"0000"} | |
| value={number} | |
| onChange={handleInputChange(index)} | |
| /> | |
| ))} | |
| </StyledCardNumberInputs> | |
| const StyledCardNumberInputs = styled.dev` | |
| width: 100%; | |
| display: flex; | |
| justify-content: space-between; | |
| ` |
There was a problem hiding this comment.
다른 부분과 카드 입력하는 부분이 뭔가 다르게 구성되어져있다고 생각해서 따로 만들었는데, 이런식으로 하면 컴포넌트를 추가로 만들 필요 없이 재사용성을 높일 수 있겠네요! 이렇게 컴포넌트 하나로만 해보고싶었는데 어떻게 해야 할 지 막막했는데 알려주셔서 감사합니다! 다시 만들어보도록 하겠습니다
| function DataInput({ type, placeholder, InputData, value, onChange }) { | ||
| return ( | ||
| <InputForm> | ||
| <label>{InputData} </label> |
There was a problem hiding this comment.
InputData만 UpperCamelCase로 구현한 이유가 궁금해요ㅎㅎ
그리고 inputData는 목적이 뚜렷해보이지 않아서 subTitle이나 다른 명시적인 변수명으로 대체해도 좋을 것 같아요ㅎㅎ
마지막으로 CardNumberInputs 에서 사용하는 DataInput의 경우, label이 따로 없기 때문에, inputData가 없으면 label테그를 렌더링하지 않는 조건도 추가되면 좋을 것 같습니다!
There was a problem hiding this comment.
실수로 InputData만 UpperCamelCase로 작성 한 것 같은데, 다음에는 이런 부분도 유심하게 살펴봐야겠네요! 확실히 subTitle이라는 이름이 더 어울리는 것 같습니다.
카드 넘버 입력 창에서 라벨 없어야 하는데 확실히 해당 변수가 없을 시에는 렌더링 하지 않도록 하는게 자연스럽겠네요!
| import DataInput from "./DataInput"; | ||
| import React, { useState } from "react"; | ||
| import styled from "styled-components"; | ||
| import Card from "./Card"; | ||
| import CardNumberInputs from "./CardNumberInputs"; |
There was a problem hiding this comment.
외부 라이브러리 import와 내부 파일 import는 각각 묶어주시는게 조금 더 보기 좋을 것 같아요ㅎㅎ
There was a problem hiding this comment.
import React, { useState } from "react";
import styled from "styled-components";
import DataInput from "./DataInput";
import Card from "./Card";
import CardNumberInputs from "./CardNumberInputs";
이런식으로 내부 파일 사용하는 것과 외부 라이브러리 import하는 것을 나누면 훨씬 가독성이 높겠네요! 감사합니다
| {cardNumbers.map((number, index) => ( | ||
| <DataInput | ||
| key={index} | ||
| placeholder={"0000"} |
There was a problem hiding this comment.
react에서는 변수가 아니여도 {} 안에 넣는게 자연스러운걸까요??
There was a problem hiding this comment.
변수나 표현식을 사용해야 할 경우에는 중괄호를 사용해야 하지만, 속성값이 정적인 문자열이라면, 바로 값을 할당하는 것이 좋겠네요!
| import Card from "./Card"; | ||
| import CardNumberInputs from "./CardNumberInputs"; | ||
|
|
||
| const CardRegisterForm = () => { |
There was a problem hiding this comment.
CardRegisterForm에 대해 몇가지 제안드리고 싶은게 있습니다.
- 카드 정보객체를 state로 만들어 관리
카드 정보를 저장하는 목적의 form이기 때문에 카드이름, 번호, cvc 등 카드 정보를 객체로 만들어 관리하는게 해당 컴포넌트의 목적에 좀 더 자연스러운 방향일 것 같습니다. - form, button 태그의 기본 속성에 집중
각 input에 name 속성을 추가하고, onChange 핸들러를 삽입하면, 이벤트로value와name을 가져올 수 있습니다. 이 값으로 1번에서 선언해놓은 카드정보 객체의 값을 업데이트한다면 사용자의 input 액션에 따라 카드 정보를 알맞게 업데이트할 수 있을 것 같아요ㅎㅎ 그리고, form 태그를FormTitle과 같은 레벨에 삽입하고 onSubmit 핸들러를 추가한다음, 완료 button에 submit type을 추가한다면 완료 버튼을 클릭할 때마다 카드 정보객체의 데이터를 Card 컴포넌트에 던져서 저장할 수 있을 것 같아요ㅎㅎ
코드로 표현하면 아래와 같아요. 정확히 구현되는지는 확인을 안해봐서 흐름만 참고 해주시면 좋을 것 같아요ㅎㅎ
// CardRegisterForm.jsx
const CardRegisterForm = () => {
// 카드 정보 객체를 state로 관리
const [cardInfo, setCardInfo] = useState({
name: "",
number: ["", "", "", ""],
expirationDate: "",
cvc: "",
passward: "",
});
// 저장된 카드 정보를 state로 관리
const [savedCardInfo, setSavedCardInfo] = useState([]);
// 사용자가 입력할 때마다 카드 정보 객체 업데이트
const handleInputEvent = (e) => {
const { name, value } = e.target;
setCardInfo((prev) => ({ ...prev, [name]: value }));
};
// 완료 클릭 시 Card 컴포넌트에 카드 정보 전달
const saveCardInfo = (e) => {
// form 기본 액션 새로고침 방지
e.preventDefault();
setSavedCardInfo((prev) => [...prev, cardInfo]);
};
return (
<div>
<FormTitle>카드 정보를 입력해주세요</FormTitle>
<form onSubmit={handleSubmit}>
<DataInput
name="name"
InputData={"카드 이름"}
placeholder={"카드 이름을 입력해주세요"}
value={cardInfo.name}
onChange={handleInputEvent}
/>
<DataInput
type={"text"}
name="expirationDate"
InputData={"유효기간"}
placeholder={"MM/YY"}
value={cardInfo.expirationDate}
onChange={handleInputEvent}
/>
<DataInput
name="cvc"
type={"password"}
InputData={"CVC"}
placeholder={"카드 뒷면 3자리 숫자"}
onChange={handleInputEvent}
value={cardInfo.cvc}
/>
<DataInput
name="password"
type={"password"}
InputData={"카드 비밀번호"}
placeholder={"카드 앞면 2자리 숫자"}
value={cardInfo.password}
onChange={handleInputEvent}
/>
<button type="submit">완료</button>
<Card savedCardInfo={savedCardInfo} />
</form>
</div>
);
};There was a problem hiding this comment.
Input에 name이라는 기본 속성이 있는지도 몰랐고, form도 사실 안써봐서 써 볼 생각 자체를 못해봤던거 같네요 기본 속성에는 어떤게 있는지 찾아보면서 익혀보도록 하겠습니다!
확실히 데이터를 저장해야 하는 미션으로 객체로 저장하는게 올바른 방법인 것 같네요 👍🏻
1. 구현 모습
2. 해결 과정
라벨과 입력창이 동일하게 반복된다고 생각해서 컴포넌트로 분리하고, 재사용 하였습니다.
카드 번호 받는 부분은 기존 DataInput 컴포넌트를 단순히 사용하기에는 4개의 입력창이 필요해서 CardNumberInputs라는 새로운 컴포넌트를 만들어서 적용시켜 주었습니다.
카드 번호를 받는 부분만 상태 관리를 해주었는데, 배열에 빈 문자열로 초기화하고 입력창에 숫자를 적으면 문자열에 업데이트 되는 방식으로 적용하였습니다.
3. To 리뷰어에게
카드 번호를 입력받는 부분에서 네개의 입력창을 하나로 묶고, 거기에 라벨을 달고싶었으나 코드를 아무리 바꿔봐도 잘 적용되지가 않았는데, 네개의 입력창을 받아오는 부분을 div로 감싸고 그 위에 라벨을 작성하는 방법이 맞을까요?
DataInput이라는 컴포넌트에서 전체를 감싸는 div 태그인 InputForm을 작성했는데요, 이처럼 컴포넌트를 만들 때 전체를 감싸는 컴포넌트의 이름을 정할 때 자꾸 해당 컴포넌트 파일 이름과 비슷하게 가져가게 됩니다. Card.jsx에서도 전체를 감싸는 컴포넌트를 CardList라고 지정해주었네요, 이런 과정이 반복되다 보니 해당 코드를 아우르는 이름을 두 번 짓게 되는거같아 어떤 이름을 지정해주어야 할 지에 대한 고민이 생기는데 이런 부분은 어떻게 해결하면 좋을까요?
소중한 시간 내어서 코드 살펴봐주셔서 감사합니다! 시간이 많지 않아 유효성검사나 카드명 받아오는 부분은 적용하지 못했으나 시간 나는대로 적용해보도록 하겠습니다!