-
Notifications
You must be signed in to change notification settings - Fork 35
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 #120
The head ref may contain hidden characters: "Basic-\uC5FC\uC131\uC9C4-sprint4"
[염성진] Sprint4 #120
Conversation
* 기존에 Script로 불러오던 폰트를 Link태그를 사용하여 불러오게 변경하였습니다. (모든 html파일, css파일에 적용) * 메인 페이지 footer 태그가 2개여서 좀 더 시맨틱 한 구조를 위해 하단부 html 태그명, class명, 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.
기본적으로 성진님이 예측할 수 있는 에러(value 값이 빈 string이던지, null 값이라던지 등등)들은 조건을 통해서 에러처리해줄 수 있을 것 같네요!
예측할 수 없는 에러가 있다는 것은 코드가 잘못되었을 확률이 높아요! 우리는 우선 예측할 수 있는 에러처리만 꼼꼼하게 해주는 것이 가장 좋아요 👍
그 외 성진님 코드 스타일이 너무 깔끔하고 좋네요!
다만 중복 if문만 한번 더 고민해보시면 좋을 것 같습니다
이번주도 화이팅!
<nav class="sns-links"> | ||
<ul> | ||
<li> | ||
<a href="/facebook" target="_blank"> |
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.
targe="_blank"
는 rel="noopener norefferer"
와 함께 사용됩니다(거의 세트로)
이유는 아래 아티클로 첨부해드릴게요!
https://velog.io/@parkseonup/noopener%EC%99%80-noreferrer
const $conditionMessage = document.querySelectorAll('.condition-message'); | ||
const $passwordEye = document.getElementById('passwordEye'); | ||
const $checkPasswordEye = document.getElementById('checkPasswordEye'); | ||
const pattern = /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/; |
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.
이런 정규표현식 혹은 변하지 않는 값들은 "상수화" 시켜주는 것이 일반적인 방법이에요.
예를 들어 PATTERN 이라는 대문자 변수명을 사용하면 우린 보통 상수화 시켜줬다 라고 생각합니다!
*참고
대문자 상수 : 기억하기 힘든 값을 변수에 할당해 별칭으로 사용되는 관습으로 대문자 상수는 ‘하드 코딩한’ 값의 별칭을 만들 때 사용
const $target = e.target; | ||
const $errorMesaage = $target.nextElementSibling; | ||
if ($target.value === '') { | ||
$target.classList.add('error2'); |
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.
전 훨씬 좋다고 생각해요!
dom에 직접적인 style을 지정해주는 것이 아닌 classList 추가를 해줌으로써 더 직관적인 코드가 된 것 같아요!
// 이메일 정규식 체크 | ||
function emailCheck() { | ||
if ($email.value === '') { | ||
$conditionMessage[0].classList.remove('waring'); |
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.
하지만 결국 conditionMessage 처럼 querySelector로 가져온 친구들도 DOM 자체이기 때문에 어쨌든 DOM에 클래스명을 추가하는 것이에요 :)
다만, style 속성을 직접 부여하지 않고 class명을 지정해줌으로써 다양한 css를 적용하기 더 편해졌다는 장점이 있죠 👍
function passwordCheck(e) { | ||
if (e.target.id === 'password') { | ||
const $target = e.target; | ||
const $Message = $target.nextElementSibling.nextElementSibling; |
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 meetsCondition() { | ||
for (let i = 0; i < $input.length; i++) { | ||
if ($input[i].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.
const isError = $input[i].classList.contains('error') || $input[i].classList.contains('error2')
pass.splice(i, 1, isError ? false : true)
요런식으로 코드를 바꿔볼 수 있겠네요
중첩 if문이 나온다면 다시 한번 고민해보는 것을 추천할게요!
제가 위에서 사용한 건 "삼항 연산자"입니다!
function meetAallConditions() { | ||
if ($input.length === 2) { | ||
const loginPass = pass.slice(0, 2); | ||
if (loginPass.every((value) => value === 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문이 중복으로 쓰였는데 다른 방법은 없을까요!?
|
||
// 모든 조건 만족 시 submit 하여 페이지 이동 | ||
$loginForm.addEventListener('submit', (e) => { | ||
e.preventDefault(); |
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.
혹시 e.preventDefault() vs e.stopPropagination()의 차이점을 아시나요?
모르신다면 아래 아티클을 공부해보시면 좋을 것 같아요 👍
https://velog.io/@yiyb0603/JS-e.preventDefault%EC%99%80-e.stopPropagation%EC%9D%98-%EC%B0%A8%EC%9D%B4%EC%A0%90
meetsCondition(); | ||
meetAallConditions(); | ||
if ($loginBtn[0].classList.contains('accept')) { | ||
if ($input.length === 2) { |
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 | ||
type="password" | ||
placeholder="비밀번호를 입력해주세요" | ||
class="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.
클래스명은 태그명과 다른 것이 좋아요!
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게
이런 경우 추천하시는 방법이나 패턴이 있을까요?