Skip to content

Conversation

minjeong9919
Copy link
Collaborator

@minjeong9919 minjeong9919 commented May 14, 2024

주요 변경사항

  • 이번 주차는 전체적인 코드의 리팩토링 작업만 진행하였습니다!
  • portal을 통해 토스트 컴포넌트를 렌더링하고,
  • context API를 이용하여 토스트 컴포넌트와 모달 컴포넌트를 전역으로 관리하도록 수정하여 전체적으로 코드를 간단하고 깔끔하게 짜려고 했습니다.
  • 또한 이를 이용하여 로그인 화면, 회원가입 화면의 미완성 부분을 완성하였습니다.

스크린샷

vercel: https://4-weekly-mission-git-part3-week20-kim-minjeongs-projects.vercel.app/

토스트 컴포넌트
스크린샷 2024-05-14 오후 2 55 20

멘토에게

  • 커밋 메시지가 대문자, 소문자로 뒤죽박죽입니다.. ㅜㅜ rebase를 통해 수정하다가 잘 안돼서
    그냥 제출했습니다... 이 점 고려하시고 봐주세요!
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

- 파일 경로 수정
- Image, a 태그를 Image, Link태그로 수정
전에 쓰던 코드 내용이 복잡하고, input으로 props도 많이 넘겨주는 것 같아 코드를 수정했습니다.
- modal portal을 이용해 토스트 컴포넌트 화면에 띄우도록 하였고, context API를 이용해 전역적으로 관리가 가능하도록 구현하였습니다.
- 토스트 반응형으로 수정
- 인풋에 키보드 이벤트 추가
Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
4-weekly-mission ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2024 6:00am
4-weekly-mission-ursd ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2024 6:00am

@minjeong9919 minjeong9919 requested a review from devToram May 14, 2024 05:59
@minjeong9919 minjeong9919 self-assigned this May 14, 2024
@minjeong9919 minjeong9919 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 14, 2024
@minjeong9919 minjeong9919 added the 미완성 죄송합니다.. label May 14, 2024
Copy link
Collaborator

@devToram devToram 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 +12 to +16
eventFunc: {
onFocusOut: () => void;
onChange: () => void;
onKeydown: (e: React.KeyboardEvent<HTMLInputElement>) => void;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

JSX.IntrinsicElements['input'] 쓰셔도 괜찮을 거 같아요!

}
};

},[inputErrorInfo])

return (
<InputDiv>
<p>{title}</p>
<EmailPasswordInput
type={isViewPassword ? "text" : type}
Copy link
Collaborator

Choose a reason for hiding this comment

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

type 도 string 보다 조금 더 구체적인 타입을 가지거나 기존 Input 의 type 프로퍼티를 확장하면 좋을 거 같아요!

Comment on lines +33 to +35
const openDeleteModal = () => {
modal.openModal(<DeleteModal />);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트 자체를 파라미터로 넘기면 나중에 추적할 때 복잡해져서 컴포넌트는 왠만하면 렌더부에만 선언하는 걸 추천드려요!


if(!modalRoot) return;

return createPortal(children, modalRoot)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

type ToastType = {
errorMessage?:string | null;
isToastView: boolean,
setIsToastView: React.Dispatch<React.SetStateAction<boolean>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

아마 useState 의 setter 부분일 거 같은데, setter 를 바로 setter 에서 넘기는 것보다는 해당 useState 를 선언한 곳에서 해당 setter 를 감싼 callback 을 만들고 그 callback 을 넘겨주시는 걸 추천드려요!
한 컴포넌트에서 선언한 state 는 해당 컴포넌트 안에서만 변경할 수 있게 하는 게 좋아요:)

@@ -0,0 +1,8 @@

const ToastPortalContent = ({children}: {children?:any}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

children 만 리턴해주는 기능이라면 <></> 랑 다를 게 없어서 따로 컴포넌트로 만들지 않아도 될 거 같아요~!

Comment on lines +11 to +15
export const useModal = () => {
const context = useContext(ModalContext);
if(!context) throw new Error ("modal error");
return context;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

훅은 따로 훅만 빼주셔요!


const openModal = (modal: React.ReactNode) => {
setIsOpen(true);
setModalComponent(modal);
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 구조를 보면 children 으로 온 모달말고도, 모달 내부에 있을 내부모달(?)로 추정되는 modalCompenent 도 렌더부에서 병렬적으로 배치하셨는데, 만약 모달 내 모달이 맞다면 해당 children 으로 들어오는 곳에서 modal 을 다시 정의하는 게 자연스러울 거 같아요!

@@ -3,29 +3,30 @@ import { postCheckDuplicationEmail } from "@/api/api";

// 이메일 유효성 검사
function emailCheck(email_address: string) {
let email_regex = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/i;
const email_regex = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/i;
return Boolean(email_regex.test(email_address));
Copy link
Collaborator

Choose a reason for hiding this comment

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

test 가 return 값이 boolean 이라 따로 래핑 안해주셔도 될 거 같아요!

Comment on lines +20 to +29
export const emailInputValidationcheck = (email: string): ErrorValid => {
let emailValid: ErrorValid;
if (!email) {
status = INPUT_STATUS.noEmail;
emailValid = INPUT_STATUS.noEmail;
} else if (!emailCheck(email)) {
status = INPUT_STATUS.wrongEmail;
emailValid = INPUT_STATUS.wrongEmail;
} else {
status = "valid";
emailValid = "valid";
}
return status;
return emailValid;
Copy link
Collaborator

Choose a reason for hiding this comment

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

early return 적용하면 emailValid 변수 선언 필요없을 거 같아요!

@devToram devToram merged commit f73136f into codeit-bootcamp-frontend:part3-김민정 May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants