[이지현] Sprint1 #21
[이지현] Sprint1 #21
Conversation
two678
requested review from
GANGYIKIM and
bootcampCodeit
and removed request for
bootcampCodeit
There was a problem hiding this comment.
지현님 첫번째 PR 제출 고생하셨습니다!
리뷰 드린 코멘트는 모두 필수 반영사항은 아니고 코드에 정답은 없으니
보고 마음에 드시는 방향으로 수정하시면 좋겠습니다!
다음 주차도 화이팅입니다~
-
commit 단위를 나눠 진행하신 것 좋습니다. 다만 저희가 PR 템플릿이 있는 것처럼 형식을 정해두시고 그 형식에 따라 commit 하시면 더 좋을 것 같아요~
-
심화 과정은 어떻게 해야하는 지 궁금합니다.: 해당 요구사항 그대로 반응형 단위를 사용하는 연습을 해보시라는 의미입니다. 다만 PC 버전의 디자인이 제공된 것과 달리 단위만 반응형을 쓰면 되는 미션이니, 문자 그대로 이해하시면 될 것 같아요~ 구현 결과가 다양하니 다른 분들의 PR을 구경하셔도 좋을 것 같습니다. -
화면이 원래 사이즈의 절반이 되면 다들 비율이 이상한데 이걸 어떻게 해야할지 모르겠습니다.: 위에서 말씀드린 것처럼 지현님은 현재 PC 버전의 디자인을 구현하신거라 브라우저 사이즈를 축소했을때의 대응이 되지 않는 것이 정상입니다. 추후 주차가 진행됨에 따라 tablet, mobile 디자인이 제공되어 그때 대응하셔도 되고, tablet, mobile에서 원하시는 구현상이 있으시면 반응형 단위를 이용해 구현하셔도 됩니다~
There was a problem hiding this comment.
❗️ 수정요청
해당 이미지 파일의 크기를 보면 588× 444로 되어 있습니다.
저희는 반응형 사이트를 만들고 있고, 이런 이미지들은 모바일에서도 사용할 것이기 때문에 추출하실때, 1배수가 아닌 2배수로 추출해주시는 것이 화질면에서 좋습니다.
아래처럼 figma export 탭에서 2x 로 설정하고 추출해주세요~
There was a problem hiding this comment.
💊 제안
한 레포안에서는 동일한 룰을 따르는 것이 좋습니다.
따라서 다른 이미지 파일명처럼 소문자로 이루어지게 img_home_01.png로 변경하시는 걸 추천드려요~
There was a problem hiding this comment.
❗️ 수정요청
로고 이미지가 여러개가 필요한 이유가 있을까요?
하나의 이미지만 남기시는 것을 추천드립니다~
| @@ -0,0 +1,82 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="kr"> | |||
There was a problem hiding this comment.
❗️ 수정요청
html lang 속성은 스크린리더, 검색 엔진등에게 해당 사이트가 어떤 언어 기반으로 작성되었는지 설명하는 속성입니다.
해당 속성에는 이미 특정 언어들을 의미하는 예약어가 들어가야 합니다. 예를들어 en은 english 즉 영어를 의미합니다.
kr은 신기하게도 Kanuri 어를 의미합니다. 한국어는 ko를 써야합니다~
https://developer.mozilla.org/ko/docs/Web/HTML/Global_attributes/lang
(해당 페이지 sidebar LockUp 에서 kr과 ko를 검색해보세요~)
https://r12a.github.io/app-subtags/
| <html lang="kr"> | |
| <html lang="ko"> |
| <body> | ||
| <header> | ||
| <div class="logo"> | ||
| <img src="/assets/logos/logo_1.png" alt="판다마켓 로고"> |
There was a problem hiding this comment.
👍 칭찬
img 태그의 alt 속성은 alternative 라는 의미로, 이미지 파일을 다운로드하는 것에 실패해서 이미지 파일을 보여줄 수 없을 때
어떤 이미지인지 파악할 수 있게 대신 제공되거나, 스크린리더로 읽혀지는 문자를 의미합니다.
따라서 가능한 이미지를 설명할 수 있는 대체 텍스트를 입력해주신 점 좋습니다~
| <div class="logo"> | ||
| <img src="/assets/logos/logo_1.png" alt="판다마켓 로고"> | ||
| </div> | ||
| <button class="login_btn" onclick="location.href='login.html'">로그인</button> |
There was a problem hiding this comment.
💊 제안
해당 디자인이 버튼처럼 보이지만 기능상 페이지 이동이기에 a 태그 사용을 추천드립니다.
(디자인대로 태그를 작성하시기보다 역할을 보시고 태그를 선택하시는 것을 추천드려요.)
a 태그를 사용하시면 링크이기때문에 웹 접근성에도 좋으며 alt 를 누르고 링크 클릭시 새창으로 열 수도 있어 사용성면에서도 더 좋다고 생각합니당
| <button class="login_btn" onclick="location.href='login.html'">로그인</button> | |
| <a class="login_btn" href="/login.html">로그인</a> |
| <div class="icons"> | ||
| <a href="https://www.facebook.com/"> | ||
| <img src="/assets/icons/ic_facebook.png" alt="페이스북 아이콘"> | ||
| </a> | ||
| <a href="https://x.com/"> | ||
| <img src="/assets/icons/ic_twitter.png" alt="트위터 아이콘"> | ||
| </a> | ||
| <a href="https://www.youtube.com/"> | ||
| <img src="/assets/icons/ic_youtube.png" alt="유튜브 아이콘"> | ||
| </a> | ||
| <a href="https://www.instagram.com/"> | ||
| <img src="/assets/icons/ic_instagram.png" alt="인스타 아이콘"> | ||
| </a> | ||
| </div> |
There was a problem hiding this comment.
💊 제안
이 영역은 로고들의 모음이라고 할 수 있습니다. 관련된 요소들끼리 묶인 목록이니 의미록적인 마크업을 작성한다면 list 관련 태그로 작성하시는 것을 추천드립니다.
<ul class="icons">
<li>
<a href="https://www.facebook.com/">
<img src="/assets/icons/ic_facebook.png" alt="페이스북 아이콘">
</a>
</li>
</ul>| a:hover { | ||
| cursor: pointer; | ||
| } |
There was a problem hiding this comment.
❗️ 수정요청
동작 측면에서 차이는 없을 것 같습니다만 css에서 cursor 속성을 pointer 로 해주는 것은 해당 속성이 클릭 가능하다는 것을 의미하기 때문이라, :hover 상태가 아닌 a 자체에 추가하시는 게 더 이해하기 쉬울 것 같습니다.
다만 대부분의 브라우저들은 자체적으로 a 태그에 cursor pointer 속성을 가지고 있어서 동작은 같을 것 같습니다~
| a:hover { | |
| cursor: pointer; | |
| } | |
| a { | |
| cursor: pointer; | |
| } |
| body { | ||
| margin: 0 auto; | ||
| font-family: -apple-system, BlinkMacSystemFont, "Apple SD Gothic Neo", "Pretendard Variable", Pretendard, Roboto, "Noto Sans KR", "Segoe UI", "Malgun Gothic", "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", sans-serif; | ||
| } |
There was a problem hiding this comment.
❗️ 수정요청
폰트 패밀리 속성을 왜 이렇게 작성하셨을까요? mdn의 font-family 설명을 보면 해당 속성에 나열된 폰트명들은 순서대로 우선순위를 가집니다.
즉 위의 값에서는 apple의 시스템 폰트가 우선순위가 제일 높다는 의미로 저희가 사용하고자 했던 Pretendard는 4번째의 우선순위로 앞의 폰트들이 없을때 선택되어집니다.
아래처럼 사용하고자 하는 pretendard 폰트의 우선순위를 높여주시는 것을 추천드려요~
| body { | |
| margin: 0 auto; | |
| font-family: -apple-system, BlinkMacSystemFont, "Apple SD Gothic Neo", "Pretendard Variable", Pretendard, Roboto, "Noto Sans KR", "Segoe UI", "Malgun Gothic", "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", sans-serif; | |
| } | |
| body { | |
| margin: 0 auto; | |
| font-family: "Pretendard Variable", Pretendard, -apple-system, BlinkMacSystemFont, system-ui, Roboto, "Helvetica Neue", "Segoe UI", "Apple SD Gothic Neo", "Noto Sans KR", "Malgun Gothic", "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", sans-serif; | |
| } |
There was a problem hiding this comment.
💊 제안
브라우저는 기본적으로 자체적인 스타일 파일을 가지고 있습니다.
그렇지 않다면 html 파일은 그냥 문서처럼 보여질거에요. 이러한 스타일은 사실 브라우저마다 다르기 때문에
이러한 스타일을 없애 원하는 스타일을 적용하기 위한 밑작업이 필요합니다.
주로 이러한 역할을 하는 css 파일을 reset.css 혹은 normalize.css 라는 이름으로 만들어서 관리합니다.
지금은 필요한 경우 스타일을 overriding 하시면서 작업하셨지만 reset.css 같은 파일을 만드셔서
다른 태그들의 기본 스타일도 초기화하고 작업하시는 것이 예상치 못한 동작을 방지할 수 있어서 추천드려요!
혹시 어떤 내용들이 들어가는지 궁금하시다면 reset.css, normalize.css 를 구글에 검색해보시면 다양한 파일이 나오니
확인해보시고 마음에 드는 것을 참고하시면 좋을 것 같습니다.
2 participants
요구사항
[기본]
title을 "판다마켓"으로 설정함cursor: pointer속성을 적용하여 사용자에게 클릭 가능한 요소임을 알려줌심화
주요 변경사항
스크린샷
멘토에게