-
Notifications
You must be signed in to change notification settings - Fork 3
Feat/insert 페이지 step2 메인 컴포넌트 및 카드 컴포넌트 전체 구현 #55
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
Feat/insert 페이지 step2 메인 컴포넌트 및 카드 컴포넌트 전체 구현 #55
Conversation
|
@MINYOUNG-SEOK is attempting to deploy a commit to the tkyoun0421's projects Team on Vercel. A member of the Team first needs to authorize it. |
czmcm5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엄청 많이 생겼어요~! 수고하셨습니다🩷
| const isMobile = useMediaQuery(theme.breakpoints.down("md")); | ||
|
|
||
| // 초기 포지션이 없을 때 하나 추가 | ||
| const positions = value.length > 0 ? value : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value는 배열 타입으로 넘어오는 거지요? 이 코드는 사실 상 value가 무조건 배열이기에 굳이 선언하지 않아도 괜찮아보입니다 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이제보니 밑에서 아래와 같은 로직이 실행되더라구요!
// 최소 하나의 포지션이 없으면 추가
if (positions.length === 0) {
setTimeout(() => addPosition(), 0);
return
}
setTimeout을 실행해 주는 것 보다 차라리 여기서 배열검사를 하니 여기서 선언해 주는 편이 스크립트 실행을 하나라도 줄이고 가독성 측면에 좋아보입니다!
const initData = { ... } // addPosition안의 객체를 밖으로 빼어 선언
const positions = value.length === 0 ?[iniTData] : value
| // 현재는 setps를 객체로 된 배열로 관리하지만 단순한 스타일의 반복이니 | ||
| // JS로 탐색 후 뿌려주는 것 보단 공통 스타일 컴포넌트를 생성하여 구성하는 것이 더 좋아보입니다! | ||
| // 추후에 여유가 될 때 리팩토링 해주시면 좋을거같습니다 ~ | ||
| // TODO: JS로 탐색 후 뿌려주는 방법 -> 공통 스타일 컴포넌트 생성하여 구성 처리 예정 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제 의견을 반영해 주셧군요 !!
공통 스타일 컴포넌트 구성하느라 고생하셨어요 ><!
그런데 아직 .map으로 steps를 뿌려주고 있어서 해당 컴포넌트를 열게 될 때 JS가 배열을 한번 순회(탐색)하고 있어요
공통 컴포넌트로 뺀김에 그냥 를 4번 써주면 더 깔끔해지고 좋아질 것 같습니다
HoneyTipBox 컴포넌트를 참고해주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어라 제 코멘트에 왜 선이 좍좍 그어져있을까욧.. 😓
| outline: "none", | ||
| transition: "border-color 0.2s ease-in-out", | ||
| }} | ||
| onFocus={(e) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홋 컴포넌트 내부 매소드로도 이런 css 구성이 가능하군요 처음알았어요
이후에 스타일들을 컴포넌트로 분리한다면 css파일에서 쓰는것 처럼 :hover, :foucs로 간단하게 구현이 가능 할 것 같아서 코멘트 남깁니다! 👍
namee-h
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오우 고생많으셨어요 ! 👏🏻
tkyoun0421
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 👍 👍
| const handlePrev = (): void => { | ||
| setCurrentStep((prev) => prev - 1); | ||
| window.scrollTo({ top: 0, behavior: "smooth" }); | ||
| }; | ||
| const handleNext = (): void => { | ||
| setCurrentStep((prev) => prev + 1); | ||
| window.scrollTo({ top: 0, behavior: "smooth" }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 두 함수가 동일한 구조의 로직을 담당하고 있고 두 가지의 일을 하고 있는 것 같습니다!
버튼은 step 상태를 변경하는 것만 담당하고 scrollToTop 로직은 따로 분리하여 step 상태 변경시 실행되는 구조가 관심사도 분리되고 함수 단일 책임원칙에 더 부합하다고 생각을 합니다
e781c25
into
amicable-development-center:develop
개요
프로젝트 등록 페이지의 Step2 단계 전체 구현 - 팀 규모, 예상 기간, 기술 스택, 모집 포지션 입력 폼
변경 사항
구현 내용
개발 후기 및 개선사항
이번 작업에서 배운 점
어려웠던 점 / 에로사항
다음에 개선하고 싶은 점
팀원들과 공유하고 싶은 팁