-
Notifications
You must be signed in to change notification settings - Fork 4
[당근마켓 클론코딩] 이정혁 미션 제출합니다. #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
base: main
Are you sure you want to change the base?
Conversation
|
굿모닝입니당. 위와 같은 형태로요!! |
anys34
left a comment
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.
수고하셨습니다
mission/static/index-tab/index.html
Outdated
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.
svg를 import하는 부분을 좀 더 생각하면 좋을 것 같습니다.
| div.innerHTML += `<card-comp | ||
| title="${dataa.name}" | ||
| price="${dataa.price}원" | ||
| price="20000원" |
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.
저희가 미리 만들어둔 data.json을 이용해 미션을 완수해주세요ㅠㅠ
| <div id="header-div"> | ||
| <div id="header-left"> | ||
| <a href="/mission/static/index-tab/index.html"> | ||
| <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.
이렇게 svg 코드를 다 가져와 사용하게 되면 코드가 무자비하게 길어지고, 가독성이 떨어집니다.
<img src="hihi.svg(이미지 경로)" alt="hihi img" />와 같이 img태그를 사용해주세요!
| <div class="download-btns"> | ||
| <a href="" class="store-btn"> | ||
| <svg | ||
| width="18" |
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.
이것도 위에 코멘트와 같습니다~
| <script src="../../script.js"></script> | ||
| </body> | ||
| <div id="lang-select"> | ||
| <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.
이것두요
mission/static/index-tab/section.css
Outdated
| #home-main-desc > h1 { | ||
| margin-bottom: 32px; | ||
| padding-top: 200px; | ||
| margin-top: 200px; |
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.
되도록이면 margin보다는 padding을 지향해주세용
| #fleamarket-keywords { | ||
| width: 100%; | ||
| height: 104px; | ||
|
|
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.
불필요한 공백들을 제거해주세요
data.json
Outdated
| { | ||
| "id": 1, | ||
| "img": "./assets/kitty.png", | ||
| "img": "/assets/kitty.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.
data.json파일은 고치시면 안됩니다ㅠ 저희가 만들어둔 데이터를 써주세요
mission/header.css
Outdated
| background-color: #ffffff; | ||
| height: 64px; | ||
| width: 1680px; | ||
| max-width: 100vw; |
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.
단위들을 통일시켜주세요!!
반응형 웹을 만들려면 px은 피해주세요!!
반응형에 대해 공부해보시는 것이 좋을 것 같습니다~
mission/index.html
Outdated
| <section> | ||
| <div id="home-main-top"> | ||
| <div id="home-main-desc"> | ||
| <h1>당신 근처의<br />지역 생활 커뮤니티</h1> |
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.
br은 지양해주세요
|
너무 수고많으셨습니다! 직접 파일정리를 해보셨다니 너무나도 대단해요 |
|
피드백 내용 수정했습니다 |
흥미있었던 점:
article을 추가하는 코드가 번거로울 것 같아 웹 컴포넌트를 찾아 사용해보았는데,
다른 라이브러리 없이도 컴포넌트를 쓸 수 있다는 점이 흥미로웠습니다.
어려웠던 점:
position에 대한 사용이 익숙치않아 작은 어려움이 있었습니다.
궁금한 점:
클론코딩을 하며 파일정리를 조금 해보았는데,
실제 프로젝트를 할 때는 어떤 식으로 파일 정리를 하는지 궁금했습니다.