-
Notifications
You must be signed in to change notification settings - Fork 0
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주차 기본/심화/공유 과제 ] 로그인, 회원가입 구현해보기 #5
base: main
Are you sure you want to change the base?
Conversation
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.
전체적으로 코드의 로직을 분리하려고 노력한 게 눈에 정말 많이 보이네요!
input 요소가 많이 사용되는 걸 알면서도 애써 무시하고 똑같은 코드를 복붙해서 작성했어서... 화랑님 코드 보면서 정말 반성했습니다
커스텀훅을 사용해서 로직들을 분리해주니까 코드의 가독성도 훨씬 좋아지고 각 코드의 길이도 줄어들어서 너무 잘 활용하신 것 같습니다! 고생많으셨어요 정말 많이 배워갑니다
합세도 화이팅 잘 부탁드립니다!!🔥
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.
사용하지 않는 파일이라면 지워주는 게 어떨까요?
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.
바로 삭제했습니다~
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.
리액트 기초 세팅할 때 이렇게 사용하지 않는 파일(파비콘 파일)도 같이 지워주면 좋을 것 같습니다!
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.
넵!
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.
저도 화랑님처럼 공통적으로 사용하는 버튼 컴포넌트들이 정말 많은데 전 맘이 급해서 죄다 복붙해버렸네요..
반성하고 갑니다!
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.
이번주가 유독 바빴죠.. ㅠ
width: 4rem; | ||
height: 2rem; | ||
|
||
/* padding: 0.5rem 1rem; */ |
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.
사용하지 않는 코드는 지워줍시다아~
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.
네에~
inputType: string; | ||
id: string; | ||
value: string | number; | ||
onChange?: () => void; |
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.
onChange 타입도 명시하셨군요 배워갑니다!!
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.
저도 배워가요!
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.
모각공에서 정안님이 알려주셨어요 ㅋㅋㅋ
<S.HomePageHeader>4주차 과제중~ing</S.HomePageHeader> | ||
<ReactPlayer url={video} controls playing /> | ||
<S.ButtonsContainer> | ||
<RoutingBtn route={`mypage/${params.memberId}`} btnText="내 정보" /> |
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.
버튼의 타입이 기본적으로 submit이니까 타입을 button으로 지정해주면 좋을 것 같습니다!
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.
제가 이해를 잘 못했는데 RoutingBtn 컴포넌트의 type을 button으로 지정하라는 뜻일까요?
const { | ||
idErrMessage, | ||
pwErrMessage, | ||
handleIdChange, | ||
handlePwChange, | ||
handleLogin, | ||
} = useLogin(); |
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.
이런식으로 커스텀 훅을 사용하니 확실히 코드 가독성도 좋아지고 전체적인 코드의 로직이 분리돼서 넘 좋네요 ... 많이 배워갑니다
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.
감사합니다!
id="pw" | ||
onChange={handlePwChange} | ||
/> | ||
<S.ErrorMessage>{pwErrMessage ? pwErrMessage : ""}</S.ErrorMessage> |
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.
else 경우 빈 문자열이니
<S.ErrorMessage>{pwErrMessage && pwErrMessage}</S.ErrorMessage>
요런식으로 작성하는 건 어떨까요?
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.
넵 반영하겠습니다!
align-items: center; | ||
` | ||
|
||
export const MyPageBox = styled.div` |
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.
export const MyPageBox = styled.section`
영역이라면 section, 재사용되는 부분이라면 article 태그와 같은 시맨틱 태그를 활용해서 웹 접근성을 높여보는 것도 좋겠네요!
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.
오 저기서도 시맨틱 태그를 쓸 수 있겠네요 반영하겠습니다!
<div | ||
style={{ | ||
visibility: isToggleOpen ? 'visible' : 'hidden', | ||
width: '80%', | ||
display: 'flex', | ||
flexDirection: 'column', | ||
alignItems: 'center', | ||
gap: '1rem', | ||
}} |
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.
여기 부분만 인라인 스타일을 사용해주신 이유가 있을까요?
인라인 스타일의 경우 코드의 재사용성이 떨어지고 유지 보수 측면에서 좋지 못해서 지양한다고 알고 있습니다!
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.
isToggleOpen 값에 따라 스타일을 달리 주려고 하다가 아마 시간이 없어서 그렇게 한 것 같아요 수정해보겠습니다!
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 App = () => { | ||
return ( | ||
<> | ||
<RouterProvider router={router} /> |
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.
오.. 다은님은 이거 그냥 router.tsx로 컴포넌트 만들었던데 이런 방법도 있었네요.
하나 또 배웁니다!
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.
맞아요 컴포넌트로 만들어도 됩니다!
export const router = createBrowserRouter([ | ||
{ | ||
path: "/", | ||
element: <Login />, | ||
}, | ||
{ | ||
path: "/main/:memberId", | ||
element: <Home />, | ||
}, | ||
{ | ||
path: "/mypage/:memberId", | ||
element: <MyPage />, | ||
}, | ||
{ | ||
path: "/signup", | ||
element: <SignUp />, | ||
}, | ||
]); |
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.
오 이렇게 그냥 한번에 정리해버리는 방법이..!
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.
good!
@@ -0,0 +1,8 @@ | |||
import React from "react"; | |||
import * as S from "./FnBtn.styled"; |
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.
컨벤션으로 논의했던건데 바로 적용하셨네요!
전 코드 작성하는데 급급해서 막 썼는데.. 공통컴포넌트 잘 나눈것도 잘 보고 갑니다!
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.
저도 3주차 코리조에서 정안님이 하시는것을 보고 배웠습니다!
inputType: string; | ||
id: string; | ||
value: string | number; | ||
onChange?: () => void; |
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 [formInfo, setFormInfo] = useState({ | ||
id: "", | ||
pw: "", | ||
}); |
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.
오.. 활용하는 거 진짜.. 배워갑니다!
alert(response.data.message) | ||
navigate('/') |
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.
저도 이런식으로 try안에 성공 후 코드를 박아뒀는데, 다은님은 .then 메서드를 썼더라구요. 혹시나 해서 알려드립니다!
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.
다은님은 axios로 api 요청하는 코드 양식을 커스텀해서 사용하신것 같더라구요
if (!id) { | ||
setIdErrMessage(INPUTEMPTY.ID); | ||
return; | ||
} | ||
if (!pw) { | ||
setPwErrMessage(INPUTEMPTY.PW); | ||
return; | ||
} |
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.
전 인풋값 비교하는 부분, else if 로 연결해서 하나 하나 체크하게 만들어 조금이라도 if문을 덜 들어가게 만들었는데,
전부 체킹하도록 만드셨군요! 뭐가 더 좋은지는 모르겠지만, 전부 다 확인할 수 있다는 점에서는 이런 코드가 나을 수도 있다는 생각이 드네요. 하나 배워갑니다!
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.
여기선 값이 있나 없나만 체크를 하기때문에 이렇게 코드를 적었는데 뭔가 애매하네요 ㅋㅋㅋ
import React, { useState } from 'react' | ||
import axios from 'axios' | ||
import { useNavigate } from 'react-router-dom' | ||
import { INPUTEMPTY } from '../constants/inputEmptyComment' |
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.
계속 궁금해서 코드 이곳저곳 뒤져봤는데, ../constants/inputEmptyComment'가 어디에 존재하는건가요?
상수를 뭐라 지정해둔건지 보고 싶었는데.. 깃헙에는 안 올리신건가요..?
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.
오 뭐지 gitignore에도 해당 폴더 올리지 말라는 코드는 없는데 왜 안올라갔을까요.. 알아보겠습니다
return { | ||
formInfo, | ||
idErrMessage, | ||
pwErrMessage, | ||
nErrMessage, | ||
phoneErrMessage, | ||
handleIdChange, | ||
handlePwChange, | ||
handleNicknameChange, | ||
handlePhoneChange, | ||
handleSignup, | ||
}; | ||
}; |
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.
커스텀 훅 사용으로 코드 분리해주는거 최고인 것 같습니다.
전 아직 잘 못 쓰는데.. 배워갑니다 ! 언젠가 저도 연습해봐야겠어요
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.
저도 처음해봤습니다.. 다음번엔 더 하나의 커스텀훅 파일이 하나의 기능만을 가지고 재사용 할 수 있도록 만들어봐야 할 것 같아요
handleNewPassword, | ||
handleNewPasswordVerification, | ||
handleChangePw, | ||
} = useMypage() |
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.
커스텀훅 사용은 다시봐도 .. 👍
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.
재사용하기 용이한 컴포넌트는 무엇인가?
에 대한 고민을 하고 계시는 것 같아 너무 좋습니다
컴포넌트를 재사용하는데에 props를 너무 많이 넘겨준다 라는 것은 이전 컴포넌트와 결합도가 높아질 수 있음을 의미합니다. 이럴 때 이 부분을 왜 재사용하는 가 에 대한 고민을 한 번 해볼 수 있겠지요
정답은 없고 상황에 따라 좋은 선택지는 있겠지요?
링크하나 공유합니다~
https://medium.com/@jang.wangsu/%EC%84%A4%EA%B3%84-%EC%9A%A9%EC%96%B4-%EC%9D%91%EC%A7%91%EB%8F%84%EC%99%80-%EA%B2%B0%ED%95%A9%EB%8F%84-b5e2b7b210ff
🧩 기본 과제
로그인 페이지
회원가입 페이지
마이페이지
🔥 심화 과제
메인페이지
로그인 페이지
회원가입 페이지
input이 비어있는 상태로 api연결 시도했을시
해당 input 테두리 색상 변경
input에 focus 맞추기
api요청 금지
전화번호 양식 정규표현식으로 자동입력되도록 설정 (숫자만 입력해도 "-"가 붙도록)
비밀번호 검증 유틸 함수 구현 (검증 통과되지 않을시 api요청 금지)
공유과제
링크 첨부(팀 블로그 링크) : 공유과제 링크
📌 내가 새로 알게 된 부분
위 코드에서 headers와 body의 위치를 처음에는 반대로 적었었는데 고수분들의 도움을 통해 api명세서대로 body를 먼저 선언하여 트러블 슈팅을 하였습니다.
💎 구현과정에서의 고민과정(어려웠던 부분) 공유!
props
값이 많아지더군요. 반복적으로 사용되는 컴포넌트를 빼더라도 좀 더 효율적으로 prop을 몇개 전달하지 않고도 사용할 수 있을만큼 나눠야 할 것 같습니다.🥺 소요 시간
24h
🌈 구현 결과물
구현 결과물 노션 링크