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

Beginner final assignment #5

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

Beginner final assignment #5

wants to merge 15 commits into from

Conversation

ocahs9
Copy link
Contributor

@ocahs9 ocahs9 commented Jun 6, 2024

✨ 구현 기능 명세

그.. 뭐야 랜덤 시작할 때 숫자 변하는거에 애니메이션 주는 것만 제외하고 전부 구현했습니다 !


📌 내가 새로 알게 된 부분

주석 처리 해두었습니다! 엄청나게 새로운 것보다는 그냥.. 까먹은거나 헷갈렸던 것들 새로 잡는 것들이 많았어요


💎 구현과정에서의 고민과정(어려웠던 부분) 공유!

그냥.. 하나 하나 하느라 오래 걸렸을 뿐.....
네.. 코드도 지저분합니다

🥺 소요 시간

  • 12h

🌈 구현 결과물

2024-06-06.23-09-04.mp4

@ocahs9 ocahs9 requested a review from ljh0608 June 6, 2024 16:50
@ocahs9 ocahs9 self-assigned this Jun 6, 2024
@yarimu
Copy link

yarimu commented Jun 7, 2024

이 서비스... 이용해보고 싶네요...

Copy link
Contributor

@ljh0608 ljh0608 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 1 to 3
# React + TypeScript + Vite

This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules.
Copy link
Contributor

Choose a reason for hiding this comment

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

서비스와 관계없는 리드미는 삭제하거나 수정해도 좋을 것 같습니다~

personality: "passion",
isMorning: false,
isJ: false,
isSkil: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

isSkill 오타 발견 Skrr~

Comment on lines +30 to +38
<Body
isInit={init}
setInit={setInit}
setShowInitBtn={setShowInitBtn}
setChooseObj={setChooseObj}
setStep={setStep}
step={step}
chooseObj={chooseObj}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

setState함수를 props로 그대로 넘기게되면 비동기 처리 과정중 알 수 없는 에러가 날 수 있고 유지보수가 어려워질 수 있습니다. 그 부분 고려해서 setState props를 함수로 감싸서 넘겨주는걸 고려해보세요!

링크-
https://cluster-taek.tistory.com/entry/Props%EC%97%90-%EC%A7%81%EC%A0%91-setState%EB%A5%BC-%EB%84%98%EA%B8%B0%EC%A7%80-%EC%95%8A%EB%8A%94-%EC%9D%B4%EC%9C%A0

Comment on lines 15 to 16
setInit: (isInit: boolean) => void;
setChooseObj: (callBackFunc: (prevObj: Array<any>) => Array<any>) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

setState함수를 props로 넘겼을 때의 타입은

Dispatch<SetStateAction<type>>

입니다. 따라서 setInit의 타입은

Dispatch<SetStateAction<boolean>>

이겠죵??

https://velog.io/@hamjw0122/TS-SetStateAction

Comment on lines +48 to +50
case -2:
return <RandomStep setStep={setStep} />;
case -1:
Copy link
Contributor

Choose a reason for hiding this comment

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

step을 음수로 가는건 보통 예외처리로 오해할 가능성이 있어서 양수로 사용하는 것이 좋을 것 같습니다.
컴포넌트 명도 RandomSelect가 조금 더 의미에 가깝겠네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그럼 그냥 const Step = {
first : -2
second : -1
...
}
와 같은 식으로 작성하는 게 나을까요? 고민을 많이 해봤는데, 0부터 단계를 시작시킨다고 해도 뒤로가기나 그런 것을 고려했을 때 -1이 있는게 더 나은 것 같기도 해서.. -값을 없애면 어떻게 구성해야할지 아직은 잘 안 떠오르네요 하하

Copy link
Contributor

Choose a reason for hiding this comment

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

지금 제시해주신 방법도 first에 -2값이 들어가면서 코드를 이해하기 어려운 부분이 있습니다. 정말 맨 처음 컴포넌트를 0으로 시작한다면 -1을 고려하지 않아도 될까요?

사실 이런 부분은 case를 1,2,3 도 좋지만 각자의 의미를 담은 step string 변수를 사용하는 것이 가장 좋아보여요! 단순히 1,2,3은 의미있는 케이스 분류는 아니니까요! 특히 랜덤추천과 취향대로 추천은 같은 위계의 컴포넌트인데 -1, 0으로 분류하면서 컴포넌트 흐름을 파악하기 어려워 보입니다!

<ArticleHeader>당신이 원하는 성격은?! {`${step}/4`}</ArticleHeader>
<RowFlex>
<ArticleBody
$selected={selected === "kind"}
Copy link
Contributor

