[김동한] Sprint1 #12
[김동한] Sprint1 #12
Conversation
There was a problem hiding this comment.
동한님 첫번째 PR 제출 고생하셨습니다!
리뷰 드린 코멘트는 모두 필수 반영사항은 아니고 코드에 정답은 없으니
보고 마음에 드시는 방향으로 수정하시면 좋겠습니다!
다음 주차도 화이팅입니다~
-
다음에 PR 올리실때는 target branch를 main 이 아닌 Basic-김동한으로 해주세요~ 우선 제가 변경해서 merge 합니다.
-
git commit 단위를 나누시는 연습을 해보시면 좋을 것 같아요. index page css 추가, html 수정과 같은 식으로 작게 쪼개시면 시점을 돌리시기도 더 좋답니다.
-
반응형을 구현하는데 어려움을 느꼈습니다. 사용 단위나 구현 방법 등을 피드백 받고 싶습니다.:
완성하신 화면은 잘 구현하셔서 어떤 부분에서 어려움을 느끼셨는지 잘 모르겠네요~ 아마 어떻게 구현해야 하는지에 대한 디자인 없이 반응형 단위를 사용하려고 하시니 어려움을 느끼시지 않았을까 합니다. 추후 반응형 디자인이 구현되니 그때는 더 수월하게 작업하실 것 같아요! -
요구 사항 기준에 맞게 잘 적용되었는지 궁금합니다.: 요구사항대로 잘 구현하신 것 같습니다~
There was a problem hiding this comment.
❗️ 수정요청
해당 이미지 파일의 크기를 보면 588× 444로 되어 있습니다.
저희는 반응형 사이트를 만들고 있고, 이런 이미지들은 모바일에서도 사용할 것이기 때문에 추출하실때, 1배수가 아닌 2배수로 추출해주시는 것이 화질면에서 좋습니다.
아래처럼 figma export 탭에서 2x 로 설정하고 추출해주세요~
There was a problem hiding this comment.
❓ 질문
main1.png, main2.png 가 어떤 경우 필요할까요? 구현에 불필요한 이미지라면 없어도 될 것 같아요~
| <div class="icons"> | ||
| <a href="https://www.facebook.com/?locale=ko_KR" target="_blank"></a | ||
| ><img src="/image/ic_facebook.png" alt="페이스북" /> | ||
| <a href="https://x.com/?lang=ko" target="_blank"></a | ||
| ><img src="/image/ic_twitter.png" alt="트위터" /> | ||
| <a href="https://www.youtube.com/" target="_blank"></a | ||
| ><img src="/image/ic_youtube.png" alt="유튜브" /> | ||
| <a href="https://www.instagram.com/" target="_blank"></a | ||
| ><img src="/image/ic_instagram.png" alt="인스타그램" /> | ||
| </div> |
There was a problem hiding this comment.
💊 제안
이 영역은 로고들의 모음이라고 할 수 있습니다. 관련된 요소들끼리 묶인 목록이니 의미록적인 마크업을 작성한다면 list 관련 태그로 작성하시는 것을 추천드립니다.
또한 의도하신 바가 각 sns의 아이콘을 눌렀을때 해당하는 sns 로의 이동이라면 a 태그안에 img 태그를 넣으시는 것을 추천드려요~
| <div class="icons"> | |
| <a href="https://www.facebook.com/?locale=ko_KR" target="_blank"></a | |
| ><img src="/image/ic_facebook.png" alt="페이스북" /> | |
| <a href="https://x.com/?lang=ko" target="_blank"></a | |
| ><img src="/image/ic_twitter.png" alt="트위터" /> | |
| <a href="https://www.youtube.com/" target="_blank"></a | |
| ><img src="/image/ic_youtube.png" alt="유튜브" /> | |
| <a href="https://www.instagram.com/" target="_blank"></a | |
| ><img src="/image/ic_instagram.png" alt="인스타그램" /> | |
| </div> | |
| <ul class="icons"> | |
| <li> | |
| <a href="https://www.facebook.com/?locale=ko_KR" target="_blank"> | |
| <img src="/image/ic_facebook.png" alt="페이스북" /> | |
| </a> | |
| </li> | |
| </ul> |
| </div> | ||
|
|
||
| <div class="ad"> | ||
| <h1 class="title">믿을 수 있는<br />판다마켓 중고 거래</h1> |
There was a problem hiding this comment.
❗️ 수정요청
h1 태그는 heading 1이라는 의미의 태그입니다. 이러한 h1~h6까지의 태그들은 시멘틱한 작성에 중요한 역할을 합니다.
그중 h1 태그는 제목에 해당하는 친구입니다.
목차나 영화를 생각해보시면 제목은 늘 1개입니다. 그래서 h1태그는 한 페이지에 한번만 사용하는 것이 좋습니다.
다만 이는 접근성을 위한 권고사항일 뿐 필수사항은 아닙니다 그래도 권고니 지키는것이 더 좋습니다!
| <body> | ||
| <header> | ||
| <div class="header_wrap"> | ||
| <a href="/"><img src="/image/logo.png" alt="판다마켓 로고" class="logo" /></a> |
There was a problem hiding this comment.
👍 칭찬
img 태그의 alt 속성은 alternative 라는 의미로, 이미지 파일을 다운로드하는 것에 실패해서 이미지 파일을 보여줄 수 없을 때
어떤 이미지인지 파악할 수 있게 대신 제공되거나, 스크린리더로 읽혀지는 문자를 의미합니다.
적절한 alt 태그 사용 좋습니다
| @@ -0,0 +1,107 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="ko"> | |||
There was a problem hiding this comment.
👍 칭찬
html lang을 ko로 설정해주신 것 좋습니다~ 다음에는 다른 페이지들도 변경해주시면 좋겠네요~
| text-decoration: none; | ||
| cursor: pointer; | ||
| } | ||
|
|
There was a problem hiding this comment.
💊 제안
브라우저 마다 자체적으로 스타일을 가지고 있습니다.
위의 코드에서 a 태그의 text-decoration: none을 적용해주신건, 이미 text-decoration: underline 속성을 가지고 있기 때문이죠~
이런식으로 이미 가지고 있는 스타일을 없애 원하는 스타일을 적용하기 위한 작업이 필요합니다.
주로 이러한 역할을 하는 css 파일을 reset.css 혹은 normalize.css 라는 이름으로 만들어서 관리합니다.
인성님께서는 지금 필요한 태그의 스타일만 초기화하고 사용하셨지만 reset.css 같은 파일을 만드셔서
다른 태그들의 기본 스타일도 초기화하고 작업하시는 것이 예상치 못한 동작을 방지할 수 있어서 추천드려요!
혹시 어떤 내용들이 들어가는지 궁금하시다면 reset.css, normalize.css 를 구글에 검색해보시면 다양한 파일이 나오니
확인해보시고 마음에 드는 것을 참고하시면 좋을 것 같습니다.
| cursor: pointer; | ||
| } | ||
|
|
||
| /* Header */ |
There was a problem hiding this comment.
👍 칭찬
파악하기 쉽게 관련된 css 들을 모아두시고 주석으로 구분해주신점 좋습니다!
| .login:hover { | ||
| background-color: #1967d6; | ||
| } | ||
| /* Header */ |
There was a problem hiding this comment.
💊 제안
Header 관련 css의 시작인지, 끝인지 알기 모호하기 때문에, 주석을 각 영역 상하단에 모두 넣을 때는 아래처럼 하시는 것이 구분하기 좋습니다.
| /* Header */ | |
| /* End: Header */ |
이렇게 하시면 상단의 주석은 /* Start: Header */가 되겠죠?
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷
판다마켓url
멘토에게