-
Notifications
You must be signed in to change notification settings - Fork 39
[박연희] sprint3 #127
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 #127
The head ref may contain hidden characters: "Basic-\uBC15\uC5F0\uD76C-sprint3"
Conversation
…int-Mission into Basic-박연희-sprint3
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.
수고하셨습니다!
주요 리뷰 포인트
- 브레이크 포인트 사용 관련 피드백
- 반응형 이미지 리소스 최적화
| .pl0{padding-left:0 !important} | ||
| .pr0{padding-right:0 !important} | ||
| .pb0{padding-bottom:0 !important} | ||
| .fl{float: left} |
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.
| .fl{float: left} | |
| .fl { float: left } |
포맷팅 조금만 더 신경써볼까요?
| .auth-page { | ||
| color: var(--color-gray800); | ||
| /* max-width: 640px; */ | ||
| max-width: 400px; |
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.
max-width만 단독으로 쓰는것보다는 width: 100% 를 함께 써주시는게 좀더 명시적이고 디자인 의도에 맞아보이는데요!
max-width: 400px만 단독으로 사용하는 경우:
요소의 최대 너비를 400px로 제한하지만,
max-width: 400px; width: 100%를 함께 사용하는 경우:
width: 100%는 부모 요소의 전체 너비를 차지하도록 강제하고,
max-width: 400px는 400px를 넘지 않도록 제한하다보니
부모 요소의 전체 너비를 차지하되, 그 크기를 최대 400px까지 제한하게됩니다.
디자인 의도에 따라 이런 조합을 사용하시면 좋다고 팁을 드린거니까 참고해보세요! :)
|
|
||
| /*** 반응형 ***/ | ||
| /* 모바일 푸터 (768px~) */ | ||
| /* 모바일 푸터 (460px~) */ |
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.
flex의 order 속성을 사용해서 배치 순서를 바꾸셨군요! 굳굳 👍
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.
다만, 브레이크포인트(분기점) 숫자가 조금 애매한 화면 사이즈인것같은데
타블렛(768px) 이하는 모바일 기준으로 삼고, 기본 스타일은 모바일 기준으로 일관되게 작성해보시는건 어떨까요?
| label { | ||
| font-weight: var(--font-weight-700); | ||
| font-size: var(--font-size-lg); | ||
| } | ||
|
|
||
| input { | ||
| padding: 14px 24px; | ||
| font-size: var(--font-size-md); | ||
| background-color: var(--color-gray200); | ||
| border: none; | ||
| border-radius: var(--border-radius-sm); |
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.
해당 파일의 용도는 기본 스타일을 초기화하는 목적으로만 (e.g input태그의 border 스타일 없애기) 맞춰서 사용하시면 좋을것같아요.
전역에서 쓰이는 스타일을 처리하기위한 목적이라면 reset.css가 아닌 common.css에서 정의해두시는게 좋겠죠?
| <picture> | ||
| <source media="(min-width: 767px)" srcset="images/img_logo.png"> | ||
| <img class="header__logo-img" src="images/img_logo_m.png" alt="판다마켓" width="153" height="51"> | ||
| </picture> |
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.
picture태그를 사용하지않고도 화면 크기에 대응하는 이미지 옵션을 제공하고 현재 뷰포트 크기에 가장 적합한 이미지를 선택하게끔 만들 수 있습니다. 아래 코드처럼 바꿔볼까요?
| <picture> | |
| <source media="(min-width: 767px)" srcset="images/img_logo.png"> | |
| <img class="header__logo-img" src="images/img_logo_m.png" alt="판다마켓" width="153" height="51"> | |
| </picture> | |
| <img class="header__logo-img" | |
| src="images/img_logo_m.png" | |
| srcset="images/img_logo_m.png 153w, images/img_logo.png 198w" | |
| sizes="(max-width: 767px) 153px, 198px" | |
| alt="판다마켓" /> |
질문에 대한 답변
네, 축약형으로 쓰면 코드가 더 간결해보이겠죠? |
요구사항
기본
공통
랜딩 페이지
로그인, 회원가입 페이지 공통
심화
주요 변경사항
멘토에게