-
Notifications
You must be signed in to change notification settings - Fork 3
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
Kdt0 nam hyeon jun #68
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for velvety-puppy-c1029b ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for mellifluous-gnome-003496 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
style.css 에는 자주 사용되는 CSS 속성을 정리해서 추후 주로 사용하는 속성 및 값의 경우 따로 파일을 만들어서 관리하는 것이 |
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.
전반적으로 깔끔하고 되게 좋은 방식의 클론코딩인 것 같습니다!!
👍 👍
body { | ||
margin: 0; | ||
} | ||
|
||
.mb0 { | ||
margin-bottom: 0; | ||
} | ||
|
||
.mb15 { | ||
margin-bottom: 15px; | ||
} | ||
|
||
.xsmall-text { | ||
font-size: 10px; | ||
} | ||
|
||
.small-text { | ||
font-size: 12px; | ||
} | ||
|
||
.weight400 { | ||
font-weight: 400; | ||
} |
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.
이 부분이 공통적으로 사용하는 부분을 중복 최소하하기 위한 부분이군여. 저도 추후 채택해봐야겠습니다 -!
index.html
Outdated
</div> | ||
<div class="section__price-list"> | ||
<div class="section__price-name"> | ||
<p class="price-badge white-text thick">1개월 무료</p> |
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.
여기 태그에 padding 속성이 밑줄 그어져 있는데, 어떤 이유일까요
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.css 내 해당 부분 스타일 중에서 padding 부분을 주석처리했기 때문입니다... 사용하지 않는 코드들 마지막에 정리한다고 했는데도 못 보고 넘어간 부분들이 있었네요.
index.html
Outdated
<li> | ||
<img src="./assets/image/icons8-done.svg" alt=""> | ||
<p>무광고로 음악 감상하기</p> | ||
</li> |
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.
grid로 나눈 영역 내에서 li 태그들을 포함하는 section__price-ul 클래스 부분에 border가 제대로 적용이 안되어 있었네요.. 이 부분도 리팩토링하면서 다시 수정하려고 합니다!
스타일관리에 신경을 써주신 코드라 느꼈습니다! |
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.
고생하셨습니다!
크게 코드에 코멘트 할 내용은 없는 것 같습니다.
리뷰 총평
전체적으로 태그를 시멘틱하게 잘 주신 것 같습니다.
css도 영역에 따라 나누고 tailwind와 같은 방식도 시도해 보셔서 좋은 것 같습니다.
다만 일부 눈에 띄게 스타일이 깨지는 부분이 있어
해당 부분이 왜 그런지 다시 체크해 보시면 좋을 것 같습니다.
html에서 더 감싸주어야 하는 부분도 있고
특정 태그를 이용하여 js없이 구현 가능한 부분도 있습니다.
main.css
Outdated
.section__download-banner article { | ||
display: flex; | ||
flex-direction: column; | ||
/* justify-content: center; */ |
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.
마지막에 정리하면서 사용하지 않는 스타일이나, 주석 처리한 코드들을 제거했는데 미처 확인 못한 부분이 있었네요ㅠㅠ 확인해주셔서 감사합니다!
style.css
Outdated
.fs20 { | ||
font-size: 20px; | ||
} | ||
|
||
.fs24 { | ||
font-size: 24px; | ||
} | ||
|
||
.fs40 { | ||
font-size: 40px; | ||
} | ||
|
||
.thick { | ||
font-weight: bold; | ||
} | ||
|
||
.black { | ||
background-color: black; | ||
} | ||
|
||
.white { | ||
background-color: white; | ||
} | ||
|
||
.white-text { | ||
color: white; | ||
} | ||
|
||
.black-text { | ||
color: black; | ||
} | ||
|
||
.gray-text { | ||
color: gray; | ||
} |
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.
오 리팩토링시에 저도 시도해봐야겠어요!
기획 의도
링크
구성
회고