-
Notifications
You must be signed in to change notification settings - Fork 56
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
[김미진]week4 #156
base: part1-김미진
Are you sure you want to change the base?
The head ref may contain hidden characters: "part1-\uAE40\uBBF8\uC9C4-week4"
[김미진]week4 #156
Conversation
- label태그의 for값과 input태그의 id값 수정 - form태그안에 div태그의 클래스명 수정 - 파일경로 수정
refactor(mentor): - / 제거했습니다
ade9761
to
f8d1729
Compare
week4/js/signin.js
Outdated
validate(emailPattern, pwPattern); | ||
} | ||
|
||
function validate(emailPattern, pwPattern){ |
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.
validate
함수는 formBox.forEach
외부에 있어야합니다. 이렇게 되면 el
갯수만큼 해당 함수가 재선언되기 때문에 불필요한 메모리 사용이 됩니다.
제일 좋은 방법은 해당 validate
함수를 utils.js
라는 파일을 만들어서 내부에 선언해주고, 해당 함수를 export 해주면 좋겠죠.
week4/js/signin.js
Outdated
if(e.target.value === ''){ | ||
e.currentTarget.classList.add('active'); | ||
|
||
el.classList.contains('email') ? e.currentTarget.lastElementChild.textContent = '이메일을 입력하세요' : e.currentTarget.lastElementChild.textContent = '비밀번호를 입력하세요'; |
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.
서비스를 운영하는 입장에서 보면 현재 html 구조로 고정된다는 보장이 없습니다. 그렇다면 parentElement
, lastElementChild
와 같이 노드의 상대적인 구조를 파악하는 코드는 html 구조가 변경된다면 의도하지 않은 동작이 일어날 가능성이 높겠죠.
애초에 input
에 대한 제어를 깔끔하게 하고싶다면, 각 input
태그에 공통적인 class 명을 달아주시면 됩니다.
week4/js/signin.js
Outdated
}) | ||
|
||
// 눈 아이콘 클릭 시 비밀번호 안보이게 하기 | ||
function eyeOn(e){ |
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
에서도 쓰이는걸로 보아 공통 함수로 다른 파일(ex. utils.js
)에서 선언해주면 좋을 것 같네요.
|
f8d1729
to
a64ecbf
Compare
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게