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

[염성진] Sprint11 #313

Conversation

MELATONIN99
Copy link
Collaborator

@MELATONIN99 MELATONIN99 commented Aug 23, 2024

기본

회원가입

  • 유효한 정보를 입력하고 스웨거 명세된 “/auth/signUp”으로 POST 요청해서 성공 응답을 받으면 회원가입이 완료됩니다.
  • 회원가입이 완료되면 “/login”로 이동합니다.
  • 회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

로그인

  • 회원가입을 성공한 정보를 입력하고 스웨거 명세된 “/auth/signIp”으로 POST 요청을 하면 로그인이 완료됩니다.
  • 로그인이 완료되면 로컬 스토리지에 accessToken을 저장하고 “/” 로 이동합니다.
  • 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.
    메인
  • 로컬 스토리지에 accessToken이 있는 경우 상단바 ‘로그인’ 버튼이 판다 이미지로 바뀝니다.

심화

  • 로그인, 회원가입 기능에 react-hook-form을 활용해봅니다.

주요 변경사항

  • 로그아웃 기능도 추가하였습니다.
  • 기능은 정상 작동하나 추후 커밋으로 UI를 마저 구현하겠습니다.

멘토에게

  • react-hook-form을 사용하였는데 제대로 한 것이 맞는지 궁급합니다!
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@MELATONIN99 MELATONIN99 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 23, 2024
@MELATONIN99 MELATONIN99 self-assigned this Aug 23, 2024
const [isLoggedIn, setIsLoggedIn] = useState(false);
const [isOpen, setIsOpen] = useState(false);
const router = useRouter();
const handleLogout = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수 이름이 handleLogout으로 되어있는데 동작하는 코드는 로그아웃이랑 관련이 없는 코드인것같아요.
logout ui를 열었다 닫는 용도인것같아서 함수 이름을 다른 이름으로 바꿔보는건 어떨까요???

함수가 어떤 동작을 하느냐로 생각해보고 이름지으면 좋습니다 :-)

};
const onClickLogout = () => {
window.localStorage.removeItem("accessToken");
router.reload();
Copy link
Collaborator

Choose a reason for hiding this comment

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

요구사항에 로그아웃 후에 페이지가 새로고침 되어야 한다 면 무방하지만
로그아웃 이후에 프로필이 로그인으로 안바껴서 새로고침 하는거라면

localStorage는 지역state 밖이라 컴포넌트에선 감지 안되는 걸 염두해 두시고 방법을 찾아보세요 :-)

