-
Notifications
You must be signed in to change notification settings - Fork 39
[김진선] Sprint1 #7
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 #7
The head ref may contain hidden characters: "Basic-\uAE40\uC9C4\uC120-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.
수고하셨습니다! 전체적으로 깔끔하게 작업해주셨네요 👍
클래스네이밍만 조금 신경써보고, 다음엔 반응형을 고려한 작업도 연습해보시면 좋을것같아요!
주요 리뷰 포인트
- 직관적이고 계층이 뚜렷한 클래스네이밍 짓기
- CSS 변수 사용 등으로 코드 중복 줄이고 재사용성 높이기
| <img src="images/Img_home_bottom.png" alt="판다 캐릭터" /> | ||
| </div> | ||
| </section> | ||
| </main> |
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.
main 태그가 불완전한 상태로 배치되어있네요!
| <img src="images/Img_home_top.png" alt="판다 캐릭터" /> | ||
| </div> | ||
| </section> | ||
| <section class="item wrap"> |
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을 좀더 유의미하게 구분했으면 좋겠어요. 적절한 class를 추가해 구분해보고, 계층 구조를 명확히 해보면 어떨까요? 예를 들어, 반복되는 섹션은 feature 클래스로 묶어주어 일관성을 높이고 각 섹션의 변형은 modifier class로 표현해보는 방식도 괜찮아요.
예시)
<section class="feature feature--hot"><div class="feature__container wrap"></div></section>
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.
| <img src="images/Img_home_01.png" alt="아이템 콘텐츠 이미지"> | ||
| </div> | ||
| <div class="item-content"> | ||
| <p class="font-blue">Hot item</p> |
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.
클래스이름에 위치, 색상 등 UI에 한정된 네이밍을 사용하시는건 적절치않아요.
이유는 디자인이 변경된다면 실제 스타일과 클래스명이 불일치할 수 있고, 이는 곧 재사용성의 저해로 이어지기때문입니다.
따라서, 다음부터는 해당 엘리먼트의 목적과 역할을 보다 직관적으로 이해할 수 있게끔 염두에 두고 작업해보시는걸 추천드립니다 :)
| <section class="item wrap"> | ||
| <div class="item-container"> | ||
| <div class="item-image"> | ||
| <img src="images/Img_home_03.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.
NIT: 이미지 최적화를 위해서는 width와 height를 속성으로 부여해주시는게 좀 더 좋습니다.
성능 최적화 정도를 나타내는 중요한 지표중에 CLS(Cumulative Layout Shift)가 있는데요,
만약 width와 height가 HTML 속성으로 부여되어있지않다면 CSS 파일이 로드되기전에는 브라우저가 이미지의 크기를 알 수 없고, CSS 파일은 나중에 로드되거나 변경될 수 있기때문에 선택자 매칭과 계산 과정이 필요하게됩니다.
따라서 레이아웃이 갑자기 변경될 수 있는 가능성을 방지(CLS 방지)하면서 CSS가 로드되지않더라도 항상 안정적으로 레이아웃을 유지시키고싶다면, HTML 속성으로 width와 height를 미리 지정해 브라우저에게 이미지 로드전에 필요한 정확한 크기 계산(레이아웃 계산)을 수행하게 만들어줄수있습니다. 물론 이정도로 가시적이고 유의미한 차이는 없겠지만, 이 기회에 브라우저에서 html, css, js파일이 어떻게 로드되는지 동작 원리나 배경을 학습해보시면 도움이 될 것 같아 알려드립니다! :)
| * { | ||
| box-sizing: border-box; | ||
| font-family: 'Pretendard', sans-serif; | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| padding: 0; | ||
| background-color: #ffffff; | ||
| 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.
전역적인 스타일 처리는 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.
아래 코멘트에서 달아드린 css 변수 처리도 해당 파일에 두면 유지보수 및 관리에 용이할거예요 :)
| /* 버튼스타일 */ | ||
| .btn-small-40 { | ||
| padding: 12px 24px; | ||
| 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.
반복되는 값들은 css variables로 처리해서 재사용해줄까요?
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: #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;
}
요구사항
기본
심화
배포한 사이트 링크
👉 내 배포 사이트
멘토에게