[김성주] Sprint1 #8
Conversation
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨습니다!
꽤 자주 동일한 선택자에서 같은 스타일을 중복선언하시는데, 피드백 보시고 리팩토링 진행해보시면 좋을것같습니다 :)
주요 리뷰 포인트
- CSS 우선순위 관련 수정
- 클래스이름 계층 위계 관련 수정 & CSS 활용
index.html
Outdated
| <nav class="nav-side-margin"> | ||
| <a href="./"> | ||
| <img class="logo" src="./images/logo.png" alt="판다마켓로고"> | ||
| </a> | ||
| <a class="login-btn" href="./login">로그인</a> | ||
| </nav> |
There was a problem hiding this comment.
계층간의 위계가 잘 보이지 않는것 같아요. 이런식으로 리팩토링해보면 어떨까요?
nav__container, nav__logo, nav__button
그러면 css를 작성할때도 이렇게 명확하고 직관적이게 작성하실수있을거예요!
.nav {
&__container {
}
&__logo {
}
&__button {
}
}
style.css
Outdated
| .w1120 { | ||
| width: 1120px; | ||
| margin: 0 auto; | ||
|
|
||
| width: 70rem; | ||
| } |
There was a problem hiding this comment.
width 속성이 두번 쓰였네요! 이렇게 동일한 선택자 내에서 같은 속성이 중복 선언된 경우 나중에 작성된 속성이 우선순위를 가지므로, width: 70rem이 최종적으로 적용됩니다.
이유는, CSS는 cascading style sheet라서 "cascade(계단식)" 특성을 가지기때문입니다.
일반적인 우선순위는 이렇습니다.
- !important 선언
- 인라인 스타일 (style 속성을 사용해 부여)
- 선택자 명시도 (ID > 클래스 > 요소)
- 소스 코드 순서 (같은 명시도인 경우 나중에 선언된 것이 우선)
CSS 우선순위에 대해서 이 기회에 더 공부해보시면 좋을것같네요 :)
index.html
Outdated
| </header> | ||
| <main> | ||
| <section class="main-img-area"> | ||
| <article class="main-img-wrap w1120"> |
There was a problem hiding this comment.
클래스이름에 구체적으로 너비를 명시해주는게 어색해보이긴하네요.
width: 1120px의 너비를 가진 스타일 규칙을 재사용하고싶은 목적이라면 container 정도로 네이밍해주는건 어떨까요?
예를 들어 BEM 규칙을 사용해 클래스이름을 지어주고 지금과 같은 유틸리티 클래스를 함께 사용할수도있는데,
main-img-wrap container 이런식으로 같이 사용해주거나, css 변수를 같이 사용하게되면 일관된 방식으로 CSS를 작성하면서도 코드 중복을 줄이고 재사용성을 높일수있답니다 :)
| margin: 0; | ||
| padding: 0; | ||
| box-sizing: border-box; | ||
| } |
There was a problem hiding this comment.
7번째 라인 및에 공백 한칸 넣어주세요! 포맷팅 신경써보면 좋겠네요 :)
| flex-grow: 1; | ||
| height: 51px; | ||
|
|
||
| height: 3.1875rem; |
There was a problem hiding this comment.
여기서도 동일한 선택자 아래에서 같은 속성이 중복으로 선언됐네요!
style.css
Outdated
| .login-btn { | ||
| display: inline-block; | ||
| padding: 12px 23px; | ||
| background-color: #3692FF; |
There was a problem hiding this comment.
이렇게 반복해서 사용되는 값들은 css 변수로 추출해볼까요?
예시
:root {
/* 컨테이너 관련 변수 */
--container-width: 1120px;
--container-padding: 1rem;
/* 간격 관련 변수 */
--spacing-xs: 0.5rem;
--spacing-sm: 1rem;
--spacing-md: 2rem;
--spacing-lg: 3rem;
/* 색상 변수 */
--primary-color: #3692FF;
--primary-hover: #1251AA;
--background-color: #CFE5FF;
--text-color: #374151;
}| width: 46.625rem; | ||
| height: 21.25rem; |
기본 체크리스트 중 4번을 제대로 이해한 것인지 모르겠습니다.1920보다 작아지면 바로 200px로 유지하는 것인지
즉 기본 여백을 항상 200px로 유지하면서 전체 nav를 가운데정렬시키고 화면이 작아질때는 두개 아이템 간격이 가까워지게 만드시면 됩니다. 심화 체크리스트 구현을 위해서 가변단위로 덮어서 작성했습니다. 관련된 스타일 설정이 같은 곳에 있는 게 나중에 가변단위를 다시 수정할 때 편하다고 생각해서 이렇게 작성을 했는데 안 좋은 습관인지 궁금합니다.아하, 왜 중복 선언을 자주하셨나했는데 덮어서 작성하시려는 의도였군요. |
- 선택자들 위계 잘 지켜서 BEM방식으로 이름 다시 - :root 포멧팅(들여쓰기) 오타 수정 - 기본 체크사항 4번 말씀해주신대로 동작하도록 수정 - w1120 이름 container로 변경 - 색상변수에 #fff 추가
2 participants
요구사항
기본
심화
스크린샷
배포
https://sprint-mission-pandamarket.netlify.app/
멘토에게
기본 체크리스트 중 4번을 제대로 이해한 것인지 모르겠습니다.
우선 2번으로 했는데 잘못 이해한 거면 수정하도록 하겠습니다.
심화 체크리스트 구현을 위해서 가변단위로 덮어서 작성했습니다. 관련된 스타일 설정이 같은 곳에 있는 게 나중에 가변단위를 다시 수정할 때 편하다고 생각해서 이렇게 작성을 했는데 안 좋은 습관인지 궁금합니다.
셀프 코드 리뷰를 통해 질문 이어가겠습니다.