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

[4주차 기본/심화/생각 과제] 로그인/회원가입 #3

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

Conversation

eonseok-jeon
Copy link
Contributor

@eonseok-jeon eonseok-jeon commented Nov 17, 2023

🌱 기본 조건

  • .env 파일 사용하기

🧩 기본 과제

[ 로그인 페이지 ]

  1. 로그인
    • 아이디와 비밀번호 입력후 로그인 버튼을 눌렀을시 성공하면 /mypage/:userId 로 넘어갑니다. (여기서 userId는 로그인 성공시 반환 받은 사용자의 id)
  2. 회원가입 이동
    • 회원가입을 누르면 /signup으로 이동합니다.

[ 회원가입 페이지 ]

  1. 중복체크 버튼

    • ID 중복체크를 하지 않은 경우 검정색입니다.
    • ID 중복체크 결과 중복인 경우 빨간색입니다.
    • ID 중복체크 결과 중복이 아닌 경우 초록색입니다.
  2. 회원가입 버튼

    • 다음의 경우에 비활성화 됩니다.
    • ID, 비밀번호, 닉네임 중 비어있는 input이 있는 경우
    • 중복체크를 하지 않은 경우
    • 중복체크의 결과가 중복인 경우
    • 회원가입 성공시 /login 으로 이동합니다.

[ 마이 페이지 ]

  1. 마이 페이지
    • /mypage/:userId 의 userId를 이용해 회원 정보를 조회합니다.
    • 로그아웃 버튼을 누르면 /login으로 이동합니다.

🌠 심화 과제

[ 로그인 페이지 ]

  1. 토스트
    • createPortal을 이용합니다.
    • 로그인 실패시 response의 message를 동적으로 받아 토스트를 띄웁니다.

[ 회원가입 페이지 ]

  1. 비밀번호 확인

    • 회원가입 버튼 활성화를 위해서는 비밀번호와 비밀번호 확인 일치 조건까지 만족해야 합니다.
  2. 중복체크

    • 중복체크 후 ID 값을 변경하면 중복체크가 되지 않은 상태(색은 검정색)로 돌아갑니다.

생각과제

  • API 통신에 대하여
  • 로딩 / 에러 처리를 하는 방법에는 어떤 것들이 있을까?
  • 패칭 라이브러리란 무엇이고 어떤 것들이 있을까?
  • 패칭 라이브러리를 쓰는 이유는 무엇일까?


💎 PR Point

더 나은 코드로 리팩토링하는 거에 대해 알려주시면 감사드리겠습니다


🥺 소요 시간, 어려웠던 점

7h, 무난했습니다


🌈 구현 결과물

vercel을 이용해서 배포하려고 하고 있습니다
근데 지금 오류 뜸 + 다른 더 바쁜 일 비동기 처리 중으로 인해 뒤로 미뤄지고 있네요ㅜ

2023-12-14.5.45.01.mov

🍒 주저리주저리

최근 들어 느끼는 게
저질러 놓은 일들은 많은데 이를 다 가져가려고 하니까
하나씩 빠지는 것들이 생기다
이제는 와랄라 다 쏟아져 버리는 거 같아요

그러다 보니 점점 퀄리티가 떨어지고, 마감에 치여 얼레벌레 하고 넘어간다는 느낌이 강하게 들어요
그러면 안 된다는 걸 알면서도 쉽지가 않네요
저의 욕심 때문에 다른 분들께도 피해가 가는 거 같아 죄송할 따름입니다

너무 나약해진 거 같아요
다시 정신 차리고 그동안 못 했던 것들 다 부지런히 해나가고
처음 초심으로 돌아가 솝트 활동 열심히 이어나가보도록 하겠습니다

@eonseok-jeon eonseok-jeon self-assigned this Nov 17, 2023
@eonseok-jeon eonseok-jeon changed the title [4주차 기본/생각 과제] 로그인/회원가입 [4주차 기본/심화/생각 과제] 로그인/회원가입 Nov 18, 2023
@eonseok-jeon eonseok-jeon added the 🐧 심화과제 심화과제 label Nov 18, 2023
Copy link
Member

@qwp0 qwp0 left a comment

Choose a reason for hiding this comment

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

진짜 YB 맞냐며,,과제하느라 넘 고생하셨습니다 ☺️ 컴포넌트 분리도 너무 깔끔하게 잘하신 것 같아요.. 제가 실력 이슈로 리팩토링에 도움이 되는 코드 리뷰는 아니지만 실력,,키워서 점점 더 좋은 코리할 수 있도록 하겠습니당 !

Comment on lines +8 to +13
const router = createBrowserRouter([
{ path: '/', element: <Login />, errorElement: <Error /> },
{ path: '/login', element: <Login /> },
{ path: '/signup', element: <SignUp /> },
{ path: '/mypage/:userId', element: <MyPage /> },
]);
Copy link
Member

Choose a reason for hiding this comment

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

덕분에 createBrowserRouter에 대해서 알게 되었습니다 !! 굿굿 👍🏻

Comment on lines +3 to +6
const reqAPI = axios.create({
headers: { 'Content-Type': 'application/json' },
baseURL: 'http://3.39.54.196/',
});
Copy link
Member

Choose a reason for hiding this comment

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

오 이렇게 create 사용해서 객체 생성해준 거 너무 좋아요! 저도 axios가 제공하는 기능에 대해서 더 공부해보겠습니다!
근데 저희 env파일 이용해서 BASE_URL : http://3.39.54.196 관리해주기로 했으니깐 여기에 직접 쓰지 않고 env 파일에서 가져와서 사용하면 좋을 것 같아요!

const reqAPI = axios.create({
  headers: { 'Content-Type': 'application/json' },
  baseURL: `${import.meta.env.BASE_URL}`
});

요렇게요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헉스!! 굉장히 중요한 부분인데, 이걸 놓쳤네요ㅜㅜ 감사합니다!!!

Copy link
Member

Choose a reason for hiding this comment

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

보안을 위해서 .env 파일을 사용하는 것인만큼 .gitignore파일에 .env 추가하면 좋을 것 같습니당

Copy link
Contributor Author

Choose a reason for hiding this comment

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

악 주의하겠습니다 ㅜ 🥲

Copy link
Member

Choose a reason for hiding this comment

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

컴포넌트 분리 진짜 잘하시네요..배워갑니당 ㅎㅎ

Comment on lines +72 to +116
<S.IdDuplicateCheckBox>
<Input
label="ID"
type="text"
placeholder="아이디를 입력해"
onChange={(e) => {
setEnteredID(e.target.value);
setButtonColor('black');
}}
/>
<S.DuplicateCheckButton
width="10rem"
onClick={duplicateHandler}
button_color={buttonColor}
>
중복체크
</S.DuplicateCheckButton>
</S.IdDuplicateCheckBox>
<Input
label="PASSWORD"
type="password"
placeholder="비밀번호 입력해"
onKeyDown={signUpHandler}
onChange={(e) => {
setEnteredPW(e.target.value);
}}
/>
<Input
label="PASSWORD CONFIRM"
type="password"
placeholder="비밀번호 한 번 더 입력해"
onKeyDown={signUpHandler}
onChange={(e) => {
setConfirmPW(e.target.value);
}}
/>
<Input
label="NICKNAME"
type="text"
placeholder="닉네임 입력해"
onKeyDown={signUpHandler}
onChange={(e) => {
setEnteredNickName(e.target.value);
}}
/>
Copy link
Member

Choose a reason for hiding this comment

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

여기 데이터 상수로 분리한 다음 가져와서 사용해도 좋을 것 같습니다아

Copy link
Contributor Author

Choose a reason for hiding this comment

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

소영님 코드 참고해서 다음에 한 번 적용해 보겠습니다!!

Copy link
Member

Choose a reason for hiding this comment

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

프로필 짱귀 ㅋㅋㅎ

Comment on lines +3 to +5
const Card = ({ width = '50rem', children }) => {
return <S.CardBox width={width}>{children}</S.CardBox>;
};
Copy link
Member

Choose a reason for hiding this comment

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

헉 children prop 너무 잘 사용하시는 것 같아요 😮 저도 다음번에 써보도록 하겠습니당

Copy link
Member

@simeunseo simeunseo left a comment

Choose a reason for hiding this comment

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

아고고 언석 진짜 대단합니다,,, 일단 공통컴포넌트 사용 방식에서 진짜 인사이트 많이 얻어가구요! 전체적으로 저랑 리액트 코드 짜는 방식이 되게 다른거같아서 진짜 흥미로웠어요 ㅎㅎㅎ 또 생소한 방식임에도 불구하고 가독성이 정말 좋았습니다. 컨벤션이 엄청 일관되고 네이밍도 잘하셔서 그런거같아요.. 멋집니다..!! 더 스마트한 리뷰를 드리고싶었는데 하 제가 이번과제도 못하고 그래가지구ㅠㅠㅠㅠ 진짜 아쉬울따름...

주저리주저리에 쓰신 내용이 꼭 제 일기같네요.. 그치만 제가 보고 있는 언석님은 너무나 많은 것을 또 잘 해내고 있는걸요!ㅠㅠ 바쁜와중에 저번 과제도 미리미리 해놓고 가고, 이번 과제도 완벽히 해내구 코드리뷰도 엄청 공들여해주셔서 존경스러웠답니다,,, 저도 제 자신에게 기대했던 만큼을 해내지못해서 아쉬운 나날들이지만 충분히 최선을 다하고 있으니까요!! 다시 부지런히 힘내봅시다 👏

<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>뭐요</title>
Copy link
Member

Choose a reason for hiding this comment

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

요즘 많이 힘들군요 언석,,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 ㅋㅋㅋㅋㅋㅋㅎㅋㅎㅋㅋㅎㅎㅋ

Comment on lines +37 to +41
const loginHandler = (e) => {
if (e.key === 'Enter') {
submitHandler();
}
};
Copy link
Member

Choose a reason for hiding this comment

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

디테일 너무조아요...

enteredPW === confirmPW &&
buttonColor === 'green';

const router = useNavigate();
Copy link
Member

Choose a reason for hiding this comment

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

보통 navigate로 쓰는게 약간 관행인거같은데 router로 쓰는 이유가 따로 있나요?! 저번처럼 특정 컨벤션을 따르는 건지~~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아뇨!! 제가 공부한 예제에선 router라고 사용을 해서 router라고 했었습니다!! navigate라고 많이 사용하는군요?? 몰랐어요!! 저도 관행을 따라가도록 하겠습니다 :)

Comment on lines +22 to +24
&:not(:last-child) {
margin-bottom: 1rem;
}
Copy link
Member

Choose a reason for hiding this comment

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

아니 이거진짜 폼미쳤다 이거 꼭 써먹을게요!!!


