-
Notifications
You must be signed in to change notification settings - Fork 39
[신성오] sprint3 #96
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
[신성오] sprint3 #96
The head ref may contain hidden characters: "Basic-\uC2E0\uC131\uC624-sprint3"
Conversation
GANGYIKIM
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.
성오님 3번째 PR 수고하셨습니다~
새로운 지식을 학습 하시고 구현하시는 것이 쉽지만은 않을 것 같습니다.
어떤 점이 어려우신지, 어떤 점에서 제대로 구현이 되지 않을 것 같다고 느끼시는 지 등을 자세히 적어주시면 도움을 드리기 더 편할 것 같습니다~
참고로 스프린트 미션은 학습을 위한 미션이므로 이렇게 구현해서 제출하시는 것만으로도 충분히 잘 하고 계신거라고 생각합니다.
아직 미흡한 부분이 있지만 이는 추후 충분히 개선이 가능한 사항들이니 우선 주차별로 작업해보시고 어려움이 있으실때 도움을 받으시는 것을 추천드려요!
다음 미션도 화이팅입니다!
| <img src="./images/Property 1=sm.png" alt="판다마켓" class="header_img"> | ||
| </a> | ||
| <a href="login.html" target="_self" class="login_btn"> | ||
| <img src="./images/btn_small.png" alt="btn_small.png" class="login"> |
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주차에도 드렸지만 중요한 내용이라 다시 남깁니다~
구경하러 가기 버튼을 바꿔주신 것처럼 로그인 버튼도 변경해주세요!
| <img src="./images/btn_small.png" alt="btn_small.png" class="login"> | |
| 로그인 |
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과 동일한 페이지의 수정 전 버전 파일 같네요~
둘 중 사용하지 않는 파일은 지워주세요~
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.
| @media (min-width: 1200px) { | ||
| .container { | ||
| width:50%; | ||
|
|
||
| } | ||
| /* Tablet ver */ | ||
| @media (max-width: 1199px) and (min-width: 768px) { | ||
| body { | ||
| width: 70%; | ||
| } | ||
| .header { | ||
| padding: 0 24px; | ||
| } | ||
| } | ||
| /* Mobile ver */ | ||
| @media (max-width: 767px) and (max-width: 375px) { | ||
| body { | ||
| width:90%; | ||
| } | ||
| .header { | ||
| padding: 0 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.
❗️ 수정요청
PC 미디어 쿼리문안에서 중첩될 수 없는 조건문안에 tablet 과 moblie 이 작성되어 있어 적용이 되지 않네요!
밖으로 빼주세요~
| @media (min-width: 1200px) { | |
| .container { | |
| width:50%; | |
| } | |
| /* Tablet ver */ | |
| @media (max-width: 1199px) and (min-width: 768px) { | |
| body { | |
| width: 70%; | |
| } | |
| .header { | |
| padding: 0 24px; | |
| } | |
| } | |
| /* Mobile ver */ | |
| @media (max-width: 767px) and (max-width: 375px) { | |
| body { | |
| width:90%; | |
| } | |
| .header { | |
| padding: 0 16px; | |
| } | |
| } | |
| } | |
| /* PC ver*/ | |
| @media (min-width: 1200px) { | |
| .container { | |
| width: 50%; | |
| } | |
| } | |
| /* Tablet ver */ | |
| @media (max-width: 1199px) and (min-width: 768px) { | |
| body { | |
| background-color: red; | |
| width: 70%; | |
| } | |
| .header { | |
| padding: 0 24px; | |
| } | |
| } | |
| /* Mobile ver */ | |
| @media (max-width: 767px) and (max-width: 375px) { | |
| body { | |
| width: 90%; | |
| } | |
| .header { | |
| padding: 0 16px; | |
| } | |
| } |
| } | ||
| } | ||
| /* Mobile ver */ | ||
| @media (max-width: 767px) and (max-width: 375px) { |
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.
💊 제안
min-width가 조건에 걸려있어 375px 미만으로 화면이 작아지게 되면 PC 스타일이 적용되네요~
body에 min-width: 375px를 추가하셔서 해당 사이즈 미만으로 작아지게 되면 가로 스크롤이 생기게 해주시거나
아니라면 min-width를 조건문에서 빼주시는 것을 추천드려요~
| .header { | ||
| padding: 0 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.
❗️ 수정요청
media query 문은 일반적으로 하단에 적습니다~
css 에서 추후 선언된 스타일이 우선순위를 가지기 때문에 반응형 관련 css들을 하단에 적어 같은 선택자 우선순위더라도
잘 적용될 수 있게 하기 위해서입니다~
따라서 하단에 적는 것을 추천드립니다~
// case1
. header { background-color: red }
@media (max-width: 800px) {
. header { background-color: blue } // 미디어 쿼리의 조건을 만족시키면 해당 사항이 적용.
}
// case2
@media (max-width: 800px) {
. header { background-color: blue }
}
. header { background-color: red } // 무조건 해당 사항이 적용| } | ||
|
|
||
| } | ||
| .header { |
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.
❗️ 수정요청
해당 클래스의 padding: 9px 250px 0 9px; 속성으로 인해 우측이 길어져 가로 스크롤이 생깁니다~
box-sizing: border-box를 추가하셔서 padding 값들이 width 값안에 포함되게 해주시거나 아니면 padding 값을 변경하셔서 가로 스크롤이 생기지 않게 변경하시는 것을 추천드려요.

요구사항
기본
랜딩 페이지
로그인, 회원가입 페이지 공통
심화
주요 변경사항
스크린샷
성5-sprint-mission3.netlify.app
멘토에게