<Link href="/login" className={S.loginBtn}>
로그인
</Link>
{isLoggedIn ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

중첩된 삼항연산자는 코드를 읽는 흐름이 좋지않게 되어서 렌더 단위를 함수로 만들어서 JSX를 반환하거나
if-elseif 문으로 보기쉽게 만드는것도 좋을것같네요

 const renderLoggedIn = () => (
    <div className={S.userProfileIcon}>
      <Image
        src="/images/icon/ic_null_user_profile_image.png"
        fill
        alt="유저 프로필 아이콘"
        onClick={handleLogout}
      />
      {isOpen && (
        <div className={S.dropDown} onClick={onClickLogout}>
          로그아웃
        </div>
      )}
    </div>
  );

  const renderLoggedOut = () => (
    <Link href="/login" className={S.loginBtn}>
      로그인
    </Link>
  );

  return isLoggedIn ? renderLoggedIn() : renderLoggedOut();

Comment on lines +43 to +62
<label htmlFor="email">이메일</label>
<input
id="email"
className={S.inputBox}
type="email"
placeholder="이메일을 입력해주세요"
{...register("email", {
required: "이메일을 입력해주세요",
pattern: {
value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/,
message: "잘못된 이메일 형식입니다",
},
})}
aria-invalid={isSubmitted ? (errors.email ? "true" : "false") : undefined}
/>
{errors.email && (
<small role="alert" className={S.errorMessage}>
{String(errors.email.message)}
</small>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<label htmlFor="email">이메일</label>
<input
id="email"
className={S.inputBox}
type="email"
placeholder="이메일을 입력해주세요"
{...register("email", {
required: "이메일을 입력해주세요",
pattern: {
value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/,
message: "잘못된 이메일 형식입니다",
},
})}
aria-invalid={isSubmitted ? (errors.email ? "true" : "false") : undefined}
/>
{errors.email && (
<small role="alert" className={S.errorMessage}>
{String(errors.email.message)}
</small>
)}
<label htmlFor="email">이메일</label>
<input
id="email"
className={S.inputBox}
type="email"
placeholder="이메일을 입력해주세요"
{...register("email", {
required: "이메일을 입력해주세요",
pattern: {
value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/,
message: "잘못된 이메일 형식입니다",
},
})}
aria-invalid={isSubmitted ? (errors.email ? "true" : "false") : undefined}
/>
{errors.email && (
<small role="alert" className={S.errorMessage}>
{String(errors.email.message)}
</small>
)}
  • label부터 errorMessage까지 signup,login에 똑같은 코드를 쓰고있어서 반복되는 룰이나, 컴포넌트는 재사용 해 보는건 어떨까요?
    *룰은 변수에 담아서, JSX는 컴포넌트로 추상화 해서 추출 할 수 있을것같아요 :-)
<small role="alert" className={S.errorMessage}>
            {String(errors.email.message)}
</small>

=>
import React from 'react';

interface ErrorMessageProps {
  message?: string;
}

const ErrorMessage: React.FC<ErrorMessageProps> = ({ message }) => {
  if (!message) return null;

  return (
    <small role="alert" className={S.errorMessage}>
      {message}
    </small>
  );
};

export default ErrorMessage;
{
            required: "이메일을 입력해주세요",
            pattern: {
              value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/,
              message: "잘못된 이메일 형식입니다",
            },
          }

=>

const emailValidationRules = {
  required: "이메일을 입력해주세요",
  pattern: {
    value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/,
    message: "잘못된 이메일 형식입니다",
  },
};
  • pattern에 정규표현식에는 어떤 조건인지 주석으로 남기면 다른 개발자가 이해하기 수월할 것같아요

Comment on lines +63 to +81
<label htmlFor="password">비밀번호</label>
<input
id="password"
className={S.inputBox}
type="password"
placeholder="비밀번호를 입력해주세요"
{...register("password", {
required: "비밀번호를 입력해주세요",
minLength: {
value: 8,
message: "비밀번호를 8자 이상 입력해주세요",
},
})}
aria-invalid={isSubmitted ? (errors.password ? "true" : "false") : undefined}
/>
{errors.password && (
<small role="alert" className={S.errorMessage}>
{String(errors.password.message)}
</small>
Copy link
Collaborator

Choose a reason for hiding this comment

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

위에 리뷰랑 동일해요! 특히 에러 메시지는 여기서도 쓰일 수 있겠네요 :-)

Comment on lines +49 to +55
{...register("email", {
required: "이메일을 입력해주세요",
pattern: {
value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/,
message: "잘못된 이메일 형식입니다",
},
})}
Copy link
Collaborator

Choose a reason for hiding this comment

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

login에 리뷰 참고 해 보세요! :-)

required: "비밀번호를 다시 한 번 입력해주세요",
minLength: {
value: 8,
message: "비밀번호를 8자 이상 입력해주세요",
Copy link
Collaborator

Choose a reason for hiding this comment

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

요구사항에 "비밀번호를 8자 이상 입력해주세요" 라고 메시지를 렌더하도록 되어있다면 상관없지만,
요즘 유저 정보 에러 메시지는 대부분 구체적으로 알려주지않아요! 유추해서 풀 수도 있기때문에 보안상 좋지않기때문이에요.
잘못입력했다 라던가, 다시 입력하라고 하거나 모호하게 알려줘서 여러번 시도하기 어렵게 만드는게 좋습니다 :-)

@201steve
Copy link
Collaborator

  • react-hook-form을 사용하였는데 제대로 한 것이 맞는지 궁급합니다!

공식문서 보시고 쓰셔서 원하는데로 렌더하고, form 양식에 맞춰서 서버로 보내고, 에러메시지 렌더가 잘 됐다면 잘 쓰신게 맞습니다 :-)
이게 맞는건가 싶을땐 원하는데로 동작하면 맞는거라고 생각하시고, 동작하는 방법대로 익숙해질때 까지 많이 써보다가 다른 방법도 하나씩 터득해 나가면 됩니다.
만약 동작하지않으면 다시 공식문서 보시고 차근차근 해보시길 바래요 :-)

@201steve 201steve merged commit 7e16e0c into codeit-bootcamp-frontend:Next-염성진 Aug 28, 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