[하재호]sprint1 #120
Merged
[하재호]sprint1 #120
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addiescode-sj
approved these changes
Apr 28, 2025
Collaborator
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨습니다!
첫 PR인데 큰 실수없이 잘 작성해보셨네요 👍 다음에도 미션 제출하셔서 많이 연습해봅시다 ㅎㅎ
주요 리뷰 포인트
- 접근성 향상
- 올바른 이미지 포맷 사용
- 클래스이름 변경
Collaborator
There was a problem hiding this comment.
투명 배경을 지원할 필요가 없으면 png보다는 jpg 파일 사용을 권장드립니다.
추천 드리자면:
- 벡터 기반의 장점을 활용할수있거나, path 정보가 간단한 로고, 아이콘등의 파일: svg
- 투명 배경을 지원할 필요가 있는 큰 이미지 파일: png
- 투명 배경을 지원할 필요가 없는 큰 이미지 파일: 최소 2x의 고배율 jpg, jpeg => 고배율 이미지를 써야하므로 png, jpeg보다 압축률이 좋은 webp로 변환해 picture태그와 함께 사용 추천
컨텐츠의 특성 혹은 몇가지 조건에 따라 올바른 파일 포맷을 고르는것도 최적화에 포함됩니다.
제가 추천드린것말고도 여러가지 참고해보시고 적용해보세요 :)
| </head> | ||
|
|
||
| <body> | ||
|
|
| width="153" | ||
| /> | ||
|
|
||
| <a href="login.html" id="loginLinkButton" class="button">로그인</a> |
Collaborator
There was a problem hiding this comment.
스크린리더 사용자들을 위한 접근성 향상을 위해 aria-label을 사용해볼까요?
<a
href="/login"
class="login"
aria-label="로그인 페이지로 이동"
>
로그인
</a>접근성 개념에 대해 더 알고싶으시다면 아래 아티클 참고해보세요!
| </div> | ||
| </section> | ||
|
|
||
| <section id="bottomBanner" class="banner"> |
Collaborator
There was a problem hiding this comment.
클래스이름을 지을때는 UI의 색상, 위치, 순서등이 포함되어있으면 디자인 변경 시 대응하기도 어려워지고 유지보수에 좋지 않아요. 이런 정보들을 포함하기보다는 항상 엘리먼트의 역할을 명확하게 나타낼수있는 네이밍을 써볼까요? :)
Comment on lines
+1
to
+18
| * { | ||
| margin: 0; | ||
| padding: 0; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| a { | ||
| text-decoration: none; | ||
| color: inherit; | ||
| } | ||
|
|
||
| button { | ||
| background: none; | ||
| border: none; | ||
| outline: none; | ||
| box-shadow: none; | ||
| cursor: pointer; | ||
| } |
Collaborator
There was a problem hiding this comment.
용도에 따라 CSS를 나눠서 관리해볼까요?
- reset.css: CSS 초기화
- common.css: 전역 스타일
이렇게 용도에 따라 파일을 나눠 관리하는건 어떤 이점을 가져다줄수있는지 한번 더 생각해볼수있어요.
- 역할 분리가 되어 관리가 쉬움: 각 파일의 목적이 명확해서 수정이나 유지보수가 쉬워질거예요.
- 사용성과 확장성 증가: 여러 페이지 혹은 프로젝트에서 거의 동일하게 재사용이 가능해요.
- 스타일 우선순위가 더 예측 가능해짐: reset.css → common.css → 페이지나 컴포넌트별 스타일 순으로 로딩하면 우선순위가 명확해지고 스타일 충돌을 줄일 수 있어요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게