-
Notifications
You must be signed in to change notification settings - Fork 30
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
[김성빈] Sprint2 #17
The head ref may contain hidden characters: "Basic-\uAE40\uC131\uBE48-sprint2"
[김성빈] Sprint2 #17
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.
성빈님 이번에도 잘 완성해주셨군요! 👍
BEM을 적용해주신 점이나, js를 벌써 적용하신 점 등 빠르게 잘 학습하고 계신거 같습니다 :)
함수명, 클래스명, 파일명 등 이름 짓는 거에 조금만 더 신경 써주시면 좋을 거 같아요!
<head> | ||
<meta charset="UTF-8" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<title>login</title> | ||
<title>판다마켓 | 로그인</title> | ||
<link rel="stylesheet" href="src/assets/css/main.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.
login page에서 사용되지 않는 스타일이 많이 포함되어있습니다!
페이지별로 스타일을 나누거나 공통 부분을 따로 빼보시면 좋을 거 같아요 :)
</div> | ||
</div> | ||
</div> | ||
<div class="container"> | ||
<section class="main-section"> | ||
<div class="main-section__inner"> | ||
<div class="main-section__left main-section__left--image"> | ||
<div class="main-section__left main-section__imagewrap"> |
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.
bem을 잘 적용해주셨네요! 👍
</h1> | ||
<form class="form"> | ||
<div class="form__group"> | ||
<label for="email">이메일</label> |
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.
label을 잘 적용해주셨네요! 👍
class="form__input" | ||
placeholder="비밀번호를 입력해주세요" | ||
/> | ||
<div class="visible-icon"></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.
아이콘에 대한 정보가 부족해 보여요!
aria-label 등을 통해서 접근성을 높여주시면 좋습니다 :)
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.
벌써 js를 작성해주셨군요~!
파일명을 좀 더 의미있게 작성해주시면 좋을 거 같습니다 :)
}); | ||
}); | ||
|
||
function Check() { |
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.
일반적으로 js는 카멜 케이스를 많이 사용합니다!
function check {
}); | ||
}); | ||
|
||
function Check() { |
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.
함수명은 명확하게 지어주시는 것이 좋아요! 이 함수는 어떤 것을 check하는 건가요!?
@@ -0,0 +1,30 @@ | |||
let visible_icon = document.querySelectorAll(".visible-icon"); |
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를 씁니다 :)
@@ -0,0 +1,30 @@ | |||
let visible_icon = document.querySelectorAll(".visible-icon"); |
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.
일반적으로 js는 카멜 케이스를 많이 사용합니다! const visibleIcons =
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게