Skip to content
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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Kdt0 nam hyeon jun #68

wants to merge 10 commits into from

Conversation

applevalley
Copy link

기획 의도

  • Spotify 멤버십 페이지를 클론코딩하기
  • flex와 grid를 사용해, css를 통한 웹 페이지 레이아웃에 대한 이해 키우기

링크

구성

  • header, nav, main, footer의 시맨틱 태그로 구성됨
  • style.css 파일을 통해, 공통적으로 사용되는 font-size, color, margin 등의 속성을 정의해 같은 css 속성을 태그마다 중복해서 입력하는 경우를 최소화함

회고

  • 처음부터 레이아웃에 대한 구조를 그려놓고 시작했어야 했는데, 막연히 시작하다 보니 원하는 속성이 제대로 적용되지 않아 계속 같은 작업을 반복하는 문제가 있었습니다.
    • 이번 과제를 수행하면서, 웹 페이지를 보면서 어떤 레이아웃으로 구성되었는지 분별할 수 있는 능력을 키우게 되었습니다.
    • 순수하게 CSS만으로 페이지를 구현할 의도로 JS를 사용하지 않았지만, 필요한 부분에서는 사용했어야 했다는 생각이 뒤늦게 들었습니다..

@netlify
Copy link

netlify bot commented Jul 28, 2023

Deploy Preview for velvety-puppy-c1029b ready!

Name Link
🔨 Latest commit ba16baa
🔍 Latest deploy log https://app.netlify.com/sites/velvety-puppy-c1029b/deploys/64d174b1f75fa80008cf1951
😎 Deploy Preview https://deploy-preview-68--velvety-puppy-c1029b.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jul 28, 2023

Deploy Preview for mellifluous-gnome-003496 ready!

Name Link
🔨 Latest commit 4adf2fc
🔍 Latest deploy log https://app.netlify.com/sites/mellifluous-gnome-003496/deploys/64c3d70f6b017600082cb1c6
😎 Deploy Preview https://deploy-preview-68--mellifluous-gnome-003496.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@wowba
Copy link

wowba commented Jul 30, 2023

style.css 에는 자주 사용되는 CSS 속성을 정리해서
필요한 HTML 태그에 적절히 위치시키는 점이 인상깊었습니다!

추후 주로 사용하는 속성 및 값의 경우 따로 파일을 만들어서 관리하는 것이
프로젝트가 거대해지면서 전체적인 디자인 시스템을 유지할 때 도움이 될 것 같습니다!

Copy link

@zoeyourlife zoeyourlife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전반적으로 깔끔하고 되게 좋은 방식의 클론코딩인 것 같습니다!!
👍 👍

Comment on lines +1 to +23
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;
}

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>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 태그에 padding 속성이 밑줄 그어져 있는데, 어떤 이유일까요

Copy link
Author

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
Comment on lines 144 to 147
<li>
<img src="./assets/image/icons8-done.svg" alt="">
<p>무광고로 음악 감상하기</p>
</li>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 주변에 흰색 배경이 넘치는 듯한? 부분은 혹시 어떤 부분인지 알 수 있을까요?

Copy link
Author

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가 제대로 적용이 안되어 있었네요.. 이 부분도 리팩토링하면서 다시 수정하려고 합니다!

@turkey-kim
Copy link

스타일관리에 신경을 써주신 코드라 느꼈습니다!
주요 섹션별로 스타일시트를 나누어둔 부분이랑, 반복되는 스타일 속성들을 처리한 방식도 인상깊게 봤습니다.
추후에 스타일 변경이 있을 때 편하게 작업할 수 있을 것 같다고 느껴진 코드였습니다 👍

Copy link

@iamidlek iamidlek left a 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; */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용하지 않는 주석 처리된 부분은 없애는 게 좋을 거 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마지막에 정리하면서 사용하지 않는 스타일이나, 주석 처리한 코드들을 제거했는데 미처 확인 못한 부분이 있었네요ㅠㅠ 확인해주셔서 감사합니다!

style.css Outdated
Comment on lines 25 to 59
.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;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 리팩토링시에 저도 시도해봐야겠어요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants