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

[안상균] sprint1 #55

Merged

Conversation

emotigom
Copy link
Collaborator

요구사항

넷틀리파이 링크 https://velvety-haupia-f4c1e8.netlify.app

기본

  • 헤더, 배너, 상세, 푸터 작성
  • [x]헤더 일부 반응

심화

  • [ ] 반응형 완벽하지 않음
  • [ ] 위치, 사이즈, 여백 완벽하지 않음

주요 변경사항

  • 첫 제출입니다!

스크린샷

판다마켓

멘토에게

  • 오늘 궁금한점 해결해주셔서 감사합니다!

@emotigom emotigom requested a review from Il9 March 16, 2024 05:18
@emotigom emotigom added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Mar 16, 2024
@emotigom emotigom changed the title Part1 안상균 sprint1 [안상균] sprint1 Mar 16, 2024
Copy link
Collaborator

@Il9 Il9 left a comment

Choose a reason for hiding this comment

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

완벽하진 않아도 반응형이 작동하는 첫 스프린터 분이십니다 👍
LGTM

Copy link
Collaborator

Choose a reason for hiding this comment

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

앞으로는 하신거처럼 PR Description에만 남기시면 됩니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네~ 자세한 리뷰 감사합니다~^^
주말에 시간 내주셔서 고맙습니다

</div>
</div>
</section>
<section class="main">
Copy link
Collaborator

Choose a reason for hiding this comment

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

main이라는 태그도 있습니다

Comment on lines +39 to +43
<h3>Hot item</h3>
<h1>인기 상품을<br>
확인해 보세요</h1>
<h2>가장 HOT한 중고거래 물품을<br>
판다 마켓에서 확인해 보세요</h2>
Copy link
Collaborator

Choose a reason for hiding this comment

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

h는 heading을 표현할 때 주로 사용합니다.
제목형태의 text가 아니라면 p나 span등을 활용해보세요

Copy link
Collaborator

Choose a reason for hiding this comment

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

class name의 정해진 규칙은 없지만 좀더 가독성이나 충돌이 없게끔 정리를 잘 하는게 중요합니다.
위계에 맞춰서 접두사들을 붙여주고, 일관성을 유지하도록 해보세요.
예를들면 위계는 _로 분리하고 네이밍은 kebab case로 하여 header_container_login-button이런 식으로 작성할 수 있습니다.

추가로 buttonLogin이나 bannerBottom등의 형식보단 loginButton, bottomBanner의 형태처럼 (목적 or 위치) + 명사의 형태로 작성하는게 일반적이라 다음부터는 이런쪽에도 신경을 써보세요

Copy link
Collaborator

Choose a reason for hiding this comment

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

Figma에 설정된 여백과 다른부분이 꽤 보였습니다.
디자인된것과 동일하게 구현하는 것 또한 중요한 역량이기 때문에 다음부터는 꼼꼼하게 체크하시고 적용하셨으면 합니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

같은 스타일을 적용함에도 모든 클래스가 나누어져서 각각 적용되고 있습니다.
공통적으로 사용할 수 있는 스타일은 하나의 클래스로 묶어서 같이 적용될 수 있도록 해보세요

Copy link
Collaborator

Choose a reason for hiding this comment

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

flex를 사용하시긴 하셨는데 margin등이 이상하게 섞여서 사용되고 있는 것 같습니다.
충분히 flex만 사용하여서 구현할 수 있는 레이아웃들이니 멘토링때 말씀드린 것 처럼
현재의 컨테이너에서 어느 위치에 존재하는가를 생각하면서 최대한 단순하게 만드는 것을 지향해보시면 좋을 것 같습니다

@@ -0,0 +1,236 @@
/* 헤더 */
Copy link
Collaborator

Choose a reason for hiding this comment

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

주석으로 어떤 영역인지 표시해 주신건 좋은 것 같습니다 👍

@Il9 Il9 merged commit d5e0c7d into codeit-bootcamp-frontend:part1-안상균 Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants