-
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주차 기본/공유 과제 ] #4
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.
전체적으로 깔끔하게 코드 작성하신 게 느껴지네요. axios
를 통한 API request를 많이 접해보시지 않았음에도 불구하고, 고민해보신 흔적이 남아있어서 좋습니다. 고생하셨고 합세도 화이팅입니다. !
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.
안쓰는 파일 지워주면 좋을 것 같네요 !
return ( | ||
<LogInStyled> | ||
<h1>Login</h1> | ||
<img src={props.src}></img> |
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.
img의 alt
속성 넣어주시면 더 좋을 것 같네요 ㅎㅎ. 접근성과 SEO 측면에서 좋을 것 같습니다 !
<input type="text" value={userId} onChange={(e) => setUserId(e.target.value)} /> | ||
<p>PW </p> | ||
<input type="text" value={userPw} onChange={(e) => setUserPw(e.target.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.
onChange
로 setter
를 바로 넘기는 것 보다, handle
로 시작하는 이벤트 핸들러 함수를 만들어 연결해주는 것이 더욱 가독성 측면에서 좋을 것 같다고 생각이 드네요 !
<input type="text" value={nowPw} onChange={(e) => setNowPw(e.target.value)} /> | ||
<p>새로운 비밀번호</p> | ||
<input type="text" value={newPw} onChange={(e) => setNewPw(e.target.value)} /> | ||
<p>비밀번호 확인</p> | ||
<input type="text" value={confirmPw} onChange={(e) => setConfirmPw(e.target.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.
특정 정보를 입력하기 위한 input
태그가 존재한다면, 해당 정보의 제출을 위한 form
태그도 꼭 써주는 것을 추천드려요 ! method
, action
, onSubmit
등의 프로퍼티들과 같이, 특정 정보들의 제출과 관련된 api
또는 핸들러를 쉽게 연결해줄 수 있습니다.
또한 접근성과 가독성 측면에서도 더 좋을 것 같네요
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.
https://jsdev.kr/t/form/5468/2 요것도 한 번 읽어보시면 좋을 것 같습니다!
const submitBtn = async () => { | ||
try { | ||
const response = await axios.post(`${baseURL}/member/join`, { | ||
authenticationId: userId, | ||
password: userPw, | ||
nickname: userName, | ||
phone: number, | ||
}); | ||
navigate('/login'); | ||
alert(response.data.message); | ||
return response.data; | ||
} catch (e) { | ||
alert(e.response.data.message); | ||
} | ||
}; |
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.
핸들러이기 때문에, 이전에 한서님이 작성하신 것처럼 handle
로 시작하게끔 통일시키는게 좋아보이네요 !
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 base_URL = 'http://34.64.233.12:8080'; | ||
|
||
const LogIn = (props) => { |
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에 src밖에 없어보이는데 이 부분은 구조분해할당으로 src라는 변수로 바로 받아오면 더 좋을 것 같습니다!
props.src
보다는 src
가 코드가 더 간결해보이니까요!
|
||
export default LogIn; | ||
|
||
const LogInStyled = 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.
style 관련된 코드는 Login.style.js 파일로 분리시키면 가독성이 더 좋을 것 같아요 !! 어떻게 생각하시나요?
const [isOpen, setIsOpen] = useState(false); | ||
const navigate = useNavigate(); | ||
const toggleModal = () => { | ||
setIsOpen((isOpen) => !isOpen); |
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.
isOpen 상태변수랑 이름이 겹쳐서 혼란을 줄 수 있을 것 같아요!
prevIsOpen 이라는 변수명 사용에 대해서 어떻게 생각하시나요?
return; | ||
} | ||
try { | ||
const pwData = await axios.patch( |
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.
api 받아오는 부분은 api 폴더에서 따로 관리해주는 게 함수형 프로그래밍의 취지와도 맞을 것 같아요!
어떻게 생각하시나요?
<p>ID : {user.data?.authenticationId}</p> | ||
<p>닉네임 : {user.data?.nickname} </p> | ||
<p>전화번호 : {user.data?.phone}</p> | ||
<span onClick={() => toggleModal()}>▾ 비밀번호 변경</span> |
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.
p태그, span태그등 시맨틱 태그를 정말 잘 활용하신 것 같아요!
저는 구현에 급급해서 div로 다 일관했는데 반성하고 갑니다 ㅎㅎ
✨ 구현 기능 명세
🧩 기본 과제
로그인 페이지
회원가입 페이지
마이페이지
🔥 심화 과제
메인페이지
로그인 페이지
회원가입 페이지
input이 비어있는 상태로 api연결 시도했을시
해당 input 테두리 색상 변경
input에 focus 맞추기
api요청 금지
전화번호 양식 정규표현식으로 자동입력되도록 설정 (숫자만 입력해도 "-"가 붙도록)
비밀번호 검증 유틸 함수 구현 (검증 통과되지 않을시 api요청 금지)
공유과제
링크 첨부(팀 블로그 링크) :
https://forweber.palms.blog/eslint-prettier-hs
📌 내가 새로 알게 된 부분
💎 구현과정에서의 고민과정(어려웠던 부분) 공유!
데이터를 화면에 불러오는 건 되는데 다른 부분을 구현하려고 하자 화면 렌더링이 계속 안되고 untyped caught error가 계속 떠서 물음표 연산자로 해결했다.
- useParams() 이용하는 법
🥺 소요 시간
🌈 구현 결과물
https://chill-lemon-56c.notion.site/4-768db5395fcb4eaaa029612976ce7398?pvs=4