-
Notifications
You must be signed in to change notification settings - Fork 39
[남만재]Sprint1 #17
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
[남만재]Sprint1 #17
The head ref may contain hidden characters: "Basic-\uB0A8\uB9CC\uC7AC-sprint1"
Conversation
addiescode-sj
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배수, 3배수 export하셔서 srcSet을 적용하신건가했는데 아니었네요 ㅎㅎ
다음에는 반응형도 고려해서 작업해봐요!
주요 리뷰 포인트
- 네이밍 케이스, 네이밍 컨벤션
- 클래스이름
- 스타일 중복 제거
| * { | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| background-color: #ffffff; | ||
| color: #374151; | ||
| font-family: 'Noto Sans KR', sans-serif; | ||
| } |
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 (스타일 초기화)
- common.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.
스타일시트를 두 개 만들어서 전체에 적용되는 스타일과 개별로 나눠 적용한 스타일을 구분해서 관리하는 말씀이실까요?
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 파일은 필요한 파일에서 import 받아오시면 됩니다 :)
index.css
Outdated
| } | ||
|
|
||
| #button { | ||
| background-color: #3692ff; |
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.
자주 사용하는 값들은 변수로 만들어서 중복을 줄이고 재사용성을 높여볼까요?
예시)
:root {
/* Colors */
--color-primary: #3692ff;
--color-primary-dark: #1967d6;
--color-text: #374151;
--color-text-light: #9ca3af;
--color-background: #ffffff;
--color-background-alt: #fcfcfc;
--color-background-landing: #cfe5ff;
--color-background-footer: #111827;
--color-text-light: #f3f4f6;
--color-text-lighter: #f9fafb;
/* Typography */
--font-family-base: pretendard, sans-serif;
--font-family-heading: "ROKAFSansMedium", sans-serif;
/* Spacing */
--spacing-xs: 8px;
--spacing-sm: 12px;
--spacing-md: 24px;
--spacing-lg: 32px;
--spacing-xl: 64px;
--spacing-xxl: 138px;
/* Layout */
--container-max-width: 1520px;
--container-content-width: 1120px;
--container-section-width: 988px;
/* Border Radius */
--border-radius-sm: 8px;
--border-radius-lg: 40px;
}
index.css
Outdated
| #button { | ||
| background-color: #3692ff; | ||
| padding: 16px 124px; | ||
| border-radius: 9999px; | ||
| color: #f9fafb; | ||
| margin-bottom:100px; | ||
| text-decoration: none; | ||
| 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.
버튼 스타일이 #button과 #id에서 중복되는게 몇가지 있네요!
스타일 중복을 최대한 줄이기 위해 이런식으로 변수로 관리도 해주고, 버튼 클래스를 만드는건 어떨까요?
중복이 많이 되는 스타일은 공통 .button 클래스에 적용하고 variant (변형 버전)은 modifier를 사용해 따로 만들어주세요.
이렇게하면 스타일 중복도 최대한 없앨 수 있고 유지보수하기도 편해질거예요 :)
/* Button styles */
.button {
display: inline-block;
padding: 16px 34px;
border-radius: 8px;
text-decoration: none;
font-size: var(--font-size-small);
transition: background-color 0.3s ease;
}
.button--primary {
background-color: var(--color-primary);
color: var(--color-text-light);
}
.button--primary:hover {
background-color: var(--color-primary-dark);
}
.button--rounded {
border-radius: 9999px;
padding: 16px 124px;
}
addiescode-sj
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.
코멘트 위치 변경해서 다시 리뷰 드립니다~
index.html
Outdated
| 일상의 모든 물건을<br> | ||
| 거래해 보세요 | ||
| </div> | ||
| <a id="button" href="/item"> |
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 선택자를 쓰셨네요! id는 고유 요소를 식별하기위해 사용되는데, 여기서 사용될 필요가 있었을까요?
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.
버튼 속성이 조금 다르길래 시도했는데 너무 생각이 너무 짧았습니다!
더욱 간결하게 수정해보겠습니다!
| </section> | ||
|
|
||
| <div class="main"> | ||
| <section class="hotItem"> |
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.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게