-
Notifications
You must be signed in to change notification settings - Fork 42
[이형탁] sprint2 #152
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
The head ref may contain hidden characters: "Basic-\uC774\uD615\uD0C1-sprint2"
[이형탁] sprint2 #152
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.
수고하셨습니다!
주요 리뷰 포인트
- 코드 중복 제거
- 포맷팅
- id 선택자 사용 관련 피드백
* { | ||
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
body{ | ||
font-family: 'Pretendard-Regular'; | ||
font-size: 16px; | ||
} |
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 파일을 구분해서 써볼까요?
- reset.css : CSS 초기화
- common.css: 전역 스타일 처리
display: flex; | ||
justify-content: center; | ||
align-items: center; |
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.
이런 자주 사용되는 스타일 블락들은 유틸리티 클래스로 만들어서 재사용해주면 코드 중복이 줄어들어서 더 괜찮겠죠?
.flex-center {
display: flex;
justify-content: center;
align-items: center;
}
} | ||
|
||
input{ | ||
border: none; |
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.
이런 초기화 코드는 제가 코멘트드린것처럼 reset.css 파일에서 선언하고 재사용하면 매번 다시 코드를 반복할 필요가 없어질거예요 :)
} | ||
|
||
.btn_off, .btn_on { | ||
border: none; |
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.
요것도 마찬가지! reset.css 파일에서 기본 스타일을 초기화해주면 따로 써줄 필요가 없어지겠죠?
@@ -0,0 +1,142 @@ | |||
@charset "utf-8"; | |||
@font-face { |
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.
font 최적화를 시도할수있겠군요!
아래 아티클 참고해보시고, font-display 속성도 건드려볼까요?
#login_main { | ||
width: 100%; | ||
height: 100vh; | ||
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
} | ||
|
||
#login_main .container { | ||
width: 640px; | ||
display: flex; | ||
flex-direction: column; | ||
justify-content: center; | ||
align-items: center; | ||
} |
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에서 login 페이지에 관련된 CSS가 필요한 이유가 있을까요?
} | ||
|
||
/* ---------------------- banner_bottom --------------------- */ | ||
#banner_bottom { |
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.
header, banner다 id 선택자를 사용하셨는데, id로 관리하기엔 네이밍이 너무 일반적이고 id로 관리하는것보다는 클래스로 바꿔주는게 훨씬 적절할것같아요. 이유는 id는 특성상 페이지에서 단 한 번만 사용해야하는데, 불필요하게 ID를 많이 사용하면 해당 요소의 스타일이나 기능을 다른 요소에 재사용하기 어려워지고, 일반 선택자보다 훨씬 높은 명시도를 가지기때문에
나중에 스타일을 재정의하기 어려워지고, 결국 !important 사용을 하게 만들수 있습니다. 참고해보시고, 꼭 필요할때가 아니면 id 선택자를 사용하지않는 방법을 써봐요 :)
질문에 대한 답변
본문 내에 상세히 피드백 드렸습니다 :) |
요구사항
기본
심화
주요 변경사항
스크린샷
https://htak0601.netlify.app/


멘토에게
아직 태그나 css정리가 안되는 것 같습니다. 피드백 부탁드립니다.
셀프 코드 리뷰를 통해 질문 이어가겠습니다.