-
Notifications
You must be signed in to change notification settings - Fork 39
Basic 남만재 sprint3 #138
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
Basic 남만재 sprint3 #138
The head ref may contain hidden characters: "Basic-\uB0A8\uB9CC\uC7AC-sprint3"
Conversation
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.
수고하셨습니다!
주요 리뷰 포인트
- 미디어쿼리 사용 관련 피드백
|
|
||
| .section-title { | ||
| font-size: 40px; | ||
| white-space: 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.
white-space: normal; 은 기본값이라 써주시지않으셔도됩니다 :)
| padding: 0 10px; | ||
| } | ||
|
|
||
| @media (max-width: 1199px) { |
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.
현재 CSS파일에서 max-width: 1199px -> max-width: 767px 순서로 미디어쿼리가 작성되어있어 specificity 문제가 발생할수있습니다. 기본 스타일은 모바일에 맞추고, (작은 화면에서부터) 큰 화면으로 점차 확장해나가는 순서로 작성하시면 불필요한 스타일 재정의 및 코드 중복을 효과적으로 줄일 수 있습니다.
또, 미디어쿼리 조건 평가 시 max-width를 사용하게되면 max-width: 767px 이하의 화면에서는 두가지 미디어쿼리가 모두 적용됩니다. 이때 첫번째로 얘기한 specificity(특이성) 문제가 생길 수 있는데요! 두 미디어쿼리의 특이성이 동일하므로 나중에 선언된 스타일이 적용됩니다. 따라서 767px 이하 스크린에서 동일한 선택자의 속성을 재정의하지않으면 1199px이하에 적용되어있는 스타일이 동시에 적용됩니다. 관리에 그닥 좋지 않겠죠?
| <div class="input-wrapper"> | ||
| <input class="form-content-input" id="password" name="password" type="password" placeholder="입력"> | ||
| <img class="visibility-icon" src="/img/Property 1=Variant2.png"> | ||
| <input |
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.
접근성과 데이터 제출을 잘 고려해서 작성하셨네요 👍 굳굳!
| </a> | ||
| </div> | ||
| <img class="home" src="img/Img_home_top.png" alt="동산 위, 가운데 장바구니를 들고있는 판다마켓 캐릭터를 중심으로 왼 편에는 주거 건물을 표현한 그림이 있고 오른 편에는 나무 두 그루가 있다."> | ||
| <img class="imageSet" src="img/Img_home_top.png" alt="동산 위, 가운데 장바구니를 들고있는 판다마켓 캐릭터를 중심으로 왼 편에는 주거 건물을 표현한 그림이 있고 오른 편에는 나무 두 그루가 있다."> |
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.
여기 쓰인 이미지도 srcset과 sizes 사용을 고려한다면 반응형 이미지 리소스 최적화가 이루어질수있겠죠? 첫 화면에 보이는 이미지가 아닌 스크롤을 쭉 내려야 보이는 이미지라면 레이지로딩을 적용할수도있고요 :)
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.
이 파일에서도 위에서 코멘트 드렸던 미디어 쿼리를 사용하면서 불필요한 스타일 재정의를 최소화하는 방법 잘 참고해보세요!
그리고 컨테이너 요소의 레이아웃과 관련된 속성 (너비, 간격 등) 은 CSS 변수로 만드시는것도 아래 예시와 같이 코드를 훨씬 명료하게 만들어줄 수 있습니다 :)
예시)
:root {
--container-width: 100%;
}
@media (min-width: 768px) {
:root {
--container-width: 750px;
}
}
.container {
width: var(--container-width);
}
요구사항
기본
심화
주요 변경사항
스크린샷
https://6812dd3d39170ac353315aa0--deluxe-banoffee-89d731.netlify.app/
멘토에게