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

[이형준]sprint8 #240

Conversation

leehj322
Copy link
Collaborator

@leehj322 leehj322 commented Aug 2, 2024

사이트 배포 링크

요구사항

기본 요구사항

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • Typescript를 사용합니다.

체크리스트 (기본)

  • 미션 1~7까지의 내용을 TypeScript로 만들어주세요

멘토에게

  • 아직 마이그레이션 중이라 코드리뷰 전까지 최대한 작업해서 PR 올리도록 하겠습니다!
  • type이 여기저기에 흩어져 있는 상황인데 어떤식으로 정리하는것이 좋을까요?

@leehj322 leehj322 requested a review from jyh0521 August 2, 2024 10:02
@leehj322 leehj322 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 2, 2024
Copy link
Collaborator

@jyh0521 jyh0521 left a comment

Choose a reason for hiding this comment

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

과제 하느라 고생하셨습니다 ~ 👍

};

const deviceSizes = {
MOBILE_MIN_WIDTH: 375,
Copy link
Collaborator

Choose a reason for hiding this comment

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

모바일 최소 크기는 375보다 작아지는 경우도 있습니다.

@@ -0,0 +1,140 @@
import styled from "styled-components";
import headerLogoImg from "../../assets/images/panda_logo_with_typo.png";
Copy link
Collaborator

Choose a reason for hiding this comment

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

alias path 사용해보시는 것을 추천드립니다.

Copy link
Collaborator

@jyh0521 jyh0521 Aug 3, 2024

Choose a reason for hiding this comment

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

저는 개인적으로 스타일드 컴포넌트를 하단에 작성하는 것을 선호합니다. 페이지의 내용이 많아질수록 스타일드 컴포넌트가 위쪽에 자리를 잡고 있으면 컴포넌트를 확인하러 가기가 꽤 번거롭더라구요.

Comment on lines +76 to +78
@media ${({ theme }) => theme.device.mobile} {
gap: 8px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

theme 활용 좋습니다 👍

Comment on lines +62 to +75
const EXTERNAL_LINKS_DETAILS = [
{
alt: "페이스북 링크",
link: "https://www.facebook.com",
img: icFacebook,
},
{ alt: "트위터 링크", link: "https://www.twitter.com", img: icTwitter },
{ alt: "유튜브 링크", link: "https://www.youtube.com", img: icYoutube },
{
alt: "인스타 링크",
link: "https://www.instagram.com",
img: icInstagram,
},
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

map을 활용하기 위해서 상수로 관리하신 점 좋습니다.

@jyh0521
Copy link
Collaborator

jyh0521 commented Aug 3, 2024

type이 여기저기에 흩어져 있는 상황인데 어떤식으로 정리하는것이 좋을까요?

타입은 해당 컴포넌트 폴더에 types 파일을 만들어서 따로 관리해주셔도 좋고 반복적으로 사용되는 타입이라면 common 같은 폴더에 types 파일을 만들어서 사용해주시는 것도 좋습니다. 지금처럼 필요한 곳에서만 선언 하시는 것도 괜찮은 방법입니다.

@jyh0521 jyh0521 merged commit 019a4f5 into codeit-bootcamp-frontend:React-이형준 Aug 3, 2024
@leehj322
Copy link
Collaborator Author

leehj322 commented Aug 3, 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