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단계 - 행운의 로또미션] 하루(김하루) 미션 제출합니다. #58

Merged
merged 42 commits into from
May 15, 2021

Conversation

365kim
Copy link

@365kim 365kim commented May 1, 2021

Jbee님, 안녕하세요!

또다시 오랜만에 (...) 인사드리는 🙋🏻‍♀️ 하루(@365kim) 입니다.
페이먼츠 미션이 시작되면서 로또 2단계 제출 타이밍을 놓쳤습니다,,, 죄송합니다,,, 🥲

주말에 집중 빡🥊 해서 함수컴포넌트로 마이그레이션 해보았습니다!
훅을 사용하지 않아도 되는 shared 컴포넌트 부터 우선 함수 컴포넌트로 변경한 후, containers 컴포넌트를 변경하면서 훅을 적용해 보았습니다.


✅ 구현 기능 목록

  • Step1의 클래스 컴포넌트를 함수 컴포넌트로 마이그레이션 한다.

💪 새롭게 시도한 부분

  • Proptypes를 명시했습니다.
  • 린터 예외 규칙(rules)을 모두 삭제하였습니다.
    • 예외처리를 해야할 경우 전역 스코프가 아닌 라인 또는 파일 스코프에 예외처리 되도록 했습니다.
    • 앞으로는 린터 규칙을 제대로 확인하지 않고 급하다는 핑계로 rules에 무작정 추가하는 안 좋은 버릇을 고치고, 예외 처리할 경우 명확하게 어떤 규칙인지 이해하고 넘어가려고 합니다!

💪 UX/UI고려사항

  • 모달에서 Dimmed 영역 클릭으로 모달을 닫을 수 있도록 했습니다.


아직 많이 부족하지만 리뷰어님의 귀중한 조언 덕분에 나날이 성장하고 있습니다!!
시간내서 살펴봐 주셔서 정말 정말 감사드립니다,,,!

행복한 5월의 시작되시길 바라요 😆😆

@365kim
Copy link
Author

365kim commented May 1, 2021

학습로그

리액트(React)

태그

  • 리액트, JSX, 컴포넌트, props, state, lifecycle, 조건부 렌더링, key, 비제어 컴포넌트, 제어 컴포넌트, 상태 리프팅

Copy link

@JaeYeopHan JaeYeopHan left a comment

Choose a reason for hiding this comment

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

하루님 수고하셨어요!
굵직한 코멘트가 있는데 같이 고민해보면 좋을 것 같아요~ 🤔

const [isSubmitButtonDisabled, setIsSubmitButtonDisabled] = useState(
initialState.isSubmitButtonDisabled,
);
const paymentInput = createRef();

Choose a reason for hiding this comment

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

createRefuseRef의 차이점을 살펴보면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

  • createRef: 클래스 컴포넌트에서 사용 (리액트 16.3버전에 추가)
  • useRef: 함수 컴포넌트에서 사용 (리액트 16.8버전에 추가)

함수 컴포넌트에서 createRef를 사용하려고 하면 사용할 수는 있지만(?) hook이 아니라서 상태를 가질 수 없다. 즉 컴포넌트(함수)가 다시 호출될 때마다 초기화된다.

클래스 컴포넌트에서 함수 컴포넌트로 마이그레이션 하면서 간과했던 부분이네요...! 짚어주셔서 감사합니다 😆

자동포커스랑 reset할 때만 사용해서 PurchaseForm이 다시 호출될 때 초기화 되어도 운좋게 문제없어 보였던 것 같은데 함수컴포넌트에서 createRef를 쓰는 레퍼런스도 보지 못했고 API의 의도와도 다르기 때문에 useRef로 수정했습니다..!

📌 관련커밋: 22ea77b

Comment on lines 18 to 23
Button.propTypes = {
className: PropTypes.string,
children: PropTypes.string,
type: PropTypes.string,
onClick: PropTypes.func,
};

