-
Notifications
You must be signed in to change notification settings - Fork 39
[이유진] sprint1 #15
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 #15
The head ref may contain hidden characters: "Basic-\uC774\uC720\uC9C4-sprint1"
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.
수고하셨습니다!
주요 리뷰 포인트
- 접근성 향상
- 이미지 최적화
- 네이밍 케이스, 포맷팅
- 미디어 쿼리 사용
| /* 1280px ~ 1919px */ | ||
| @media (max-width: 1920px) { | ||
| html { | ||
| font-size: 56.25%; /* 1rem = 9px */ | ||
| } | ||
| header{ | ||
| padding: 9px 200px; | ||
| } | ||
| } | ||
|
|
||
| /* 768px ~ 1279px */ | ||
| @media (max-width: 1279px) { | ||
| html { | ||
| font-size: 50%; /* 1rem = 8px */ | ||
| } | ||
| header{ | ||
| padding: 9px 24px; | ||
| } | ||
| .hero-inner, .feature,.footer-banner-inner,footer{ | ||
| padding:0 24px; | ||
| } | ||
| .feature-inner{ | ||
| padding: 16px; | ||
| } | ||
| } | ||
|
|
||
| /* <768px */ | ||
| @media (max-width: 767px) { | ||
| .feature{ | ||
| height: auto; | ||
| padding: 64px; | ||
| } | ||
| } | ||
|
|
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.
이 미디어쿼리 구조에는 몇가지 잠재적인 문제가 있을수있어요.
우선 현재 큰 화면에서 정의된 스타일이 작은 화면에서도 유지되고 미디어 쿼리에 의해 다시 정의되고있는데요, 이런 스타일 중복이나 오버라이드 관련 문제를 해결해주려면
/* Tablet (768px ~ 1279px) */
@media (min-width: 768px) {
:root {
--header-padding: 100px;
--container-padding: 100px;
--feature-padding: 80px;
}
html {
font-size: 50%; /* 1rem = 8px */
}
.feature {
height: 720px;
padding: 0;
}
}
/* Desktop (1280px ~ 1919px) */
@media (min-width: 1280px) {
:root {
--header-padding: 200px;
--container-padding: 200px;
}
html {
font-size: 56.25%; /* 1rem = 9px */
}
}
/* Large Desktop (1920px and above) */
@media (min-width: 1920px) {
:root {
--header-padding: 400px;
--container-padding: 400px;
}
html {
font-size: 62.5%; /* 1rem = 10px */
}
}작은 화면에서부터 미디어 쿼리에 의한 재정의를 하는 구조로 바꿔보시는게 불필요한 스타일을 제거하는데 좀 더 유리합니다.
그리고 공통으로 사용되는 값들 (header의 padding처럼) 을 css 변수로 정의한다음 각 브레이크 포인트에서 변수값만 변경해주는식으로 활용하시면 스타일도 중복 제거하면서 유지보수에도 용이해집니다.
참고해보시고 리팩토링해보세요! :)
질문에 대한 답변피그마 시안에서 요소 크기가 px로 지정되어 있어서, 그대로 px 단위로 구현했습니다.그런데 실제 개발에서는 반응형을 고려해 다른 단위를 사용하는 게 더 좋은지 궁금합니다. 반응형 대응을 고려한다면 px같은 값이 변하지 않는 고정 단위보다는 가변되는 값을 가진 상대 단위를 사용해주시는게 좋습니다. 현재 작성한 HTML 구조(요소의 묶음, 시맨틱 구조 등)가 적절하게 구성됐는지 궁금합니다.특히 features section 내부 구조가 반복되고 깊어졌다는 느낌이 들어서, 이런걸 궁금해하시고 고민해보는 시도 너무 좋습니다 👍 features section내부에 불필요한 중첩이 있고, 스타일 중복도 많은 편인것같네요. <article class="feature">
<div class="feature-content">
<span class="badge Text-2lg font-semibold">Hot item</span>
<h2 class="Text-4xl font-bold">
인기 상품을 <br />
확인해보세요
</h2>
<p class="Text-2xl font-medium">
가장 HOT한 중고거래 물품을 <br />
판다 마켓에서 확인해 보세요
</p>
</div>
<div class="feature-image">
<img src="images/Img_feature_01.svg" alt="인기 상품" />
</div>
</article>
<article class="feature feature--reverse">
<div class="feature-content">
<span class="badge Text-2lg font-semibold">search</span>
<h2 class="Text-4xl font-bold">
구매를 원하는<br />상품을 검색하세요
</h2>
<p class="Text-2xl font-medium">
구매하고 싶은 물품은 검색해서<br />쉽게 찾아보세요
</p>
</div>
<div class="feature-image">
<img src="images/Img_feature_02.svg" alt="상품 검색" />
</div>
</article>feature-inner 와 같은 불필요한 중첩을 줄이고 폰트 관련해서는 다음 두 가지가 궁금합니다.rem 단위를 사용할 때 기준(root font-size)을 어떻게 설정하는 게 좋을지 보통 rem을 사용할때는 계산하기편하게 default font size를 10px로 설정하는편입니다. commits 계속 저 내용을 적는 걸까요..?refactor(mentor): 태그 사용을 말씀하시는거라면 제가 피드백드린 부분에 대한 수정 작업을 하실때만 사용하시면 됩니다 :) |
요구사항
기본
/)로 설정<title>은 "판다마켓"으로 설정cursor: pointer적용/)로 이동/login으로 이동 (빈 페이지)/items로 이동 (빈 페이지)/privacy이동 (빈 페이지)/faq이동 (빈 페이지)심화
주요 변경사항
스크린샷
멘토에게
피그마 시안에서 요소 크기가
px로 지정되어 있어서, 그대로px단위로 구현했습니다.그런데 실제 개발에서는 반응형을 고려해 다른 단위를 사용하는 게 더 좋은지 궁금합니다.
다른 단위를 선택할 때 어떤 기준을 적용하는지도 알고 싶습니다.
현재 작성한 HTML 구조(요소의 묶음, 시맨틱 구조 등)가 적절하게 구성됐는지 궁금합니다.
특히
featuressection 내부 구조가 반복되고 깊어졌다는 느낌이 들어서,더 간결하게 구성할 수 있는 팁이 있다면 듣고 싶습니다.
폰트 관련해서는 다음 두 가지가 궁금합니다.
rem단위를 사용할 때 기준(root font-size)을 어떻게 설정하는 게 좋을지font-size와font-weight를 class로 관리했는데, CSS 변수를 사용하는 방식과 비교했을 때 어떤 방식이 더 효율적인지클래스 네이밍도 쉽지 않았는데요, 일반적으로 어떤 기준이나 관습이 있는지 조언 듣고 싶습니다.
현재 구현한 내용이 심화 체크 기준에 잘 맞는지도 검토 부탁드립니다.
commits 계속 저 내용을 적는 걸까요..?
셀프 코드 리뷰를 통해 이어서 질문드리겠습니다. 감사합니다.