-
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 #11
The head ref may contain hidden characters: "part1-\uAE40\uC9C0\uC724-sprint1"
[김지윤] Sprint1 #11
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.
지윤님 첫번째 스프린트 미션 제출 고생하셨습니다~
아래는 부분적이 아니라 전반적으로 참고하시면 좋을 내용들입니다.
- 동일한 내용에 대해서는 코드에서 한번만 코멘트를 답니다.
- 커밋을 더 단위별로 쪼개주시면 좋을 것 같습니다.
- 스타일링을 기준을 잡으셔서 구분해 작성하신 것이 좋았습니다.
- 추후 반응형 디자인을 고려해 image 태그의 width 나 인라인 스타일링보다 css 를 통해 스타일링하시기를 권장드립니다.
- 일관적인 방식으로 class명을 작성해주셔서 좋았습니다.
/* Q. height의 높이가 차이 나는 이유? | ||
div, div > a !== div > a > img | ||
height = 55 !== 51 | ||
display: inline-block or flex로 해결 */ |
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 태그 (인라인 태그) 안에 img 태그 (인라인 태그) 가 들어가며 vertical-align 속성의 영향을 받기 때문에 수직정렬이 의도하지 않게 적용되는 현상입니다.
따라서 display: flex
보다 아래와 같이 처리하시는 걸 추천드립니다.
.header__login-btn img {
vertical-align: middle;
}
color: var(--Secondary-700); | ||
font-size: 40px; | ||
font-weight: 700; | ||
line-height: 140%; /* 56px */ |
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:
아래와 동일한 표현입니다.
line-height: 140%; /* 56px */ | |
line-height: 1.4; |
/* Primary color */ | ||
--Primary-100: #3692ff; | ||
--Primary-200: #1967d6; | ||
--Primary-300: #1251aa; |
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는 내림차순인게 조금 신경쓰이네요.
기왕이면 같은 방식으로 정렬되면 좋을 것 같습니다.
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.
P1:
이미지 명에 공백은 들어가지 않는 것이 좋습니다.
또한 어떤 이미지인지 알 수 있는 이름(logo.png)이면 좋을 것 같습니다.
https://www.webauthorings.com/spaces-in-image-names-file-names/
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,185 @@ | |||
<!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"> |
<div class="header__content"> | ||
<a href="./" class="header__logo"> | ||
<img | ||
style="width: 153px; height: 51px" |
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 를 통해 스타일링해주세요.
나중에 반응형 처리하실때도 미디어쿼리문을 통해 화면 사이즈별로 해당 로고 변환이 어렵습니다.
<h1 class="landing-vertical__title"> | ||
일상의 모든 물건을<br /> | ||
거래해 보세요 | ||
</h1> |
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부터 사용하시면 좋을 것 같습니다.
요구사항
기본
-[x] “판다마켓” 클릭 시 루트 페이지(‘/’)로 이동합니다.
심화
주요 변경사항
스크린샷
https://sprint-mission-1-panda-market.netlify.app/
멘토에게