-
Notifications
You must be signed in to change notification settings - Fork 21
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 #6
The head ref may contain hidden characters: "Basic-\uAE40\uD76C\uC9C4-sprint1"
[김희진] sprint1 #6
Conversation
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.
희진님 첫 스프린트 미션 제출 고생하셨습니다!
아래는 전반적으로 참고하시면 좋을 것 같은 내용입니다!
- 동일한 내용에 대해서는 코드에서 한번만 코멘트를 답니다.
- 깔끔하게 commit 쪼개주신거 좋습니다.
- 시멘틱하게 태그 작성하려고 고민하신점, 네이밍 규칙에 대해 고민하신점이 좋습니다.
- 디자인상 규칙을 찾으려고 고민하시고 max-width 관련된 변수를 작성하신 점이 좋았습니다.
- 반응형을 고려해서 작업하는 것에 대해 질문해주셨는데, 우선 PC, tablet, mobile 스크린별로 디자인을 파악하고 html 작성시 해당 디자인이 구현 가능하도록 작성하시는 것이 좋습니다. 이것이 가능하려면 디자인을 보면 대략적인 태그와 스타일링이 떠올라야 가능한거라 노력을 해보시다가 잘 안되시면 시간이 필요한 것이니 가능한 방식으로 작업하시면 좋겠습니다.
--primary-100: #3692ff; | ||
--primary-200: #1967d6; | ||
--primary-300: #1251aa; | ||
--main-bg-color: #cfe5ff; | ||
--secondary-gray-900: #111827; | ||
--secondary-gray-800: #1f2937; | ||
--secondary-gray-700: #374151; | ||
--secondary-gray-600: #4b5563; | ||
--secondary-gray-500: #6b7280; | ||
--secondary-gray-400: #9ca3af; | ||
--secondary-gray-200: #e5e7eb; | ||
--secondary-gray-100: #f3f4f6; | ||
--secondary-gray-50: #f9fafb; |
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.
P3:
사소한거지만 primary color는 오름차순, secondary color는 내림차순인게 조금 신경쓰이네요.
기왕이면 같은 방식으로 정렬되면 좋을 것 같습니다.
--main-font: Pretendard; | ||
--size-max-width: 1200px; | ||
} |
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.
P3:
한가지 폰트만 사용되니 변수로 폰트명을 선언해두시고 사용하기보다 그냥 바로 스타일링해주셔도 괜찮을 것 같습니다.
--main-font: Pretendard; | |
--size-max-width: 1200px; | |
} | |
--size-max-width: 1200px; | |
} | |
html { | |
font-family: "Pretendard", 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.
P2:
이미지 명들의 규칙이 통일되면 좋을 것 같습니다.
@@ -0,0 +1,95 @@ | |||
<!DOCTYPE html> | |||
<html lang="en"> |
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.
P2:
<html lang="en"> | |
<html lang="ko"> |
.btn-login { | ||
background-color: var(--primary-100); | ||
color: var(--secondary-gray-100); | ||
padding: 12px 23px 12px 23px; |
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.
P3:
width로 버튼의 너비를 정해주셨으니 padding값은 아래처럼 기입해도 좋을 것 같습니다.
아래처럼 작성하시면 나중에 버튼의 너비를 바꿔야할때, width, padding 속성 중 width값만 바꿔주시면 됩니다.
padding: 12px 23px 12px 23px; | |
padding: 12px 0; | |
text-align: center; |
.register-content__span { | ||
color: var(--primary-100, #3692ff); | ||
font-size: 18px; | ||
font-style: normal; |
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.
P2:
불필요한 css 속성은 없는것이 더 좋겠습니다.
gap: 30px; | ||
} | ||
|
||
.footer-mid__a { |
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.
P3:
태그명은 클래스명에 들어가지 않는 것을 추천드립니다.
좀더 의미가 있는 클래스명을 추천드립니다.
.footer-mid__a { | |
.footer-mid__link { |
<a href="/"> | ||
<img src="/images/logo.png" alt="판다 로고" /> | ||
</a> | ||
<button class="btn-login" onclick="location.href='/login.html'">로그인</button> |
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.
P3:
페이지 이동의 경우 a tag 를 추천드립니다.
<button class="btn-login" onclick="location.href='/login.html'">로그인</button> | |
<a class="btn-login" href="./login">로그인</button> |
<section class="hero"> | ||
<div class="hero-container"> | ||
<div class="hero-content"> | ||
<h2 class="hero-content__title">일상의 모든 물건을<br>거래해 보세요</h2> |
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.
P2:
한 페이지안에서 h1, h2 태그는 여러개 쓰지 않는 것을 추천드립니다.
h1, h2 태그는 각각 하나씩만을 사용하시고 h3부터 사용하시면 좋을 것 같습니다.
</section> | ||
<section class="hot-item"> | ||
<div class="hot-item-container"> | ||
<img src="/images/Img_home_01.png" alt="Img_home_01.png" class="hot-item__image" /> |
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.
P3:
이미지의 알트 속성은 이미지가 불러올 수 없거나 스크린 리더기에서 이미지 대신 읽어주는 용입니다.
가능하다면 이미지를 설명하는 텍스트를 기입해주시고 아니라면 비워두셔도 됩니다.
<img src="/images/Img_home_01.png" alt="Img_home_01.png" class="hot-item__image" /> | |
<img src="/images/Img_home_01.png" alt="쇼핑하는 판다 둘" class="hot-item__image" /> |
@GANGYIKIM 리뷰 내용 현재 브랜치는 머지가 되가지구 sprint2 브랜치에 적용 완료했습니다~! |
요구사항
기본
심화
배포주소
멘토에게