-
Notifications
You must be signed in to change notification settings - Fork 38
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
[홍준기] Sprint 1 #20
The head ref may contain hidden characters: "Basic-\uD64D\uC900\uAE30-sprint1"
[홍준기] Sprint 1 #20
Conversation
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.
일단 코드만 보고 코멘트를 남겼는데, 한번 UI 돌려보고 추가로 코멘트 남겨둘게요ㅎㅎ!!
개발하시느라 고생하셨어요~
(지난번에도 얘기하긴 했지만 원래 첫 PR이 코멘트 제일 많아요...ㅠ)
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.
가능하면 프로젝트와 상관이 없는 파일은 무시하는게 좋아요...!
멘토링떄 말씀드리겠지만 gitignore 라는게 있는데, 여기에 등록해두면 등록된 파일들은 따로 git에 올라가지 않아서 관리하기 좋습니다ㅎㅎ;;
보통 현업에선 https://www.toptal.com/developers/gitignore/ 이걸 자주 사용하긴 해요!
있다가 멘토링때 공유드리겠습니다~
|
||
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.
마지막줄에 endline 을 붙여주시는게 좋아요~
이건 필수는 아니긴 해요ㅎㅎ;;
https://stackoverflow.com/a/34184247/5949460 이 글 참고하시면 좋을것같아요!
요약하자면...
- 변경점 파악이 좋음
- 규격화
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.
이건 원래 로그인 버튼이 이미지인건가요...?
버튼같은건 이미지로 처리하면 별로 좋진 않긴 해요ㅠ
예를들면... 버튼을 클릭했을때 물결무늬 애니메이션이나 조금 들어가는듯한 이펙트를 주려면 이미지로는 조금 힘들거든요ㅠ
그리고 다국어 처리하려면 좀 까다로워서 이건 이미지가 아닌 코드로 구현해보시는게 좋을거에요!
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.
그리고 파일명이 조금 좋지 않은것같아요ㅠ
statusDefault.png 처럼 = 같은 특수문자는 뺴는게 좋을것같구요,
네이밍도 의미있는 네이밍이 좋을것같아요~ㅎㅎ
|
||
body { | ||
margin: 0 auto; | ||
|
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.
의미없는 라인은 지워주시는게 좋을것같아요ㅎㅎ!
} | ||
|
||
.section2-text{ | ||
text-align: right; |
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.
section2-text 라는 이름과 text-align right 는 서로 관련이 없는것같아요ㅠ
차라리 right-text 라던가... class 이름만 봐도 어떤 역할을 하는지 알 수 있으면 좋을것같아요!
margin-bottom: 138px; | ||
} | ||
|
||
.bottomBackground { |
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.
가능하면 코드에서 일정한 컨벤션을 가져가는게 좋아요ㅎㅎ!
네이밍 컨벤션인데, bottomBackground 는 camel case 이구요
아래 bottom-panda 같은건 kebab case 라고 해요...!
일반적으로 css는 kebab case 를 사용하긴 해요~
컨벤션은 프로젝트의 규칙이니 규칙이 일정하게 지켜지기만 하면 괜찮다고 생각해요ㅎㅎ (아니라고 생각하는 개발자도 많으니 참고...ㅠ)
<body> | ||
<header> | ||
<div class = "header-content"> | ||
<a href = "/"><img id = "판다마켓" src = "image/Property 1=Variant3.png"></a> |
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 href = "/">...</a>
올바른 표기
<a href="/">...</a>
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 class = "section1"> | ||
<img class = "image" src = "image/Img_home_01.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.
들여쓰기가 잘 되어있어야 코드가 읽기가 좋아요ㅎㅎ
이런 들여쓰기를 indent 라고 합니다!
<a id = "footer-center-text" href = "/faq">FAQ</a> | ||
</div> | ||
<div class = "footer-snsLink"> | ||
<a href = "https://www.facebook.com/?locale=ko_KR"><img class = "icon" src = "image/facebook.png"></a> |
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.
src에 맨 앞에 / 를 붙이면 절대경로, / 가 없이 바로 image 같이 나오면 상대경로가 되어요...!
그래서 어떤 url path 에서 이 html이 노출되면 정상적으로 이미지를 읽어올 수 없을꺼에요ㅠ
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게