-
Notifications
You must be signed in to change notification settings - Fork 39
[최창환] Sprint2 #23
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
[최창환] Sprint2 #23
The head ref may contain hidden characters: "Basic-\uCD5C\uCC3D\uD658-sprint2"
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.
수고하셨습니다!
주요 리뷰 포인트
- 이미지 최적화
- 반응형 컨테이너 최적화 및 코드 중복 제거 방법
- 접근성 향상
index.html
Outdated
| <a class="market-login button" href="/login.html">로그인</a> | ||
| </div> | ||
| </header> | ||
| <main> |
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.
main 태그 아래 div들을 section태그로 사용해주세요! 시맨틱 태그를 잘 사용해봅시다 :)
| <img | ||
| class="link-img-panda" | ||
| src="img/Img_home_top.png" | ||
| alt="top-img-of-panda" | ||
| /> |
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.
이미지 최적화도 조금 더 신경써볼까요?
웹페이지 성능 측정 지표중 CLS(Cumulative Layout Shift) 라는 지표가 있는데요,
현재 보고 있는 페이지에 갑자기 발생하는 레이아웃의 변경이 발생하면 사용자 입장에서 불편함을 겪을 수 있기때문에 레이아웃 이동(layout shift) 빈도를 측정해 성능을 평가한다고 보시면 됩니다. 보통 이미지 및 비디오 요소를 삽입할때 이런 현상이 자주 발생될수있겠죠?
따라서 레이아웃 시프트를 방지해주려면 이미지 및 비디오 요소에 width 와 height 속성을 항상 포함하거나 또는 CSS를 사용하여 필요한 공간(aspect-ratio-box)을 잡는 방법이 있습니다.
해당 이미지의 사이즈가 어느정도는 고정적이라면 width와 height를 직접적으로 넣는 방법이 있지만, 반응형을 고려해 비율을 지키며 변해야한다면 aspect-ratio를 사용하는 방법도 있습니다.
저라면 반응형을 고려해 이런식으로 css를 작성해줬을것같네요 :)
.link-img-panda {
display: block;
aspect-ratio: 16/9;
width: 100%;
height: auto;
object-fit: cover;
}이렇게 한다면 여러 스크린사이즈에서도 유연하게 이미지가 비율에 맞춰 사이즈를 가질수있으면서도 레이아웃시프트를 방지해줄수있겠죠?
이미지 최적화는 제가 제안드린 방법말고도 webp, avif 같은 차세대 확장자를 사용한다던지
srcSet을 사용해 각 스크린사이즈에 맞는 이미지파일을 로드한다던지
우선순위를 낮춰 레이지 로딩하는 등 여러가지 기법들이 있습니다.
차차 학습해보시면 좋을것같네요 👍
| .market { | ||
| height: var(--header-height); | ||
| display: flex; | ||
| justify-content: space-around; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| margin: 0; | ||
| margin: 0 200px; | ||
| } |
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.
반응형을 고려해 동적으로 컨테이너 영역을 스타일링하려할때 유리한 방법이 있어요.
우선, 컨테이너에 고정 width나 height 값을 주는 것보다는 max-width와 min() 함수를 사용해 반응형에 최적화된 컨테이너를 만들어주고, 패딩과 마진의 경우 css 변수로 만들어 미디어쿼리에 의해 동적으로 조정되게끔 해주시면 코드의 중복을 최대한 줄이면서도 어느 스크린 사이즈든 유연하게 대응할수있습니다.
예시를 하나 드려볼게요:
--container-padding: 20px;
--content-max-width: 1200px;.market {
height: var(--header-height);
display: flex;
justify-content: space-between;
align-items: center;
max-width: var(--content-max-width);
margin: 0 auto;
padding: 0 var(--container-padding);
}이렇게 스타일 규칙을 만들어주고,
미디어쿼리에 의해 css변수가 동적으로 바뀔수 있게 처리한다면:
@media (max-width: 768px) {
.market {
padding: 0 var(--spacing-md);
}
}좀더 깔끔하게, 코드 중복없이, 예측 가능한 코드의 흐름이 만들어지죠?
이렇게 리팩토링 하실때는, 어떻게 하면 무언가를 하기에 더 유리하거나 나은 구조를 만들 수 있을지 고민해보시는게 좋습니다 👍
index.html
Outdated
| ><img src="img/ic_facebook.png" alt="" | ||
| ><img src="img/ic_facebook.png" alt="facebook-link_img" | ||
| /></a> | ||
| <a href="https://x.com/?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.
외부 링크의 경우, 보안을 위해 rel="noopener noreferrer"를 추가해주세요! :)
noopener: 새 탭에서 열린 페이지가 원본 페이지에 접근하는 것을 방지
noreferrer: 리퍼러 정보가 새 페이지로 전달되는 것을 방지
| <input | ||
| type="email" | ||
| name="이메일" | ||
| id="email" | ||
| placeholder="[email protected]" | ||
| /> |
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.
폼 요소에 대한 접근성을 향상해볼까요?
현재 input 폼 요소에 라벨을 사용하지않아 스크린리더를 활용하지못해요.
귀찮더라도 접근성 향상을 위해 폼 요소를 꼭 라벨과 같이 활용해주세요 :)
예시)
<label for="email">이메일</label>
<input
type="email"
name="email"
id="email"
placeholder="[email protected]"
required
aria-required="true"
autocomplete="email"
/>
login.html
Outdated
| <p>이메일</p> | ||
| <input | ||
| type="email" | ||
| name="이메일" |
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.
name="이메일"과 같이 name 속성을 한글로 써주면 다국어 지원이 어려워지고, 서버에서 디코딩 시 인코딩 방식에 따라 깨질 수 있으니 주의해주세요. 항상 웹 표준은 영어가 기본이라서 유지보수에 문제가 생길수있답니다 :)
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.
| name="이메일" | |
| name="email" |
signup.html
Outdated
| type="password" | ||
| name="비밀번호" | ||
| id="password" | ||
| placeholder="pAssWoRD" |
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.
음 요런건 저를 위해 숨겨놓으신 이스터에그같은건가요...? ㅎㅎㅎ PR 올리실땐 작업 퀄리티를 꼭 신경써서 마무리해주세요!
질문에 대한 답변
body에 max-width를 부여하기보다는 body 아래에 컨테이너 역할을 하는 엘리먼트를 만들고 max-width를 부여해주시는게 일반적입니다! :) |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게