-
Notifications
You must be signed in to change notification settings - Fork 39
[양예지] Sprint1 #3
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 #3
The head ref may contain hidden characters: "Basic-\uC591\uC608\uC9C0-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.
수고하셨습니다!
주요 리뷰 포인트
- 시맨틱 태그 사용 권장
- 불필요한 div 중첩 수정
- 이미지 alt 사용 권장 및 레퍼런스 제공
- 사용 목적별로 파일 분리 권장
- 매직 키워드를 사용한 클래스명 수정
- 포맷팅
| <link rel="stylesheet" href="style.css"> | ||
| </head> | ||
| <body> | ||
| <header> |
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.
header보다는 nav가 더 적절한 태그가 될 것 같네요!
| </div> | ||
| </header> | ||
| <main> | ||
| <div class="banner-top"> |
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과 같은 시맨틱 태그를 사용해볼까요?
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.
banner-top, blue-01 과 같이 위치, 색상등의 키워드를 포함한 클래스명을 사용하는것은 다소 적절하지 않습니다. 이유는 디자인이 변경된다면 실제 스타일과 클래스명이 불일치할 수 있고, 이는 곧 재사용성의 저해로 이어지기때문입니다. 따라서, btn-primary, hero와 같이 해당 엘리먼트의 목적과 역할을 보다 직관적으로 이해할 수 있게끔 염두에 두고 작업해보시는걸 추천드립니다 :)
| <p>일상의 모든 물건을<br>거래해 보세요</p> | ||
| <a class="btn-l blue-01" href="items.html">구경하러 가기</a> | ||
| </div> | ||
| <img src="./img/home_top.png"> |
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.
귀찮더라도 img태그에는 꼭 alt를 사용해주세요! :)
웹 접근성뿐만 아니라, 이미지 로딩 실패 시 대체 텍스트를 표시하거나 검색엔진최적화에도 도움을 줄 수 있기 때문에 꼭 사용해주시는게 좋습니다.
레퍼런스로 애플 공식 웹사이트에 가서 alt텍스트를 어떻게 사용했는지 참고해보시면:
<img src="/kr/macbook-air/images/overview/design/design_hero_static__e56c1v71mr6u_large.jpg" onload="__lp(event)" alt="열려있는 MacBook Air 13 및 15의 모습. 한 대에는 디자인 작업을 진행 중인 화면이, 다른 한 대에는 이메일과 스프레드시트를 넘나들며 멀티태스킹을 하는 화면이 표시되어 있습니다">이런식으로 alt 속성에 이미지 설명을 위해 구체적이고 명확한 설명을 제공하는 모습을 확인해보실 수 있습니다 :)
| </head> | ||
| <body> | ||
| <header> | ||
| <div class="content"> |
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.
불필요한 div 중첩이 보이네요! .content에 공통적으로 적용할 스타일 규칙이 따로 있는게 아니라면 + 충분히 다른 선택자를 사용해 요소에 대한 구분이 가능하다면 따로 만들어서 관리해주실필요는 없습니다 :)
| * { | ||
| font-family: 'Pretendard', 'Noto Sans KR', sans-serifs; | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| } | ||
|
|
||
| p { | ||
| margin: 0; | ||
| } | ||
|
|
||
| a { | ||
| text-decoration: none; | ||
| } | ||
|
|
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.
사용 목적별로 파일을 분리해서 사용해보시는건 어떨까요?
- reset.css (스타일 초기화 css 파일)
- common.css (전역 스타일 css 파일)
이렇게 스타일 초기화 목적의 코드는 따로 파일을 분리해서 필요할때 재사용하게되면 코드 중복도 줄일수있어서 좋겠죠? :)
|
|
||
| /* 배경색 */ | ||
|
|
||
| .blue-01 { |
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.
다음부터는 클래스이름만 봐도 어떤 용도/목적인지 명확히 구분되게끔 신경써서 작업해봐요! :)
| footer .content .notice span a{ | ||
| color: #fff; | ||
| } |
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 다음에 띄어쓰기 한칸 해주세요! 포맷팅이 어긋나면 작업 퀄리티가 상대적으로 많이 떨어져보여서 포맷팅을 일정하게 잘 지켜주시는 습관을 초반에 들여보시는것도 좋은 공부가 될 것 같네요 :) 들여쓰기도 4칸씩 되어있는데, 2칸 정도만 들여쓰기하셔도 충분합니다! :)
질문에 대한 답변기본 체크리스트 4번째 항목이 어떤 스타일을 적용해야 하는지 몰라서 우선 양쪽 여백 200px가 되도록만 작업했습니다.다음 미션에서는 요구사항을 더 많이 체크해볼수있도록 노력해봅시다 ! :) |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게