-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Fix] 상품 정보 옵션 컴포넌트 수정 #176
The head ref may contain hidden characters: "173-fix-\uC0C1\uD488-\uC815\uBCF4-\uC635\uC158-\uCEF4\uD3EC\uB10C\uD2B8-\uC218\uC815"
Conversation
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.
고생 많으십니다 도원님 최고
@@ -48,6 +48,7 @@ | |||
color: $black; | |||
text-overflow: ellipsis; | |||
-webkit-line-clamp: 2; | |||
line-clamp: 2; |
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.
아니 이거 꿀팁이네요~
texture: switchOption.customOption.appearanceTexture, | ||
type: switchOption.customOption.layout, | ||
boardColor: switchOption.customOption.appearanceColor, | ||
switchType: switchOption.customOption.keyboardSwitch, | ||
baseKeyColor: switchOption.customOption.baseKeyColor, | ||
hasPointKeyCap: switchOption.customOption.hasPointKey, | ||
pointKeyType: switchOption.customOption.pointKeyType, | ||
pointSetColor: switchOption.customOption.pointSetColor, | ||
individualColor: switchOption.individualColor, |
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.
여기도 구조분해 해서 쓰면 조건이 너무 생길까유>?
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.
얘는 타입 조건에 따라서 사용되는 거라, 애매하지 않을까요?
다른 분들의 의견도 궁금하네요!
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.
switchOption 객체 통으로 넣어도 입력 받는쪽에서 type에 의해 필요한것만 걸러지게 되니까 저도 구조분해가 좋을거같아요
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.
구조분해가 되나요...?
타입이 string 또는 객체이고, 전달하는 값도 key가 달라서 바로 전달할 수 없는데...
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.
고생하셨습니다 LGTM!
src/app/my-info/(account)/orders/_components/Orders/OrderItemList/OrderItem/OrderItem.tsx
Outdated
Show resolved
Hide resolved
texture: switchOption.customOption.appearanceTexture, | ||
type: switchOption.customOption.layout, | ||
boardColor: switchOption.customOption.appearanceColor, | ||
switchType: switchOption.customOption.keyboardSwitch, | ||
baseKeyColor: switchOption.customOption.baseKeyColor, | ||
hasPointKeyCap: switchOption.customOption.hasPointKey, | ||
pointKeyType: switchOption.customOption.pointKeyType, | ||
pointSetColor: switchOption.customOption.pointSetColor, | ||
individualColor: switchOption.individualColor, |
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.
switchOption 객체 통으로 넣어도 입력 받는쪽에서 type에 의해 필요한것만 걸러지게 되니까 저도 구조분해가 좋을거같아요
@@ -9,6 +9,8 @@ export type OptionTypes = { | |||
optionName: string; | |||
}; | |||
|
|||
export type CategoryNameType = 'switch' | 'keyboard' | 'keycap' | 'etc'; |
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.
as const 로 사용해도 좋을거같군요
팀프로젝트나 개인 작업에서 저는 enum을 많이 활용하고 또 리뷰에서 사용을 추천하고 있었는데 알고보니 enum사용은 모던한 방법이 아니었습니다... ㅋㅋㅋ 여러분들도 enum사용이 컴파일 단계에서 어떤 차이를 두고 변환되는지 알고계시면 도움 될거같아서 공유 드립니다
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.
제목부터 무섭네요
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.
as const는 보통 객체 key 가져올 때 타입 확정지을려고 사용하는 거 아닌가요?(enum이랑 비슷한 맥락일 것 같은데...)
오히려 그냥 type 지정하는 게 더 확실한 거 아닌가요?
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.
고생하셨습니다 👍
@@ -18,7 +20,7 @@ export interface ContentType { | |||
thumbnail: string; | |||
price: number; | |||
reviewCount: number; | |||
category: 'keyboard' | 'keycap' | 'switch' | 'etc'; | |||
category: CategoryNameType; |
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.
깔꼼해요
@@ -9,6 +9,8 @@ export type OptionTypes = { | |||
optionName: string; | |||
}; | |||
|
|||
export type CategoryNameType = 'switch' | 'keyboard' | 'keycap' | 'etc'; |
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.
제목부터 무섭네요
…173-fix-상품-정보-옵션-컴포넌트-수정
🚀 작업 내용
🖼️ 스크린샷
📝 참고 사항
114번 branch merge된 후에 리뷰 남겨주세요!🚨 관련 이슈 (이슈 번호)
✅ 체크리스트