const Button = ({ width = '40rem', to, as, type, onClick, disabled, children }) => {
return (
<S.ButtonLink width={width} to={to} as={as} type={type} onClick={onClick} disabled={disabled}>
Copy link
Member

Choose a reason for hiding this comment

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

헉 저 여기 prop들에 대해 굉장히 의구심이 생겼는데.. 지금 Button은 사실상 Link태그인거고, 따라서 to를 넘겨주는거까진 알겠는데 as랑 type은 무엇이죠..?!!

Link 태그에 to말고 다른 prop이 있나싶어서 공식문서를 읽어봤는데 .. 없는거같거든요,,? 잘못읽었나.. 근데 GPT는 또 as라는 prop이 있다고 설명해주네요...
알려주세요 궁금해요!!! 그냥 사용방식을 기록(?)하기 위함인가... 그러진않을거같은데 저 as랑 type prop을 어디서 쓰는건지 모르겠어요..!

일단 그거는 차치하고,, Link태그를 button 대신 사용하는 방식이 진짜 재밌네요 보통은 페이지 이동해야되면 onClick에 navgiate넣어서 하곤했는디...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 이게; 저도 짜면서 아 뭔가 좀 짜친다 싶었는데, 저는 해당 컴포넌트를 Link로 만들었어요! 근데 다른 곳에서 이를 재사용하는데, button 태그로 사용이 필요한 경우가 있었어요 (form submit을 위해)
이때 as를 통해 as="button" 이렇게 해주면 a tag가 아닌 button tag로 바꿔주게 돼요!
button에는 type이 필요하니, type 속성도 넣어주었습니다
아 근데 지금 작성하면서 생각해보니, 그냥 button 태그로 Link를 감싸주고 거기에 onSubmit 넣었으면 as랑 type 안 써도 되었을 거 같다는 생각이 드네요; 크흠,,

아 그리고 네이밍도 link인데 button으로 한 것도 마음에 들지 않네요 지금보니,, ㅋㅋㅎ,,
흐,,

버튼에 onClick을 넣어주면 useNavigation 함수도 이용해야 하고 onClick도 이용해야 하고 번거러운 거 같아서 저는 그냥 Link를 이용해서 라우팅 해주는 편이에요
흠 근데 그냥 button에 onClick을 이용해서 하는 편이 뭔가 컨벤션이나 가독성?(header에는 header tag를 이용하듯이 button 기능을 하니 button 태그를 이용해야 한다?) 더 좋다고 생각하시나요??

placeholder="아이디를 입력해"
onChange={(e) => {
setEnteredID(e.target.value);
setButtonColor('black');
Copy link
Member

Choose a reason for hiding this comment

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

buttonColor의 default가 이미 'black'이어서 onChange에 setButtonColor('black');를 했을 때 아무것도 변경되지 않는 것 같은데 혹시 이 코드의 의미가 무엇일까요?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

한 번 중복 체크를 한 경우 색이 빨간색이나 초록색으로 변하게 되는데 다시 새로운 값을 입력한 경우 검정색으로 바뀌게 만들고 싶었습니다!! (심화 과제이기도 하였고요)

음 뭔가 비효율적인 거 같기도 하고, 색이 빨강이나 초록일 때만 검사하도록 수정하는 게 좋을 거 같기도 하네요

좋은 방법 없을까요?

<S.DuplicateCheckButton
width="10rem"
onClick={duplicateHandler}
button_color={buttonColor}
Copy link
Member

Choose a reason for hiding this comment

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

엇 여기에만 snake_case쓴 이유가 따로있나요? 스타일링에 쓰이는건 snake로 쓰는 컨벤션인감..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 그 왜인지는 사실 찾아보지 않아서 잘 모르겠는데, camelCase로 할 경우 크롬에서 콘솔 창에 에러를 출력하더라고요? 속성값으로 대문자 말고 소문자를 이용해라고,,
렌더링에는 문제가 없지만 고런 에러들 보는 거 살짝 못 참는 병 있어가지고; buttoncolor라고 하기엔 가독성 너무 떨어지니까 스네이크로 수정을 했었습니다!

Comment on lines +17 to +22
const isAble =
enteredID.trim().length !== 0 &&
enteredPW.trim().length !== 0 &&
enteredNickName.trim().length !== 0 &&
enteredPW === confirmPW &&
buttonColor === 'green';
Copy link
Member

Choose a reason for hiding this comment

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

이 유효성 검사를 특정 handler 내부나 hook 안에서 쓴게 아니라 그냥 const로 선언해놓고 사용한게 좀 특이한것같아요..! 왜냐면 이 유효성은 폼 내부 input의 값이 바뀔때마다 작동해야하는거니까,, 흠 이게 input에 입력할때마다 SignUp 컴포넌트 전체가 리렌더링되어서 isAble값이 계속해서 업데이트가 되는 것 같아요! 뭔가 좀 더 개선할 지점이 없을지 생각하게 되네용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

useEffect 공식 문서에 참고한 내용인데, input 값이 변할 때 마다 작동해야 하니까 useEffect에 input 값을 종속성으로 추가한 뒤 구현을 하면 될 거 같지만 이를 지양하라고 나와있었습니다!!
말씀해주신대로 input 값이 바뀌면 리랜더링 되니 저런 식으로 변수 할당 해주면 이 또한 새로운 값으로 매번 초기화 되는 것이니까 useEffect를 사용하지 않아도 된다고요

input 값이 업데이트 될 때마다 버튼의 활성화를 매번 검사해줘야 해서 랜더링 될 때마다 isAble도 업데이트 되게 하기 위해 저런 식으로 구현을 했었습니다!!

이런 과정을 통해 저런 식으로 구현을 한 것인데, 혹시 좀 개선이 되어야 할 부분이 있을까요??

border: 1px solid #ddd;
border-radius: 0.5rem;
background-color: ${({ disabled }) => (disabled ? 'rgba(0,0,0,0.5)' : '#000')};
cursor: ${({ disabled }) => (disabled ? 'default' : 'cursor')};
Copy link
Member

Choose a reason for hiding this comment

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

여기 cursor: 'cursor' 이거는 좀 잘못된 것 같은데 'pointer'를 잘못 쓰신건가...?

근데 Link는 어차피 원래 cursor:pointer가 되어있으니깐, disabled 상태일때 cursor가 default가 되도록 이렇게만 써줘도될거같아요!

Suggested change
cursor: ${({ disabled }) => (disabled ? 'default' : 'cursor')};
cursor: ${({ disabled }) => disabled && 'default'};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅎㅋ ㅋㅋ 헉스;
이걸 잡으시네요,,
수정하도록 하겠습니다 감사해용 :)

e.preventDefault();

try {
const data = await reqAPI.post('/api/v1/members/sign-in', {
Copy link
Member

Choose a reason for hiding this comment

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

저는 아예 모든 api에 대한 함수를 apis 폴더에 정의해놓고 export해서 쓰는 방법을 선호하는데 그러면 이 부분이 가독성이 매우 좋아져요!

Copy link
Contributor Author

@eonseok-jeon eonseok-jeon Nov 22, 2023

Choose a reason for hiding this comment

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

헉 그렇게 하면 너무 좋을 거 같아요!!
그 부분에 대해 예시를 한 번 보고 싶어요!!

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.

3 participants