[문주영] sprint1 #36
[문주영] sprint1 #36
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
주영님 첫번째 PR 제출 고생하셨습니다!
리뷰 드린 코멘트는 모두 필수 반영사항은 아니고 코드에 정답은 없으니
보고 마음에 드시는 방향으로 수정하시면 좋겠습니다!
다음 주차도 화이팅입니다~
footer에서 a 태그로 img 태그를 감쌀 시 크기가 미묘하게 안 맞는데 맞출 방법이 있을까요?:
a 태그는 기본적으로 inline 요소입니다. 정확한 사이즈를 가질 수 있게 a 태그에 display: inline-block | block 속성을 주세요. 그리고 img 태그 또한 기본적으로 inline 요소이며 vertical-alilgn이 baseline으로 정렬되기 때문에 여백이 생길 수 있습니다. 따라서 아래처럼 하시면 원하는 결과값을 얻으실 수 있습니다~
a {
display: inline-block;
}
a img {
vertical-align: top;
}There was a problem hiding this comment.
💊 제안
한 레포안에서는 동일한 룰을 따르는 것이 좋습니다.
따라서 img_home_01.png와 같은 식으로 같은 네이밍 룰을 따르시는 것을 추천드려요~
| @@ -0,0 +1,91 @@ | |||
| <!DOCTYPE html> | |||
| <html> | |||
There was a problem hiding this comment.
❗️ 수정요청
html lang 속성은 스크린리더, 검색 엔진등에게 해당 사이트가 어떤 언어 기반으로 작성되었는지 설명하는 속성입니다.
해당 속성에는 이미 특정 언어들을 의미하는 예약어가 들어가야 합니다. 예를들어 많이 사용되는 en은 english 즉 영어를 의미합니다.
아래처럼 한국어를 의미하는 ko를 넣어주세요~
| <html> | |
| <html lang="ko"> |
| <header> | ||
| <div> | ||
| <a href="/"> | ||
| <img src="images/logo_lg.png"> |
There was a problem hiding this comment.
💊 제안
img 태그의 alt 속성은 alternative 라는 의미로, 이미지 파일을 다운로드하는 것에 실패해서 이미지 파일을 보여줄 수 없을 때
어떤 이미지인지 파악할 수 있게 대신 제공되거나, 스크린리더로 읽혀지는 문자를 의미합니다.
따라서 가능한 해당 이미지를 잘 표현할 수 있는 문자열을 넣어주는 것이 좋습니다.
지금의 경우 "판다마켓"을 추천드려요~
| <img src="images/logo_lg.png"> | |
| <img src="images/logo_lg.png" alt="판다마켓"> |
| <section class="banner top"> | ||
| <div> | ||
| <div class="banner-explain"> | ||
| <h1>일상의 모든 물건을<br>거래해 보세요</h1> |
There was a problem hiding this comment.
❗️ 수정요청
h1 태그는 heading 1이라는 의미의 태그입니다. 이러한 h1~h6까지의 태그들은 시멘틱한 작성에 중요한 역할을 합니다.
그중 h1 태그는 제목에 해당하는 친구입니다.
목차나 영화를 생각해보시면 제목은 늘 1개입니다. 그래서 h1태그는 한 페이지에 한번만 사용하는 것이 좋습니다.
다만 이는 접근성을 위한 권고 사항일 뿐 필수사항은 아니지만 지키시는 것이 좋습니다.
| </div> | ||
| </article> | ||
| <article> | ||
| <div class="right"> |
There was a problem hiding this comment.
💊 제안
classname을 지으실때 위치에 대한 이름은 추천드리지 않습니다.
나중에 디자인이 변해 태그의 순서가 변했을때, 혹은 반응형 사이트라 화면 크기에 따라 태그 위치가 변할 수 있기 때문에
해당 요소를 잘 설명하지 못하기 때문입니다.
따라서 가능하면 해당 태그의 역할이나 내용을 설명하는 이름을 사용하는 것이 좋습니다.
| <a href="/"> | ||
| <img src="images/logo_lg.png"> | ||
| </a> | ||
| <a class="login" href="/login">로그인</a> |
There was a problem hiding this comment.
❗️ 수정요청
배포 사이트에서 로그인 버튼을 누르면 당연히 파일이 없으니 에러가 납니다.
다만 파일이 있어도 다른 설정을 해주지 않으시면 같은 에러가 날 확률이 높습니다~
/login의 의미는 root 폴더의 login으로 접근하라는 의미라서, 만약 login.html 파일이 root 파일에 위치해있고
다른 설정을 안해주셨으면 아래처럼 작성해주셔야 정상동작합니다~
| <a class="login" href="/login">로그인</a> | |
| <a class="login" href="/login.html">로그인</a> |
| * { | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| html { | ||
| font-family: Pretendard; | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| } | ||
|
|
||
| a { | ||
| text-decoration: none; | ||
| cursor: pointer; | ||
| } |
There was a problem hiding this comment.
💊 제안
간략한 reset css 속성들이라 따로 분리하지 않으시고 한 파일에 작성하셨나봐요~
하지만 필요시 특정 태그의 스타일을 초기화하는건 번거로운 일이고, 이를 염두에 두고 css 작성을 해야하니 가능하면 처음부터 브라우저 스타일을 초기화하시고 reset.css로 분리하시고 작업하시는 것을 추천드립니다!
| h1 { | ||
| display: block; | ||
| margin: 0; | ||
| color: #374151; | ||
| font-size: 40px; | ||
| font-weight: 700; | ||
| line-height: 140%; | ||
| } |
There was a problem hiding this comment.
💊 제안
태그 선택자는 해당되는 태그 모두에게 적용 가능하다는 의미라 특정 디자인의 경우 클래스 선택자로 작업하시는 것을 추천드려요.
이렇게 태그 선택자는 사이트 전반에 걸쳐 동일한 스타일이 적용되는 경우 사용하시는 것을 추천드립니다~
|
|
||
|
|
There was a problem hiding this comment.
💊 제안
특정 기준에 따라 개행이 한줄이나 두줄이냐가 결정되는 것 같아요.
아마 큰 영역은 개행 두번으로, 해당 영역내의 클래스들은 개행 한번으로 구분하시는 것 같아요.
다만 이는 알아차리기 어려운 룰이니 만약 영역별 구분을 하고 싶으시다면 주석으로 하시는 것을 추천드려요~
| margin-top: 24px; | ||
| font-size: 24px; | ||
| font-weight: 500; | ||
| line-height: 133.33%; |
There was a problem hiding this comment.
💊 제안
해당 값이 어디서 나온 것인지 모르겠지만, 소수점 단위의 사용은 의도를 파악하기 어려울 수 있어 추천하지 않습니다.
참고로 숫자만으로도 비슷한 결과를 구현할 수 있습니다.
예를 들어, line-height: 1.3;은 폰트 크기의 1.3배를 의미하고, line-height: 133.333%;는 폰트 크기의 133%로 결과적으로 비슷한 효과를 냅니다.
가독성과 유지보수를 고려하면 아래와 같이 1.3 형태로 작성하는 것이 더 직관적이니 참고해 주세요~
| line-height: 133.33%; | |
| line-height: 1.3; |
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게