-
Notifications
You must be signed in to change notification settings - Fork 39
[유원규] Sprint4 #106
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
[유원규] Sprint4 #106
The head ref may contain hidden characters: "Basic-\uC720\uC6D0\uADDC-sprint4"
Conversation
- 타이틀 수정 - 불필요한 주석 삭제 - 오픈그래프 메타태그 옳바른 주소값 수정 - 가독성을 위한 들여쓰기 추가 - min-width 삭제
- 각 폴더에 js파일 추가 - 로그인,회원가입페이지에 이메일입력란 오류메세지추가 - css 인풋 높이 확장
- 로그인 유효성 확인 및 css 색상 변경 - 비밀번호 눈모양 클릭식 비밀번호 보기/가리기 가능하게 변경
- 아직 미완성됨 - 회원가입이 회색에서 안돌아옴 - 이메일, 니게임, 비밀번호, 비밀번호 확인, 회원가입버튼 - 순서대로 작성함 - toggleSignupButton(); 를 넣었음 : 버튼 상태 업데이트해주는 키 - 되는지 확인중
- 로그인, 회원가입 버튼의 서미트를 버튼으로 수정 - html의 script위치를 수정
GANGYIKIM
left a comment
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.
원규님 4번째 PR 고생하셨습니다!
점점 더 어려워지신다고 하셨는데도 모든 요구사항을 구현하셨네요~
새로운 지식을 학습하고 바로 실습하는 것이 쉬운 일은 아니라 괴로우실 수는 있지만 잘 하고 계십니다.
너무 어려우시다면 어떤것이 어려우신지 DM으로 얘기해봐요~
다음 PR도 화이팅입니다!
- 원규님 올려주신 배포주소에 페이지가 보이지 않네요! 한번 확인해보세요.
- PR 올리실 때 base branch를 main이 아닌 Basic-유원규로 해서 올려주세요~
- 제 코멘트는 필수반영사항은 아니니 반영하기 어려우시거나 다른 방식이 좋으시면 편하게 작업해주셔도 됩니다~
| :root { | ||
| --primary-0: #e6f2ff; | ||
| --primary-50: #cfe5ff; /* background */ | ||
| --primary-100: #3692ff; /* default */ | ||
| --primary-200: #1967d6; /* hover */ | ||
| --primary-300: #1251aa; /* active, focus */ | ||
| --secondary-50: #f9fafb; | ||
| --secondary-100: #f3f4f6; /*input bg*/ | ||
| --secondary-200: #e5e7eb; | ||
| --secondary-300: #fcfcfc; /* card bg, bottom banner bg */ | ||
| --secondary-350: #dfdfdf; | ||
| --secondary-400: #9ca3af; /* inactive */ | ||
| --secondary-500: #6b7280; | ||
| --secondary-600: #4b5563; | ||
| --secondary-700: #374151; | ||
| --secondary-800: #1f2937; | ||
| --secondary-900: #111827; /* footer bg*/ | ||
| --error: #f74747; | ||
| } | ||
| * { | ||
| box-sizing: border-box; | ||
| flex-wrap: wrap; | ||
| font-size: 10px; | ||
| font-family: 'Pretendard'; | ||
| } |
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.
❗️ 수정요청
해당 부분이 base.css와 동일하니 index.css에서 하신 것처럼 import해오시면 더 좋을 것 같아요!
| :root { | |
| --primary-0: #e6f2ff; | |
| --primary-50: #cfe5ff; /* background */ | |
| --primary-100: #3692ff; /* default */ | |
| --primary-200: #1967d6; /* hover */ | |
| --primary-300: #1251aa; /* active, focus */ | |
| --secondary-50: #f9fafb; | |
| --secondary-100: #f3f4f6; /*input bg*/ | |
| --secondary-200: #e5e7eb; | |
| --secondary-300: #fcfcfc; /* card bg, bottom banner bg */ | |
| --secondary-350: #dfdfdf; | |
| --secondary-400: #9ca3af; /* inactive */ | |
| --secondary-500: #6b7280; | |
| --secondary-600: #4b5563; | |
| --secondary-700: #374151; | |
| --secondary-800: #1f2937; | |
| --secondary-900: #111827; /* footer bg*/ | |
| --error: #f74747; | |
| } | |
| * { | |
| box-sizing: border-box; | |
| flex-wrap: wrap; | |
| font-size: 10px; | |
| font-family: 'Pretendard'; | |
| } | |
| @import url(base.css); |
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.
💊 제안
기존에 로그인과 회원가입에서 하나의 css를 사용하시다가 왜 분리하신걸까요? 해당 페이지들은 유사한 부분이 많아 css에서 중복이 많아서 그전 방식이 더 좋아보입니다~ 특별한 이유가 없으시면 그전처럼 합치시는 것을 추천드려요!
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.
💬 여담
signup.js 와 signin.js 파일에서 중복되는 로직들이 있어보여요!
이를 어떻게 줄일 수 있을 지 고민해보시면 실력향상에 도움이 많이 되실거에요!
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.
💊 제안
우선적으로 페이지에 종속되지 않는 이메일 확인 정규식과 테스트 로직을 분리하시는 것을 추천드려요.
일반적으로 정규식들은 여러 페이지에서 사용되니까요!
| function isValidPassword(password) { | ||
| return password.length >=8; | ||
| }; | ||
| //비밀번호 재확인 | ||
| document.getElementById('checkpassword').addEventListener('blur', function(){ | ||
| checkPasswordMatch(); | ||
| validateForm(); | ||
| }); |
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.
💊 제안
한 파일에서 최상단에 변수 선언들을 위치시키는 것처럼 일정한 순서로 코드가 작성되는 것이 가독성 측면에서 좋습니다~
지금 변수와 이벤트 바인딩문이 번갈아가며 작성되어 있고 그렇게 위치해야하는 이유도 잘 모르겠어요~
가능하면 이벤트 바인딩문은 변수 선언문보다 하단에 위치시키는 것을 추천드립니다!
| //비밀번호 확인 | ||
| function checkPasswordMatch() { | ||
| const passwordInput = document.getElementById('password'); | ||
| const checkpassword = document.getElementById('checkpassword'); |
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 checkpassword = document.getElementById('checkpassword'); | |
| const checkPasswordInput = document.getElementById('checkpassword'); |
| validateForm(); | ||
| }); | ||
| //비밀번호 | ||
| document.getElementById('password').addEventListener('blur', function(){ |
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.
💊 제안
회원가입 페이지에서 비밀번호 입력 후 비밀번호 확인 입력하고 다시 비밀번호를 변경하면 비밀번호 확인 인풋은 재확인 로직을 실행하지 않네요~ 해당 로직에서 비밀번호 확인 인풋 값도 재검증하면 해결될 것 같습니다~
| if (emailValid && usernameValid && passwordValid && passwordsMatch) { | ||
| signupButton.disabled = false; | ||
| } else { | ||
| signupButton.disabled = true; | ||
| } |
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.
💬 여담
이런경우 아래처럼 작성하셔도 됩니다~
| if (emailValid && usernameValid && passwordValid && passwordsMatch) { | |
| signupButton.disabled = false; | |
| } else { | |
| signupButton.disabled = true; | |
| } | |
| const isFormValid = emailValid && usernameValid && passwordValid && passwordsMatch; | |
| signupButton.disabled = !isFormValid; |
요구사항
기본
로그인
이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.
비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다
비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.
input 에 빈 값이 있거나 에러 메세지가 있으면 ‘로그인’ 버튼은 비활성화 됩니다.
input 에 유효한 값을 입력하면 ‘로그인' 버튼이 활성화 됩니다.
활성화된 ‘로그인’ 버튼을 누르면 “/items” 로 이동합니다
회원가입
이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.
닉네임 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “닉네임을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다
비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.
비밀번호 input과 비밀번호 확인 input의 값이 다른 경우, 비밀번호 확인 input 아래에 “비밀번호가 일치하지 않습니다..” 에러 메세지를 보입니다.
input 에 빈 값이 있거나 에러 메세지가 있으면 ‘회원가입’ 버튼은 비활성화 됩니다.
input 에 유효한 값을 입력하면 ‘회원가입' 버튼이 활성화 됩니다.
활성화된 ‘회원가입’ 버튼을 누르면 “/signup” 로 이동합니다
심화
주요 변경사항
스크린샷
https://pandanara99.netlify.app/
멘토에게