-
Notifications
You must be signed in to change notification settings - Fork 46
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-\uC6B0\uC7AC\uD604-sprint1"
[우재현] Sprint1 #3
Conversation
- modifiy bottom banner alt - change border radius value of login btn design from px to rem
|
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.
미션을 전부 잘 구현해주셨네요!
코드는 크게 리뷰드릴 사항이 없었고, 오타와 몇 불필요한 파일만 수정해주시면 충분할 것 같습니다. 👍👍
PR 양식도 잘 지켜주셔서 감사드리고, 다음 PR 부턴 netlify 링크도 같이 첨부해주시면 리뷰하는 데 조금 더 편할 것 같습니다 ㅎㅎ
첫 과제인데 정말 고생 많으셨어요. 다음 과제도 화이팅입니다~!
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.
이미지가 사이즈별로 전부 다 올라온 것 같은데, 사용하지 않는 불필요한 이미지는 제거해주시면 좋을 것 같습니다!
@@ -0,0 +1,150 @@ | |||
<!DOCTYPE html> | |||
<html lang="kr"> |
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.
한국어의 lang 코드는 ko 입니다. 오타가 난거같네요 ㅎㅎ
<meta charset="UTF-8"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>판다마켓</title> | ||
<script src="https://kit.fontawesome.com/472da87bcd.js" crossorigin="anonymous"></script> |
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.
FontAwesome 에서 아이콘 불러와서 쓰는 것도 좋은 방법입니다. 다만 현재 피그마의 아이콘과 불러온 아이콘의 디자인이 다른 부분이 좀 있는 것 같아요 (인스타그램 아이콘). 실무에서는 세부적인 디자인 가이드를 따르는게 중요하기 때문에 아이콘도 img 처럼 svg 파일로 로컬에 저장해서 사용하곤 합니다. 참고 부탁드려요~!
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.
넵. 다음 미션에 바로 적용하겠습니다!
<link rel="stylesheet" href="style.css"> | ||
</head> | ||
|
||
<body> |
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.
html 구조는 명확하고 계층적으로 잘 나누어주셨습니다 👍
container, banner, card, btn 등 classname 도 직관적으로 잘 지어주셨네요. 또 나름의 규칙을 가지고 classname 을 확장해주신 것도 잘 해주셨습니다.
앗 Netlify 링크 올려주셨군요;; approve 한 후에나 발견했네요;; |
요구사항
판다마켓
기본
심화
주요 변경사항
스크린샷
멘토에게