[유원규] sprint1 #19
[유원규] sprint1 #19
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
원규님 첫번째 PR 제출 고생하셨습니다!
리뷰 드린 코멘트는 모두 필수 반영사항은 아니고 코드에 정답은 없으니
보고 마음에 드시는 방향으로 수정하시면 좋겠습니다!
다음 주차도 화이팅입니다~
- div 보다 시멘틱한 태그작성을 해보시면 좋을 것 같습니다. section, article, nav, header, footer 등 태그 자체가 의미를 가진 태그를 사용하여 가독성과 접근성을 향상시킬 수 있습니다.
- git commit 단위를 나누시는 연습을 해보시면 좋을 것 같아요. index page css 추가, html 수정과 같은 식으로 작게 쪼개시면 시점을 돌리시기도 더 좋답니다.
There was a problem hiding this comment.
💊 제안
이해하기 쉬운 icon-facebook.svg 같은 이름을 추천드려요~
There was a problem hiding this comment.
💬 여담
로그인 페이지를 구성해두신거 귀엽네요 👍
There was a problem hiding this comment.
💊 제안
이렇게 브라우저 탭에서 보이는 이미지들을 favicon이라고 합니다.
파일명들은 최대한 어떤 이미지이고 어떤 역할을 하는지 알 수 있는 이름이 좋으니 favicon으로 변경하시는 것을 추천드려요~
| *{box-sizing: border-box; | ||
| flex-wrap: wrap;} |
There was a problem hiding this comment.
💊 제안
현재 코드에서는 { 다음 줄에서 속성이 바로 시작되는데, 일반적으로 줄바꿈하여 정렬하면 코드의 가독성이 좋아지고 유지보수하기 쉬워집니다! 이런 스타일을 일관되게 유지하는 것을 추천드려요~
| *{box-sizing: border-box; | |
| flex-wrap: wrap;} | |
| * { | |
| box-sizing: border-box; | |
| flex-wrap: wrap; | |
| } |
| html | ||
| body | ||
| { | ||
| font-family: 'Noto Sans KR', sans-serif; |
There was a problem hiding this comment.
❗️ 수정요청
디자인상 사용된 폰트는 pretendard로 알고 있습니다~
확인해보시고 수정하시는 것을 추천드려요!
| flex-direction: column; | ||
|
|
||
| } | ||
| .benner-ment { |
There was a problem hiding this comment.
❗️ 수정요청
이름은 코드의 가독성과 유지보수에 큰 영향을 줍니다.
HTML을 직접 보지 않더라도 클래스명이 의미를 명확하게 전달한다면, CSS 수정이 훨씬 쉬워집니다.
예를 들어, 단순히 box, item 같은 일반적인 이름보다는 profile-card, banner-title 같은 요소의 역할을 직관적으로 나타내는 이름이 더 좋습니다.
가능하면 의미가 분명한 이름으로 변경해 주시면 더 좋은 코드가 될 것 같아요!
| font-size: 0.9rem; | ||
| line-height: 1.193125rem; | ||
| text-align: left; | ||
| text-decoration: none; |
There was a problem hiding this comment.
💊 제안
브라우저 마다 자체적으로 스타일을 가지고 있습니다.
.foot-pri, .foot-faq 마다 text-decoration: none을 적용해주신건, 이미 text-decoration: underline 속성을 가지고 있기 때문이죠~
이런식으로 이미 가지고 있는 스타일을 없애 원하는 스타일을 적용하기 위한 작업이 필요합니다.
주로 이러한 역할을 하는 css 파일을 reset.css 혹은 normalize.css 라는 이름으로 만들어서 관리합니다.
원규님께서는 지금 필요한 태그의 스타일만 초기화하고 사용하셨지만 reset.css 같은 파일을 만드셔서
다른 태그들의 기본 스타일도 초기화하고 작업하시는 것이 예상치 못한 동작을 방지할 수 있어서 추천드려요!
혹시 어떤 내용들이 들어가는지 궁금하시다면 reset.css, normalize.css 를 구글에 검색해보시면 다양한 파일이 나오니
확인해보시고 마음에 드는 것을 참고하시면 좋을 것 같습니다.
| <div class="foot-box3"> | ||
| <div class="foot-face"> | ||
| <a href="https://www.facebook.com/" target='_blank'><img src="faceb.svg"></a> | ||
| </div> | ||
| <div class="foot-twi"> | ||
| <a href="https://x.com/?lang=ko" target='_blank'><img src="twi.svg"></a> | ||
| </div> | ||
| <div class="foot-ytub"> | ||
| <a href="https://www.youtube.com/?hl=ko" target='_blank'><img src="ytub.svg"></a> | ||
| </div> | ||
| <div class="foot-insta"> | ||
| <a href="https://www.instagram.com/" target='_blank'><img src="insta.svg"></a> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
💊 제안
이 영역은 로고들의 모음이라고 할 수 있습니다. 관련된 요소들끼리 묶인 목록이니 의미록적인 마크업을 작성한다면 list 관련 태그로 작성하시는 것을 추천드립니다.
또한 가능하면 classname도 어떤 태그인지 알 수 있는 이름으로 변경하시는 것을 추천드려요~
| <div class="foot-box3"> | |
| <div class="foot-face"> | |
| <a href="https://www.facebook.com/" target='_blank'><img src="faceb.svg"></a> | |
| </div> | |
| <div class="foot-twi"> | |
| <a href="https://x.com/?lang=ko" target='_blank'><img src="twi.svg"></a> | |
| </div> | |
| <div class="foot-ytub"> | |
| <a href="https://www.youtube.com/?hl=ko" target='_blank'><img src="ytub.svg"></a> | |
| </div> | |
| <div class="foot-insta"> | |
| <a href="https://www.instagram.com/" target='_blank'><img src="insta.svg"></a> | |
| </div> | |
| </div> | |
| <ul class="footer-sns"> | |
| <li class="footer-sns-item"> | |
| <a href="https://www.facebook.com/" target='_blank'><img src="faceb.svg"></a> | |
| </li> | |
| <li class="footer-sns-item"> | |
| <a href="https://x.com/?lang=ko" target='_blank'><img src="twi.svg"></a> | |
| </li> | |
| <li class="footer-sns-item"> | |
| <a href="https://www.youtube.com/?hl=ko" target='_blank'><img src="ytub.svg"></a> | |
| </li> | |
| <li class="footer-sns-item"> | |
| <a href="https://www.instagram.com/" target='_blank'><img src="insta.svg"></a> | |
| </li> | |
| </ul> |
| .foot-face | ||
| { | ||
| width: 1.25rem; | ||
| height: 1.25rem; | ||
| } | ||
| .foot-twi | ||
| { | ||
| width: 1.25rem; | ||
| height: 1.25rem; | ||
| } | ||
| .foot-ytub | ||
| { | ||
| width: 1.25rem; | ||
| height: 1.25rem; | ||
| } | ||
| .foot-insta | ||
| { | ||
| width: 1.25rem; | ||
| height: 1.25rem; | ||
| } |
There was a problem hiding this comment.
💊 제안
css가 반복사용되므로 하나의 클래스만 만들면 사용하시면 더 좋을 것 같습니다.
| .foot-face | |
| { | |
| width: 1.25rem; | |
| height: 1.25rem; | |
| } | |
| .foot-twi | |
| { | |
| width: 1.25rem; | |
| height: 1.25rem; | |
| } | |
| .foot-ytub | |
| { | |
| width: 1.25rem; | |
| height: 1.25rem; | |
| } | |
| .foot-insta | |
| { | |
| width: 1.25rem; | |
| height: 1.25rem; | |
| } | |
| .footer-item { | |
| width: 1.25rem; | |
| height: 1.25rem; | |
| } |
| </div> | ||
| <div class="foot-box2"> | ||
| <a class="foot-pri" href="/privacy">Privacy Policy</a> | ||
| <a class="foot-faq" href="/faq">FAQ</a> |
There was a problem hiding this comment.
❗️ 수정요청
불필요한 공백은 코드의 가독성을 해치고, 유지보수를 어렵게 만들 수 있습니다.
아래와 같이 정리해 주시면 더 깔끔한 코드가 될 것 같아요
| <a class="foot-faq" href="/faq">FAQ</a> | |
| <a class="foot-faq" href="/faq">FAQ</a> |
2 participants
요구사항
기본
https://pandanara99.netlify.app/
심화
주요 변경사항
스크린샷
멘토에게