Choose a reason for hiding this comment

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

제가 prop-types를 안 써봐서 모르는데 Button Element의 attribute를 확장할 수는 없나요?
만약 이 Button 컴포넌트에 disabled라는 props를 전달하고 싶으면 이 Button 컴포넌트 구현도 수정되어야 할 것 같아서요!

Copy link
Author

@365kim 365kim May 14, 2021

Choose a reason for hiding this comment

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

찾아보니 proptypes Github Repo에 관련 제안(Add prop types for react html elements #285)은 있었는데 HTML에서 유효한 attributes가 맨날 변해서 유지하기 힘들다고 지원할 계획이 없는 것 같슴니다...🥺 확장하는 방법을 찾지 못해 MDN 문서에 나와있는 button 요소의 attributes를 모두 명시해보았습니다...!

+ TMI 버튼 요소의 attributes를 확장한다는 말씀이 어떤 뜻인지 이해 못하다가 수업시간중 다른 크루의 타입스크립트 코드를 보고 이해할 수 있게 되었습니다... 타입스크립트에 도전할 날이 다가오고 있는 듯하네욤,,,!! ㅎㅎㅎ

📌 관련커밋: 31a3686

Button.defaultProps = {
className: '',
children: '',
type: 'button',

Choose a reason for hiding this comment

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

사용자 입장에서 button html element의 default type은 submit인데 default value를 type으로 바꾸면 실수하거나 혼동스러울 여지가 있진 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

컴포넌트 사용자는 당연히 HTML 태그의 기본값을 떠올릴텐데 그 생각을 미처 못했습니다...! 앞으로는 컴포넌트를 사용하는 입장에서 어떤 defaultProps를 지정하는게 자연스러운지 한번 더 고민해보고 기본값을 지정하도록 하겠습니다. 섬세한 피드백 감사드립니다 😆 🙏

button의 기본 type인 submitdefaultProps으로 지정하고 더불어 의미없는 defaultProps도 함께 정리하였습니다...!

📌 관련커밋: 003bea7, 31a3686


const cx = classNames.bind(styles);

export default function Title({ as, size, children }) {

Choose a reason for hiding this comment

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

as

Choose a reason for hiding this comment

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

classname도 props로 받을 수 있게 해서 외부에서 style custom이 가능하도록 해주면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

어느 컴포넌트에서는 className을 받고 있고, 어느 컴포넌트에서는 또 안 받고 있고 하는 일관성 없는 코드의 모습이 피드백을 받고 나서야 보였습니다...! 모든 컴포넌트에서도 위와 같은 모습으로 props를 전달받도록 변경해보았습니다...! 좋은 피드백 감사드립니다 🙏

const { className, children, ...rest } = props 

📌 관련커밋: 003bea7

</div>
);
}
export default function ToggleButton({ onChange, children }) {

Choose a reason for hiding this comment

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

이 ToggleButton는 어떻게 사용하길 기대하고 디자인하신건가요?

  1. Uncontrolled로 사용하길 기대했다면 ref를 받을 수 있게 끔 해당 함수 컴포넌트를 forwardRef로 export 해줘야 할 것 같습니다.
  2. controlled로 사용하길 기대했다면 value 값을 받을 수 있도록 props에 정의해줘야 할 것 같습니다!

Choose a reason for hiding this comment

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

이 컴포넌트의 주인공은 input element인 것 같은데 input element에 넘길 수 있는 attribute를 그대로 props에서 넘길 수 있어야 이 ToggleButton의 재사용성이 높아지지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

말씀해주신 대로 다양한 input attributes를 전달할 수 있도록 최상위 태그에는 containerClassname을 따로 받고, 주인공인 input...rest를 spread해서 전달해주도록 변경했습니다.

이 부분을 수정하면서, 모든 컴포넌트에서 같은 모습으로 props를 전달 받고 싶어서 다른 컴포넌트에서도 const { className, children, ...rest } = props 와 같은 모습으로 props를 전달받도록 함께 변경하였습니다.

📌 관련커밋: 003bea7


당시 input을 만들면서도 제어/비제어 어느 쪽으로 만들겠다는 확고한 방향성 없이 작성했었습니다.🥲 앞으로는 제가 작성하는 컴포넌트가 제어로 만들어야 하는 상황인지 비제어로 충분한 상황인지 생각해서 의도한 방향으로 작성하겠습니다...!

ref를 쓸 일이 없어보여서 controlled로 작성했습니다. 말씀해주신 value 값이 토글버튼에서의 checked 값일 것이라고 생각하고 코드를 수정했는데, 맞는지 궁금합니다...! 🙏

📌 관련커밋: 8ec2dc9

Choose a reason for hiding this comment

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

말씀해주신 value 값이 토글버튼에서의 checked 값일 것이라고 생각하고 코드를 수정했는데, 맞는지 궁금합니다...!

맞습니다 :)

Title.propTypes = {
as: PropTypes.string,
size: PropTypes.string,
children: PropTypes.any,

Choose a reason for hiding this comment

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

PropTypes.element 살펴보니 이런 타입이 있던데 👀

Copy link
Author

Choose a reason for hiding this comment

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

... 😭 ....반성합니다... 콘솔 에러를 빨리 피하려고 제대로 알아보지 않고 any라고 적었습니다...

에디터에서 PropTypes.any 로 검색해서 몇몇 children에 any로 들어가있는 것을 확인했습니다. array를 포함해서 '렌더링 될 수 있는 무언가'를 지정할 때 쓰는 PropTypes.node로 변경해두었습니다...! (참고한 자료: 리액트 proptypes)

📌 관련커밋: 8f65097

<div className="UserResult__inner">
<XButton onClick={onCloseUserResult} />
<Title>당첨결과</Title>
<ResultTable lottoBundle={lottoBundle} winningNumber={winningNumber} />

Choose a reason for hiding this comment

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

이름은 ResultTable인데 받고 있는 props는 lottoBundle, winningNumber입니다.
lotto와는 상관없는 네이밍의 컴포넌트 props에서 lotto관련된 props를 받고 있어서 살펴보게 되었어요.

이 LINE을 보고 일단 제가 들었던 생각은 이 컴포넌트는 추상화되지 않은 컴포넌트이고, 그냥 추출한 컴포넌트라고 생각이 됩니다. 추출된 컴포넌트는 괜히 props만 드릴링 시킵니다.

아래 flow를 같이 생각해보면 좋을 것 같습니다 ㅎㅎ

  1. 분리가 필요한 컴포넌트인가?
  2. 로또 결과가 아닌 다른 무언가의 결과가 렌더링 될 필요가 있을 때, 이 컴포넌트를 쓴다면?
  3. 이름을 바꾸면서 합성가능하도록 로또 결과를 렌더링하는 컴포넌트를 역할 기준으로 추상화할 수는 없을까?

Copy link
Author

Choose a reason for hiding this comment

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

말씀해주신 내용 참고해서 두 가지 방향으로 수정해보았습니다.

[1] 공통 Table 컴포넌트를 만들어서 다른 table을 만들 때도 재사용할 수 있도록 변경했습니다.
📌 관련커밋: 6de4540

<Tr>, <Th>, <Th>컴포넌트를 만들었다가 지웠다가, <Thead>, <Tbody>컴포넌트를 만들었다가 지웠다가,,, 하다가 지금의 Table 컴포넌트의 모습이 나왔습니다. 컴포넌트를 사용하는 입장에서 편하게 children에 내용만 쏙 넣어서 map을 바로바로 쓸 수 있도록 하면서도 <Thead><Tbody>가 서로 대칭적인(?) 구조를 유지하도록 만들고 싶었는데 그렇게 된지는 잘 모르겠습니다...🥲


[2] 별도의 모듈에 있던 ResultTable를 index.js로 다시 합치고 UserResultTable 로 네이밍 변경했습니다.
📌 관련커밋: 33e0d03

말씀해주신 것처럼 props-driling이 생기고 있어서 index.js의 메인 컴포넌트인 UserResult컴포넌트에 UserResultTable을 합쳐보았는데, UserResult가 이미 변수를 많이 갖고 있고 UserResultTable 또한 head에 들어갈 값, body에 들어갈 값 등등 복잡한(?) 변수가 많아서 누가 어디에 사용되는지 알아보기 어렵다고 판단되었습니다.

props-drilling이 생기더라도 변수 뭉치들을 따로보는 것에 이점이 있겠다 싶어서 UserResultTableUserResult와 같은 모듈 내에 위치시키되, 그 내부가 아닌 바로 아래에 위치시키게 되었습니다....!

Choose a reason for hiding this comment

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

Table 잘 만들었네요! 👍

export const PurchaseForm = (props) => {
const { setLottoBundle, shouldReset, finishReset } = props;
const [validationMessage, setValidationMessage] = useState(initialState.validationMessage);
const [isInputDisabled, setIsInputDisabled] = useState(initialState.isInputDisabled);

Choose a reason for hiding this comment

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

  1. input 컴포넌트를 분리했다면 이 state는 input만 알고 있으면 되는 state 아닐까요?
  2. onChange, reset 에 따라서 이 state를 명령형으로 true/false toggle하는 것이 아닌 input에서 다루고 있는 value 값에 따라 disabled 를 제어하도록 (선언적으로) 코드를 작성해보면 어떨까요?
  3. 이렇게 하면 무엇이 좋을까요?

Copy link
Author

Choose a reason for hiding this comment

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

이번에 전해주신 피드백 중에 개인적으로 요 피드백이 제일 어려웠습니다,,,🏋🏻‍♀️🏋🏻‍♀️🏋🏻‍♀️

어려웠던 이유는 저에게 명령형(imperative)와 대비되는 선언적(declarative)라는 개념이 이해하기 어려워서 였습니다..!
명령형은 어떻게(how)를, 선언형은 무엇(what)을 작성한다고 하는데, 제게는 for문으로 작성한 코드, array.map()으로 작성한 코드 모두 어떻게(how)로 보여서 (띠로리...) 이해에 진전이 없었습니다 🥲

그래서 우선 이해를 못한 채로, 말씀해주신 방향만 참고해서 코드를 수정해보았습니다...!

/* 변경 전 */
setIsInputDisabled(true); // 입력창을 비활성화한다.
setIsSubmitButtonDisabled(true); // 제출버튼을 비활성화한다.

/* 변경 후 */
setInputStatus((prevState) => ({ ...prevState, isSubmitted: true })); // 입력창을 제출상태로 변경한다.

수정해보니 코드가 간결해지는 것을 발견할 수 있었습니다.

각 요소를 어떻게 렌더할지를 각각의 상태로 정의한다면, 사용자가 클릭할 수 있는 n개의 버튼이 있는 경우 n개의 상태를 만들어 대응해야 합니다. 그렇게 하지 않고 input이 제출되었는지만 상태로 만들면 하나의 상태값을 가공하더라도 n개의 요소 대응이 가능해집니다.

생각이 여기까지 미치고 나니, 리액트에서 얘기하는 다른 state나 props로 계산 가능하면 state가 아니다!라는 원칙을 어겼던 것이 아닌가하는 생각이 들었습니다… 그리고 이게 의도하신 수정 방향이 맞는지도 더욱 아리송해졌습니다... 🙃

그새 2주일 남짓의 시간이 지났는데 여전히 명령형, 선언형의 개념은 잘 모르겠습니다…🥲 선언적으로 작성했다는 것은 한번 랩핑해서 무언가 추상화했다(…?) 정도의 느낌으로 받아들이고 있습니다. 개념을 조금 더 다지기 위해서 어떤 키워드로 접근해보면 좋을지 자그마한 힌트 주시면 매우매우 감사하겠습니닷,,,!! 🙏

📌 관련커밋: 3852659

Choose a reason for hiding this comment

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

저희 회사 동료가 발표한 내용인데 요거 보면 좀 이해가 잘 되려나 싶네요?ㅎㅎ
-> https://toss.im/slash-21/sessions/3-3

Copy link
Author

Choose a reason for hiding this comment

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

오오 이 영상 행사 당시에 보면서, 정말 쉽게 설명해주시는데도 제가 핵심내용을 잘 이해하지 못해서 끙끙댔던 기억이 납니다,,,🥲 그래도 지금 다시보면 그때보다는 더 잘 이해하지 않을까 싶습니다! 잘 돌려보겠습니당 ㅎㅎ 좋은 영상 감사합니다 👍

const cx = classNames.bind(styles);

export default function Title({ as, size, children }) {
const TitleClass = cx('Title', { [`Title--${size}`]: Boolean(size) });

Choose a reason for hiding this comment

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

애초에 boolean이 아닌 값을 type casting해서 사용해야만 하는 경우였나요?
그게 아니면 type casting 대신 상황에 맞는 표현식을 써주면 어떨까요?

size가 0일 경우는 없겠지만 이런 케이스에 대해 오류가 발생할 수 있을 것 같네요

Copy link
Author

@365kim 365kim May 14, 2021

Choose a reason for hiding this comment

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

처음 코드를 작성할 때 size 라고 쓰는 것보다 Boolean(size)라고 쓰는 것이 더 명시적이니까 당연히 좋겠지 (...) 🙄 라는 생각으로 코드를 작성했는데, undefined 외의도 0과 falsy 값이 들어올 수 있다는 생각을 못했었네요...!

말씀해주신 '상황에 맞는 표현식' 이 어떤 것인지 잘 모르겠어서...👉👈 우선은 불필요해보이는 Boolean 캐스팅을 없애고 size에 받을 수 있는 값을 propTypes를 `oneOf 지정해두어서, 값이 들어오지 않으면 해당 스타일이 생략되고, 이상한 값이 들어오면 에러가 뜨도록 수정해보았습니다!

/* 변경 전 */
  const classnames = cx('Title', { [`Title--${size}`]: Boolean(size) });
  Title.propTypes = {
    size: PropTypes.string,
  }

/* 변경 후 */
  const classnames = cx('Title', { [`Title--${size}`]: size });
  Title.propTypes = {
    size: PropTypes.oneOf(['small', 'medium', 'large']),
  }

📌 관련커밋: 274c583, 05a19a4

Choose a reason for hiding this comment

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

상황에 맞는 표현식

제 의도는 !array,length 보다 array.length === 0 이렇게 boolean 값을 전달해야할 경우엔 boolean임을 드러내는 것이 좋지 않을까? 였어요 ㅎㅎ


oneOf라는 타입도 있네요!ㅎㅎ 좋습니다. typescript에서는 union type으로 정의하거나 (with template literal type) enum으로 정의해서 쓰곤 합니다!

Copy link
Author

Choose a reason for hiding this comment

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

아하 설명해주셔서 감사합니다 💙 레벨2 마지막 미션때 TypeScript에 도전해볼까 하는데 말씀해주신 키워드 잘 기억해두었다가 써먹어 보겠습니다...! 메모 메모...📝

Comment on lines 19 to 22
const [isLoading, setIsLoading] = useState(initialState.isLoading);
const removeLoader = () => {
setIsLoading(false);
};

Choose a reason for hiding this comment

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

이제 hooks 만드는 연습도 슬슬 시작해보시죠!ㅎㅎ

export function useToggle(initialValue = false) {
  return useReducer(prev => !prev, initialValue);
}

Copy link
Author

Choose a reason for hiding this comment

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

추천해주신 토글 포함해서 연습삼아 🍒 useToggleButton, 🍒 useLoading, 🍒 useModal 작성해보았습니다!📝

컴포넌트 또는 상태와 연관된 함수를 별도의 모듈로 쏙 뽑아낼 수 있어서 코드가 깔끔해진다는 느낌을 받았습니다. 커스텀훅을 왜 만드는지 알것 같은 시간이었습니다 😆

추천해주신 useReducer는 여러 action을 정의해주어야할 때 더욱 유용해보여서 3개 이상의 action을 갖는 🍒 useModal에 적용해보고, 🍒 useToggleButtonuseState로 만들어봤습니다. useState를 사용하는 편이 actionTypereducer를 따로 정의하지 않아도 되어서 더 간편(?)하다고 생각했습니다...!

작성한 커스텀훅 중 🍒 useModal는 딱 떨어지는 사용성을 내지 못한 것 같아 아쉬움이 남습니다... 여기저기 산재되어 선언하고 있던 여러 상태를 정리할 수 있기는 했지만, 일부는 App.js에서, 일부는 App.js의 자식인 UserResult.js에서 사용해야 해서 괜히 props-drilling이 생기는 느낌도 들고... 아예 앱 구조를 잘못 잡은건 아닐까 하는 생각도 들다가... UserResult에 담긴 모든 코드를 App으로 들고 올라갈 순 없어서(...) 그대로 두었습니다.🥲

커스텀훅 입문단계에서 혹시 더 알아가면 좋을 점이 있다면 피드백 주시면 감사하겠습니다! 🙏


📌 관련커밋: 🍒 useToggleButton(26ba38f), 🍒 useLoading(ed23350), 🍒 useModal(d835955)

Choose a reason for hiding this comment

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

컴포넌트 또는 상태와 연관된 함수를 별도의 모듈로 쏙 뽑아낼 수 있어서 코드가 깔끔해진다는 느낌을 받았습니다.

정말 좋습니다! 어떤 기준으로 hooks를 만들지도 꾸준히 고민해보면 좋을 것 같아요.

useToggleButton

이 hooks가 컴포넌트를 내포하는 순간 UI와 의존성이 생기게 되고 Checkbox에서도 사용할 수 있던 useToggle이 사용할 수 없게 되었습니다 ㅎㅎ

useLoading

이름만 loading이고 하는 역할은 false -> true 뿐인 것 같은데, loading이라는 context를 알아야할까? 의문이 드네요!ㅎㅎ

useModal

아마 잘 만들기 어려울거라 생각해요 ㅎㅎ 이 hooks도 모달(또는 다른 컴포넌트)를 열고 닫는 것과 Modal 컴포넌트와의 의존성이 있어서 재사용, 확장성 측면에서 아쉬움이 있을 것 같아요 ㅎㅎ


다른 잘 만들어진 Hooks를 먼저 살펴봐도 좋을 것 같네요! 검색해보면 많이 나와요! react-use 요런거라던가 react-spectrum 같은 오픈소스를 살펴보세요!

Copy link
Author

Choose a reason for hiding this comment

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

앗 useLoading을 따로 빼서 유틸함수처럼 필요할 때마다 손쉽게 사용가능해졌다고 생각했는데 과한 코딩이었나봅니다 🥲 꼼꼼히 봐주시고 피드백 주셔서 감사합니다!

hook 안에 컴포넌트를 고대로 넣는 것이 좋은 패턴이 아니군요...! 그리고 hook을 작성할 때 여러 컴포넌트에 적용시킬 수 있도록 만드는 측면을 생각해보지 못했네요...! hook도 컴포넌트처럼 말랑말랑하게 만들도록 노력해보겠습니다. 좋은 피드백 감사합니다! 💙

react-use 요기는 거의 사전이네요...!! 좋은 리소스 소개해주셔서 감사합니다! 잘 참고해서 앞으로 더 잘 작성해보도록 하겠습니다. 😆

@365kim
Copy link
Author

365kim commented May 9, 2021

제이비님 안녕하세요!

제이비님께서 귀중한 🍯피드백🍯 을 보내주신지 벌써 일주일이 지났네요...
오늘 다른 크루분들 PR 머지해주신 코멘트 보고 댓글이라도 남겨봅니다,,,

사아실... 피드백해주신 내용을 정리만 조금 해두고, 아직 코드에 반영을 못했습니다,,,, 🥲
더 슬픈 소식은 아주 높은 가능성으로(...) 방학 때(6월 초) 되서야 코드반영을 할 수 있지 않을까 싶습니다.....😭

제가 기간 내 미션을 완수하지 못해서 코치님들께 여쭤봤을 때, 지난 미션을 완료하지 못했더라도, 새로 시작한 미션을 최우선으로 진행하는게 좋겠다라는 가이드라인을 받았습니다. 그래서 새로 시작한 미션을 호다닥 끝내고 리액트 로또 미션을 다듬고 싶은데, 마음과 다르게 이후 미션들 마저도 밀리고 있습니다... 🤦🏻‍♀️ 현실적으로 방학이 시작하는 6월 5일부터 로또 미션에 집중하는 시간을 가질 수 있을 것 같습니다,,,

미션 일정관리도, 프로그래밍 역량도 많이 부족한 것 같아서 부끄럽습니다...
말씀해주신 내용을 다 소화하고 다음 단계로 넘어갔으면 좋았겠다는 아쉬움도 자꾸...자꾸... 남네요 🥲
호오오옥시나 앞으로의 미션이 기한 내에 마무리가 되거나, 이전 미션에 집중할 수 있는 틈이 생기면 바로 피드백 반영해서 올리겠습니당!! 💪

꼼꼼한 피드백 남겨주셔서 다시 한 번 감사드립니다! 🙏

@365kim
Copy link
Author

365kim commented May 15, 2021

📌 리뷰반영 요약표

번호 리뷰내용 관련코멘트 반영커밋
1 컴포넌트의 '추상화 vs 단순 추출'을 구분하고, 필요한 부분 추상화하기 !
👉 재사용 가능한 Table 추상화
바로가기 6de4540 33e0d03
2 더 선언적(declarative)으로 작성하기 !
👉 각 요소의 disabled가 input 의 상태에 따라 제어되도록 변경
바로가기 3852659
3 custom hook으로 작성해보기 !
👉 useToggleButton useLoading useModal 커스텀훅 추가
바로가기 26ba38f ed23350 d835955
4 createRef vs useRef !
👉 클래스 컴포넌트의 createRef 대신 함수 컴포넌트의 훅인 useRef 사용
바로가기 e2f6bb1
5 prop-types 잘 활용하기 !
👉 html 요소 attributes 추가 명시, any 대신 유의미한 타입 사용
바로가기1 2 31a3686 8f65097
6 더 유연한 컴포넌트 만들기 !
👉 props로 className 및 input의 attributes 내려받을 수 있도록 변경
바로가기 003bea7
7 controlled vs uncontrolled 의도에 맞는 컴포넌트로 작성하기 !
👉 ToggleButton을 controlled로 작성
바로가기 8ec2dc9
8 상황에 맞는 표현식 사용하기 !
👉 불필요한 Boolean 캐스팅 삭제
바로가기 274c583 05a19a4

Copy link

@JaeYeopHan JaeYeopHan left a comment

Choose a reason for hiding this comment

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

hooks만드는건 꾸준히 연습해봐요!
질문에 조금씩 코멘트해뒀어요!

@JaeYeopHan JaeYeopHan merged commit ee2a97f into woowacourse:365kim May 15, 2021
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.

2 participants