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

feat(card): creating a card #16

Merged
merged 18 commits into from
Apr 18, 2023
Merged

feat(card): creating a card #16

merged 18 commits into from
Apr 18, 2023

Conversation

sgd122
Copy link
Contributor

@sgd122 sgd122 commented Apr 18, 2023

LH-362

수정한 내용

  • 카드 컴포넌트 개발
    • PriceCard
    • HotelCard
    • HotelFeaturedCard
    • MainReviewCard
    • HotelTitelCard
    • HotelReviewCard
    • CouponCard
    • OptionCard
    • StatusCard
    • TimeLineBanner / SkeletonTimeLineBanner

@sgd122 sgd122 requested a review from baegofda April 18, 2023 00:27
@sgd122 sgd122 self-assigned this Apr 18, 2023
display: "flex",
flexDirection: "column",
padding: theme.spacing.spacing10,
borderRadius: `${theme.radius.radius20} 0 0 ${theme.radius.radius20}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: container에 borderRadius가 적용되어있어서 overflow: hidden을 추가하고 leftBox의 borderRadius는 제거해줘도 될 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해당 백그라운드영역자체는 직사각형으로 자리가 잡혀있어서 hidden으로 영역제거가 되지 않는것 같습니다 ㅠ
해당 부분은 유지해두겠슴돠 :)

content,
className,
...props
}: PropsWithChildren<CouponCardProps<C>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: children을 사용하지 않으니 PropsWithChildren타입은 정리해줘도 좋을 것 같습니다. 다른 컴포넌트들도 동일합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

overflow: "hidden",
display: "flex",
justifyContent: "center",
borderRadius: `${theme.radius.radius20} ${theme.radius.radius20} 0 0`,
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 이부분도 container에 borderRadius가 있으니 overflow: hidden을 추가하고 imageBox는 정리해주는게 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해당 백그라운드영역자체는 직사각형으로 자리가 잡혀있어서 hidden으로 영역제거가 되지 않는것 같습니다 ㅠ
해당 부분은 유지해두겠슴돠 :)

transition: "all 0.2s cubic-bezier(0, 0, 0.5, 1)",
["&:hover"]: {
width: "110%",
height: "110%",
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 성능상 이점을 위해서 width, height를 조절하기 보다는 transform: scale를 이용하는게 어떨까요? 다른 컴포넌트들도 동일합니다!

@@ -0,0 +1,38 @@
import { createStyles } from "@travelmakers-design-v2/styles";
// import { Props } from "./MainReviewCard";
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 주석 제거하면 좋을듯합니다 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,52 @@
import { createStyles } from "@travelmakers-design-v2/styles";
// import { Props } from "./OptionCard";
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 주석제거되면 좋을듯합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,47 @@
import { createStyles } from "@travelmakers-design-v2/styles";
// import { Props } from "./HotelReviewCard";
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 주석제거되면 좋을듯 합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

display: "flex",
alignItems: !isLabel && "flex-end !important",
},
footer: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 제거 되어도 좋을것 같습니다!

</View>
</View>
</div>
{props.children}
Copy link
Contributor

Choose a reason for hiding this comment

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

P4: 그냥 children으로 정리 되어도 될 것 같습니당

@sgd122 sgd122 merged commit 513c088 into main Apr 18, 2023
@sgd122 sgd122 deleted the LH-362-creating-a-card branch April 18, 2023 05:55
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.

2 participants