Choose a reason for hiding this comment

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

요런 부분은 아주 잘 구현해주셨군요!
근데 요것두! ArticleBody가 반복되고 있네요

그렇다면? 이렇게 상수파일을 선언해주고

  const PERSONAL_TYPE = [
    {
      name: "재미",
      value: "fun",
    },
    {
      name: "열정",
      value: "passion",
    },
    {
      name: "친절",
      value: "kind",
    },
  ];

아래와 같이 구현해볼 수 있겠네요! 기능은 같은데 부여하는 값만 다르다면 컴포넌트화 해서 재사용하는 방향을 생각한다면.. 준혁이는 정말 얼마나 더 잘해질지..

    {PERSONAL_TYPE.map((item) => (
        <ArticleBody
          $width={"25%"}
          $selected={selected === item.value}
          onClick={handleClickPersonal}
        />
      ))}


interface HeaderProptTypes {
showInitBtn: boolean;
handleClickInitBtn: React.MouseEventHandler<HTMLButtonElement>;
Copy link
Contributor

Choose a reason for hiding this comment

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

리액트 18부터 React. 안해도 됩니다!
그리고 MouseEventHandler를 원래 잘 사용하지 않았어서 좀 찾아봤는데 좋은 아티클이 있네요??
공유합니다~
https://www.howdy-mj.me/react/event-handling-in-react

}

const RandomStep = ({ setStep }: RandomStepPropTypes) => {
const randomId = Math.floor(Math.random() * 24);
Copy link
Contributor

Choose a reason for hiding this comment

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

24가 아닌 webber.length로 둔다면 가독성이 더 좋지 않을까용?

setChooseObj([]);
};

// chooseObj 예시: ['fun', true, false, true]
Copy link
Contributor

Choose a reason for hiding this comment

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

이런 주석은 좋군요!
근데 위에 말씀드렸다 싶히 객체 사용을 고려해보세요!

Comment on lines +32 to +53
const handleClickMorning = () => {
setChooseObj((prev) => {
if (prev.length >= 2) {
//만약 이미 들어있다면
prev.pop(); //해당 마지막 항목 삭제
}
return [...prev, true]; //사실 ...prev는 필요없긴 하지만, 다음 스텝에서 코드 그대로 활용할거라 유지
});
setIsAvail(true);
setisMorning(true);
};
const handleClickNight = () => {
setChooseObj((prev) => {
if (prev.length >= 2) {
//만약 이미 들어있다면
prev.pop(); //해당 마지막 항목 삭제
}
return [...prev, false]; //사실 ...prev는 필요없긴 하지만, 다음 스텝에서 코드 그대로 활용할거라 유지
});
setIsAvail(true);
setisMorning(false);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

요 부분도 하나의 함수로 props만 다르게 해서 사용가능!

Copy link
Contributor

@ljh0608 ljh0608 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 +48 to +50
case -2:
return <RandomStep setStep={setStep} />;
case -1:
Copy link
Contributor

Choose a reason for hiding this comment

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

지금 제시해주신 방법도 first에 -2값이 들어가면서 코드를 이해하기 어려운 부분이 있습니다. 정말 맨 처음 컴포넌트를 0으로 시작한다면 -1을 고려하지 않아도 될까요?

사실 이런 부분은 case를 1,2,3 도 좋지만 각자의 의미를 담은 step string 변수를 사용하는 것이 가장 좋아보여요! 단순히 1,2,3은 의미있는 케이스 분류는 아니니까요! 특히 랜덤추천과 취향대로 추천은 같은 위계의 컴포넌트인데 -1, 0으로 분류하면서 컴포넌트 흐름을 파악하기 어려워 보입니다!

Comment on lines +33 to +39
setChooseObj((prev) => {
if (prev.length >= 1) {
//만약 이미 들어있다면
prev.pop(); //해당 마지막 항목 삭제
}
return [...prev, "kind"]; //사실 ...prev는 필요없긴 하지만, 다음 스텝에서 코드 그대로 활용할거라 유지
});
Copy link
Contributor

Choose a reason for hiding this comment

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

interface PersonalType {
  personal: "kind" | "fun" | "passion" | "default";
}

interface IsSkillType {
  isSkill: "true" | "false" | "default";
}

interface SelectCriteria {
  personal: PersonalType;
  isSkill: IsSkillType;
}

요렇게 사용하시면 됩니다! <> 로 사용한 부분이랑 = 붙인거 다 실수에요.. vscode상에서 안하니 문법 섞임 이슈

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.

3 participants