-
Notifications
You must be signed in to change notification settings - Fork 39
[이태식] sprint2 #100
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 #100
The head ref may contain hidden characters: "Basic-\uC774\uD0DC\uC2DD-sprint2"
Conversation
1배수 이미지를 2배수로 변경, 이름 수정, alt태그 수정, 필요없는 이미지 삭제
header의 position을 sticky로 설정
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.
태식님 2번째 스프린트 미션 수고하셨습니다!
심화 요구사항까지 모두 구현하시고 코드도 깔끔하네요 너무 잘하셨어요 👍
다음번 PR도 화이팅입니다!
-
commit 쪼개서 하신 것 너무 좋아요!
-
로그인 페이지와 회원가입 페이지의 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.
💊 제안
해당 이미지는 버튼으로 사용되긴하지만 이미지 자체는 아이콘이므로 ic_visibility_on.png와 같은 식으로 명명하시는 것을 추천드려요~
지금 프로젝트에서는 가능성이 없지만 이러한 아이콘 요소들은 버튼으로도, 이미지로도 사용될 수 있으니 이름자체에 btn, a 와 같은 태그명들이 들어갈 필요가 없습니다~
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.
💊 제안
이미지는 여백 없이 관리하시는 것이 디자인 구현할때 더 편리합니다.
이미지 자체가 여백을 가지고 있으면 헷갈릴 수 있으니 여백없이 추출하시는 것을 추천드려요!
|
|
||
| .keyword { | ||
| color: #3692FF; | ||
| color: var(--blue); |
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.
👍 칭찬
색상 변수 추가하시고 사용하신 것 좋습니다 👍
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를 분리해주신 점 좋습니다~
다만 색상변수는 모든 페이지에서 공통으로 사용되는 것이니 utility라는 파일명에 어울리지 않는 것 같아요~
reset.css 파일중에서도 아래내용의 경우 "브라우저 스타일을 초기화" 해주는 파일의 분리 기준에 어울리지 않습니다~
* {
box-sizing: border-box;
}
html {
font-family: Pretendard, sans-serif;
font-size: 16px;
}색상변수와 위의 스타일들의 공통점은 모든 페이지에 걸쳐 공통으로 적용되는 스타일이라는 것이므로 이러한 기준으로 파일을 분리하시는 것을 추천드려요~ 이름은 common.css, base.css 정도면 좋을 것 같습니다~
| <img src="images/btn_visibility_on.png" alt="눈뜸"> | ||
| </div> | ||
| <button class="btn-large text-xl semibold">회원가입</button> | ||
| </form> |
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.
👍 칭찬
form 태그로 관련된 요소만 묶어 주신 것 좋습니다~
| <div class="input-group"> | ||
| <label for="checkPassword" class="bold">비밀번호 확인</label> | ||
| <input type="text" id="checkPassword" class="text-lg regular"> | ||
| <img src="images/btn_visibility_on.png" alt="눈뜸"> |
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.
❗️ 수정요청
해당 요소는 클릭 시 비밀번호가 보이고 안 보이게 만드는 요소가 될 것이므로 button요소로 작성하시는 것을 추천드려요.
| <label for="checkPassword" class="bold">비밀번호 확인</label> | ||
| <input type="text" id="checkPassword" class="text-lg regular"> |
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과 Input을 연결해주신 점 좋습니다~
| <input type="text" id="checkPassword" class="text-lg regular"> | ||
| <img src="images/btn_visibility_on.png" alt="눈뜸"> | ||
| </div> | ||
| <button class="btn-large text-xl semibold">회원가입</button> |
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.
💬 여담
button 태그의 type 속성을 명시하지 않으면 submit을 기본값으로 가지게 됩니다~
이러한 type 속성은 button 태그에 중요한 요소이므로 명시적으로 작성해주는 것을 추천드려요.
| <button class="btn-large text-xl semibold">회원가입</button> | |
| <button class="btn-large text-xl semibold" type="submit">회원가입</button> |
+type이 submit인 button의 경우 form에서 제출 이벤트가 발생하면, 해당 요소의 onclick 이벤트가 실행되게 됩니다.
이를 더 풀어서 말하자면 form 태그안의 input, button 과 같은 상호작용 요소에서 submit 이벤트, enter key 입력이 발생하면 해당 form 에서의 submit 이벤트가 실행된다는 것입니다.
자세한 것은 추후 학습 하시면서 배우실 것이니 우선은 button 태그의 type 속성을 명시적으로 작성하는 것이 좋다는 것만 기억해주시면 좋겠습니다~
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 파일을 작성하시면 유지보수 및 가독성 측면에서 더 좋을 것 같습니다.
auth.css // login, signup 페이지에서 공통으로 쓰는 것들의 모음
login.css
signup.css
만약 login.css, signup.css 파일에서 작성되는 css가 너무 적다면, 즉 분리할 필요를 못 느끼신다면 auth.css 하나에서 다 작성하셔도 좋을 것 같아요~
요구사항
기본
공통
로그인 페이지
회원가입 페이지
심화
주요 변경사항
스크린샷
https://leetaesik-sprint.netlify.app/

멘토에게