[배수민] Sprint1 #10
[배수민] Sprint1 #10
Conversation
|
netlify 배포 주소: |
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨습니다! 조금만 더 다듬어봐요 :)
주요 리뷰 포인트
- 클래스이름을 더 잘 지어주기
- 공통된 스타일 추출 / 값 추출해서 재사용하기 (코드 중복 줄이고 유지보수 신경쓰기)
index.html
Outdated
| <!-- 상단 배너 --> | ||
| <section class="banner"> | ||
| <div class="banner-content"> | ||
| <div class="banner-left"> |
There was a problem hiding this comment.
클래스이름에 위치, 색상 등의 키워드를 포함해 네이밍을 사용하시는건 적절치않아요.
이유는 디자인이 변경된다면 실제 스타일과 클래스명이 불일치할 수 있고, 이는 곧 재사용성의 저해로 이어지기때문입니다.
따라서, 다음부터는 해당 엘리먼트의 목적과 역할을 보다 직관적으로 이해할 수 있게끔 염두에 두고 작업해보시는걸 추천드립니다 :)
There was a problem hiding this comment.
top-img, bottom-img 도 마찬가지!
index.html
Outdated
| <div class="section-text text-right"> | ||
| <span class="section-point">Search</span> | ||
| <span class="section-title">구매를 원하는<br>상품을 검색하세요</span> | ||
| <span class="section-subtitle">구매하고 싶은 물품은 검색해서<br>쉽게 찾아보세요</span> | ||
| </div> |
There was a problem hiding this comment.
전체적으로 굉장히 일반적인 클래스이름을 사용했고, 계층구조가 명확히 보이지않는 단점이 있는것같아요.
이런식으로 네이밍해주는건 어떨까요?
.feature > .feature__content > .feature__content__title, .feature__content__subtitle
좀더 계층 관계가 명확히 드러나면서도 내부 요소들을 쉽게 식별할수있지않을까요?
There was a problem hiding this comment.
예시로 들어주신 내용 반영하여 수정하였습니다!
index.html
Outdated
| <header> | ||
| <div class="header-content"> | ||
| <a href="/"><img src="img/logo.png" alt="판다마켓 로고" class="logo"></a> | ||
| <a href="/login" class="login-btn">로그인</a> |
There was a problem hiding this comment.
login-btn의 경우에도 특정 기능에 종속되어있어 재사용이 어려울수있어요 :)
그리고 items-btn과 겹치는 스타일이 꽤 있는것같은데, 공통 btn 스타일을 추출해서 재사용성을 높이는 방식으로 개선해볼까요? 이건 css 파일에서 코멘트 드려볼게요!
There was a problem hiding this comment.
공통 버튼 스타일 추출하여 수정하였습니다!
style.css
Outdated
| color: #374151; | ||
| } | ||
|
|
||
| .items-btn { |
There was a problem hiding this comment.
공통적으로 button에 적용되는 스타일을 아래 예시와 같이 추출해주고
.btn {
display: flex;
justify-content: center;
align-items: center;
text-decoration: none;
font-weight: 500;
border: none;
cursor: pointer;
transition: background-color 0.2s ease;
}만약, 변형된 버전이 사이즈에 의해 생긴다면:
.btn-small {
padding: 8px 16px;
height: 36px;
min-width: 100px;
font-size: 14px;
border-radius: 6px;
}이렇게 변형된 버전에 따른 스타일 규칙을 따로 .btn-small 선택자로 묶어준다음,
사용하실땐
<button class="btn btn-small">버튼</button>
이런식으로 클래스이름들을 조합해서 사용하실수있어요 :)
There was a problem hiding this comment.
예시처럼 공통 스타일 추출하여 버튼 클래스 수정하였습니다!
style.css
Outdated
| * { | ||
| margin: 0; | ||
| padding: 0; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| body { | ||
| font-family: "Pretendard Variable"; | ||
| } | ||
|
|
||
| a { | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| header { | ||
| padding: 9.5px 0; | ||
| } |
There was a problem hiding this comment.
스타일 전역 처리는 따로 css파일을 만들어 필요할때마다 import해오는 방식으로 구현해보셔도 좋습니다 :)
- reset.css (스타일 초기화)
- common.css (전역 스타일)
There was a problem hiding this comment.
reset.css, common.css 추가하여 코드 분리하였습니다!
style.css
Outdated
| } | ||
|
|
||
| header .header-content .login-btn { | ||
| background-color: #3692FF; |
There was a problem hiding this comment.
자주 사용되는 값들은 css 변수로 만들어 처리해보면 코드의 중복을 줄이면서도 재사용성을 높일수있어요 :)
예시)
:root {
/* Colors */
--color-primary: #3692FF;
--color-primary-dark: #3b82f6;
--color-text-primary: #333;
--color-text-secondary: #374151;
--color-background: #ffffff;
--color-background-light: #FCFCFC;
--color-background-blue: #dbeafe;
--color-footer: #111827;
--color-footer-text: #E5E7EB;
/* Spacing */
--spacing-xs: 8px;
--spacing-sm: 16px;
--spacing-md: 24px;
--spacing-lg: 32px;
--spacing-xl: 40px;
--spacing-xxl: 60px;
--spacing-xxxl: 100px;
/* Font Sizes */
--font-size-xs: 16px;
--font-size-sm: 18px;
--font-size-md: 20px;
--font-size-lg: 24px;
--font-size-xl: 40px;
/* Border Radius */
--border-radius-sm: 8px;
--border-radius-lg: 40px;
/* Container Widths */
--container-padding: 32px;
--container-max-width: 1200px;
}There was a problem hiding this comment.
만약 반복되는 공용 스타일을 따로 분리해서 사용하고있다면, 해당 값도 변수로 관리해볼까요? 유지보수할때 편리하겠죠?
:root {
/* Button Colors */
--btn-primary: #3692FF;
--btn-primary-hover: #2B7AE0;
--btn-primary-active: #1F5DB3;
--btn-text-light: #F3F4F6;
/* Button Sizes */
--btn-height-small: 36px;
--btn-height-medium: 48px;
--btn-height-large: 56px;
}```There was a problem hiding this comment.
color, border-radius, font-size 속성 변수로 추가하여 사용하였습니다!
style.css
Outdated
| .link a { | ||
| text-decoration: none; | ||
| color: #E5E7EB; | ||
| font-size: 16px; | ||
| font-weight: 400; | ||
| } | ||
|
|
||
| .link a:not(:last-child) { | ||
| margin-right: 30px; | ||
| } | ||
|
|
||
| .sns a:not(:last-child) { | ||
| margin-right: 12px; | ||
| } No newline at end of file |
There was a problem hiding this comment.
link들 사이의 간격을 조절하고싶으셨다면 .link 선택자에 display: flex 속성을 써서 정렬 및 간격 조절을 하는게 더 편하지않을까요? :)
There was a problem hiding this comment.
link, sns flex 속성으로 간격 조정하는 것으로 바꿨습니다!
질문에 대한 답변브라우저 크기에 따른 폰트 크기는 vw로 설정하면 되나요?보통 vw으로 설정하기보다는 일정한 분기점(브레이크 포인트)에서 폰트사이즈가 바뀔수있도록 미디어 쿼리를 사용해 사이즈를 조절해줍니다! :) |
심화 요구사항 반영미디어 쿼리 추가하여 1023px 이하인 경우 요소들 세로로 배치되게 수정하였습니다. |
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
배포 사이트 링크
https://sprint-mission1-suminbae.netlify.app/
멘토에게