-
Notifications
You must be signed in to change notification settings - Fork 2
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] 쇼케이스 페이지 club profile card 구현 #52
Conversation
…to feature/showcase/#47-Club-Profile-Card
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 깔끔 굿이에용
detail: '안녕하세요 티키입니다. 저희는 멋진 웹사이트를 제작합니다.', | ||
Image: Image, | ||
}, | ||
argTypes: {}, |
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.
argTypes: {
title: {
control: {
type: 'text',
},
},
detail: {
control: {
type: 'text',
},
},
},
요런식으로 컨트롤 타입도 지정해주면 스토리북을 더 잘 활용할 수 있을 것 같아요 !
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.
깔꿈!! RGTM!!
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~ 반영할 것만 해주세요
interface ClubProfileCardProps { | ||
title: string; | ||
detail: string; | ||
Image: React.FunctionComponent<React.SVGProps<SVGSVGElement>>; | ||
} |
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.
카드에서 img
로 활용할 녀석들은 svg
가 아니라 png
혹은 jpg
가 될 것 같아서 이렇게 svgr
컴포넌트를 넘겨주는게 아니라 imgUrl
을 넘겨주도록 구현하는게 맞을 것 같아요 !
prop
으로 이미지 url을 가져오고, img
태그를 활용하여 src
속성에 대입해주는 것이 올바를 것 같습니다.
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.
img
태그를 활용하는 과정에서, 최상위 컨테이너 요소를 width
100%를 적용한 후 그 안에서 img
태그는 width
백퍼, object-fit
적용, aspect-ratio
를 활용한 레이아웃 시프트 방지 등을 적용하면 좋을 것 같습니다.
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로 받아오는 걸 체크 못했는데 역시 주용님 !
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.
넵! 적용했습니다 ~ 👍
<section css={containerStyle}> | ||
<img src={imageUrl ? imageUrl : defaultImage} alt={`${title}-image`} css={imageStyle} /> | ||
<div css={descriptionStyle}> | ||
<p css={titleStyle}>{title}</p> | ||
<p css={detailStyle}>{detail}</p> | ||
</div> | ||
</section> |
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.
section
태그가 맞을까요 ?
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.
article 태그가 더 적합할 것 같아서 바꿨습니다 😶🌫️ 👍
export const containerStyle = css({ width: '24.35rem' }); | ||
|
||
export const imageStyle = css({ | ||
width: '100%', | ||
height: '14rem', | ||
|
||
borderRadius: '16px 16px 0px 0px', | ||
|
||
objectFit: 'none', | ||
aspectRatio: 16 / 9, | ||
}); |
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.
Card
의 컨테이너 속성이 100%
가 되어야 할 것 같습니다 !
img
는 width
만 백퍼를 적용한 후, aspectRatio 적용하면 될 것 같아요.
또한 objectFit
cover 로 !
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.
넵! 수정했습니다! 감사합니다~~ 👍
해당 이슈 번호
closed #47
체크리스트
📌 질문할 부분
캡쳐